The following patches propose an implementation of project support
for ext4. A project is an aggregate of unrelated inodes which might
scatter in different directories. Inodes belongs to a project
possesses a same identification i.e. 'project ID', just like every
inode has its user/group indentification. The following patches adds
project quota as supplement to the former uer/group quota types.
This project ID of an inode is iherited from its parent direcotry
and saved as an internal field of ext4 inode.
This is not the first existed attepmtion to add project quta support
for ext4. Patches of subtree quota support which was posted by Dmity
Monakhov in 2012 (http://lwn.net/Articles/506064/) implemented the
similar feature in a different way. Rather than saving the project
(or subtree) ID as an internal inode field, those patches manages
the ID as extented attributes.
We rebased both patch sets onto the same kernel version and run
benchmakrs respectively to comparing the peformance difference.
It is worth noting that patches from Lai Siyao and Niu Yawei
(quota: remove dqptr_sem,
http://article.gmane.org/gmane.comp.file-systems.ext4/44341/)
improve the performance of quota enforcement significantly, which
can be seen clearly from following results.
It is obvious that extended attribute implementation has performance
impact when creating files. That is why we choose to push the patches
which use internal inode field to save project ID.
Kernel: 3.16.0-rc5
Server: Dell R620 (2 x [email protected], 256GB memory)
Storage: 10 x 15K SAS disks(RAID10)
Test tool: mdtest-1.9.3. Mdtest created 800K files in total. Each
thread created files in unique directory.
File Creation:
1thr 2thr 4thr 8thr 16thr
- vanilla
quota disabled 66094 105781 178968 186647 172536
quotaon(ug) 60337 99582 157396 171463 162872
- vanilla + remove-dqptr_sem patches
quota disabled 65955 112082 185550 181511 171988
quotaon(ug) 62391 101905 171013 190570 168914
- prjquota(xattr)
quota disabled 61396 97580 147852 146423 164895
quotaon(ug) 57009 93435 140589 135748 153196
quotaon(ugP) 57500 89419 133604 125291 105127
- prjquota(xattr) + remove-dqptr_sem patches
quota disabled 64053 100078 147608 139403 163960
quotaon(ug) 60754 104726 149231 139053 165990
quotaon(ugP) 59238 93606 148921 138434 163931
- prjquota(internal) + remove-dqptr_sem patches
quota disabled 65826 111828 181486 189227 171241
quotaon(ug) 65418 107745 173584 180562 173752
quotaon(ugP) 64669 103890 169176 186426 172192
File Removal:
1thr 2thr 4thr 8thr 16thr
- vanilla
quota disabled 118059 169825 234661 291812 345656
quotaon(ug) 106675 135834 153532 100437 87489
- vanilla + remove-dqptr_sem patches
quota disabled 120374 168437 236818 291754 331141
quotaon(ug) 110709 161954 238333 293700 329015
- prjquota(xattr)
quota disabled 116680 161662 229190 295642 332959
quotaon(ug) 104783 134359 154950 100516 87923
quotaon(ugP) 100240 125978 108653 68286 58991
- prjquota(xattr) + remove-dqptr_sem patches
quota disabled 116281 168938 233733 286663 344002
quotaon(ug) 109775 164995 236001 299389 340683
quotaon(ugP) 113935 162979 236112 300033 356117
- prjquota(internal) + remove-dqptr_sem patches
quota disabled 119537 171565 247418 291068 350138
quotaon(ug) 121756 159580 240778 298012 342437
quotaon(ugP) 118954 168022 241206 289055 334008
[PATCH 1/4] quota: add project quota support
Adds project identifier of inode.
Project identifier is saved as an internal attribute of inode.
This patch adds a new field in inode structure and inode attribute
structure for project ID.
Signed-off-by: Li Xi <lixi <at> ddn.com>
---
/* First generic header */
Index: linux.git/include/linux/fs.h
===================================================================
--- linux.git.orig/include/linux/fs.h
+++ linux.git/include/linux/fs.h
@@ -26,6 +26,7 @@
#include <linux/shrinker.h>
#include <linux/migrate_mode.h>
#include <linux/uidgid.h>
+#include <linux/projid.h>
#include <linux/lockdep.h>
#include <linux/percpu-rwsem.h>
#include <linux/blk_types.h>
@@ -223,6 +224,7 @@ typedef void (dio_iodone_t)(struct kiocb
#define ATTR_KILL_PRIV (1 << 14)
#define ATTR_OPEN (1 << 15) /* Truncating from open(O_TRUNC) */
#define ATTR_TIMES_SET (1 << 16)
+#define ATTR_PROJID (1 << 17)
/*
* This is the Inode Attributes structure, used for notify_change(). It
@@ -238,6 +240,7 @@ struct iattr {
umode_t ia_mode;
kuid_t ia_uid;
kgid_t ia_gid;
+ kprojid_t ia_projid;
loff_t ia_size;
struct timespec ia_atime;
struct timespec ia_mtime;
@@ -504,6 +507,7 @@ struct inode {
unsigned short i_opflags;
kuid_t i_uid;
kgid_t i_gid;
+ kprojid_t i_projid;
unsigned int i_flags;
#ifdef CONFIG_FS_POSIX_ACL
@@ -701,6 +705,11 @@ static inline void i_gid_write(struct in
inode->i_gid = make_kgid(&init_user_ns, gid);
}
+static inline void i_projid_write(struct inode *inode, projid_t projid)
+{
+ inode->i_projid = make_kprojid(&init_user_ns, projid);
+}
+
static inline unsigned iminor(const struct inode *inode)
{
return MINOR(inode->i_rdev);
Index: linux.git/fs/attr.c
===================================================================
--- linux.git.orig/fs/attr.c
+++ linux.git/fs/attr.c
@@ -60,6 +60,11 @@ int inode_change_ok(const struct inode *
!capable_wrt_inode_uidgid(inode, CAP_CHOWN))
return -EPERM;
+ /* Make sure caller can change project. */
+ if ((ia_valid & ATTR_PROJID) &&
+ (!capable(CAP_SYS_ADMIN)))
+ return -EPERM;
+
/* Make sure a caller can chmod. */
if (ia_valid & ATTR_MODE) {
if (!inode_owner_or_capable(inode))
@@ -147,6 +152,8 @@ void setattr_copy(struct inode *inode, c
inode->i_uid = attr->ia_uid;
if (ia_valid & ATTR_GID)
inode->i_gid = attr->ia_gid;
+ if (ia_valid & ATTR_PROJID)
+ inode->i_projid = attr->ia_projid;
if (ia_valid & ATTR_ATIME)
inode->i_atime = timespec_trunc(attr->ia_atime,
inode->i_sb->s_time_gran);
@@ -197,7 +204,8 @@ int notify_change(struct dentry * dentry
WARN_ON_ONCE(!mutex_is_locked(&inode->i_mutex));
- if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_TIMES_SET)) {
+ if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | ATTR_PROJID |
+ ATTR_TIMES_SET)) {
if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
return -EPERM;
}
Index: linux.git/fs/inode.c
===================================================================
--- linux.git.orig/fs/inode.c
+++ linux.git/fs/inode.c
@@ -138,6 +138,7 @@ int inode_init_always(struct super_block
inode->i_opflags = 0;
i_uid_write(inode, 0);
i_gid_write(inode, 0);
+ i_projid_write(inode, 0);
atomic_set(&inode->i_writecount, 0);
inode->i_size = 0;
inode->i_blocks = 0;
On Fri 01-08-14 09:05:23, Li Xi wrote:
> The following patches propose an implementation of project support
> for ext4. A project is an aggregate of unrelated inodes which might
> scatter in different directories. Inodes belongs to a project
> possesses a same identification i.e. 'project ID', just like every
> inode has its user/group indentification. The following patches adds
> project quota as supplement to the former uer/group quota types.
> This project ID of an inode is iherited from its parent direcotry
> and saved as an internal field of ext4 inode.
>
> This is not the first existed attepmtion to add project quta support
> for ext4. Patches of subtree quota support which was posted by Dmity
> Monakhov in 2012 (http://lwn.net/Articles/506064/) implemented the
> similar feature in a different way. Rather than saving the project
> (or subtree) ID as an internal inode field, those patches manages
> the ID as extented attributes.
Thanks for reviving the feature. I think it is a useful one. I had a
brief look into the series and here are some highlevel comments:
1) It should have been also posted to [email protected], Al Viro
<[email protected]>, Christoph Hellwig <[email protected]> because
you are changing core VFS inode and infrastructure as well. For quota
changes you should have also CCed me as a quota maintainer.
2) I'm not convinced we actually want project ID in the core inode - so far
only XFS has this. For everyone else it's just extra bloat so we could just
put it in ext4_inode_info. Granted we'd need to somewhat change quota
interface so that it sees all the ids (uid, gid, projid) but they are
really needed in two places - dquot_initalize() and dquot_transfer() and
creating variants of these functions that just take an array of ids and use
them in ext4 is simple enough.
3) I see no way how to get / set project ID from userspace. Did I miss
something?
4) The ext4 change is changing on-disk format. You definitely need a
feature flag for that so that kernels that don't understand project quotas
don't corrupt the filesystem. Also you need a support for this in
e2fsprogs.
5) You make the feature configurable both in quota code and ext4. I don't
think the footprint of the feature warrants that.
> We rebased both patch sets onto the same kernel version and run
> benchmakrs respectively to comparing the peformance difference.
> It is worth noting that patches from Lai Siyao and Niu Yawei
> (quota: remove dqptr_sem,
> http://article.gmane.org/gmane.comp.file-systems.ext4/44341/)
> improve the performance of quota enforcement significantly, which
> can be seen clearly from following results.
I plan to send that to Linus in this merge window BTW.
> It is obvious that extended attribute implementation has performance
> impact when creating files. That is why we choose to push the patches
> which use internal inode field to save project ID.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
2014-08-01 20:40 GMT+08:00 Jan Kara <[email protected]>:
>
> 1) It should have been also posted to [email protected], Al Viro
> <[email protected]>, Christoph Hellwig <[email protected]> because
> you are changing core VFS inode and infrastructure as well. For quota
> changes you should have also CCed me as a quota maintainer.
Sure. Thanks for reminding me. I will add these addresses next time.
>
> 2) I'm not convinced we actually want project ID in the core inode - so far
> only XFS has this. For everyone else it's just extra bloat so we could just
> put it in ext4_inode_info. Granted we'd need to somewhat change quota
> interface so that it sees all the ids (uid, gid, projid) but they are
> really needed in two places - dquot_initalize() and dquot_transfer() and
> creating variants of these functions that just take an array of ids and use
> them in ext4 is simple enough.
OK, agreed.
>
> 3) I see no way how to get / set project ID from userspace. Did I miss
> something?
Yeah, you are right. I didn't push the interface patch. We implemeted
two kinds of interfaces. One is based on extended attribute interfaces which
simulate project ID as an extended attribute of the inode. The other implemented
a new ioctl command for ext4. Personally, I think ioctl is better. What is your
opinion?
>
> 4) The ext4 change is changing on-disk format. You definitely need a
> feature flag for that so that kernels that don't understand project quotas
> don't corrupt the filesystem. Also you need a support for this in
> e2fsprogs.
Sure. I will add feature flag for project quota of ext4. And there are
quite some
related tools need updates, including e2fsprogs and quota-tools. We will push
those patches as soon as they are ready.
>
> 5) You make the feature configurable both in quota code and ext4. I don't
> think the footprint of the feature warrants that.
Sure. Good point.
Thank you for the advices, I will refresh the patches and send the mails to all
of the related addresses soon.
Regards,
Li Xi
2014-08-01 23:28 GMT+08:00 Li Xi <[email protected]>:
> 2014-08-01 20:40 GMT+08:00 Jan Kara <[email protected]>:
>>
>> 1) It should have been also posted to [email protected], Al Viro
>> <[email protected]>, Christoph Hellwig <[email protected]> because
>> you are changing core VFS inode and infrastructure as well. For quota
>> changes you should have also CCed me as a quota maintainer.
> Sure. Thanks for reminding me. I will add these addresses next time.
>>
>> 2) I'm not convinced we actually want project ID in the core inode - so far
>> only XFS has this. For everyone else it's just extra bloat so we could just
>> put it in ext4_inode_info. Granted we'd need to somewhat change quota
>> interface so that it sees all the ids (uid, gid, projid) but they are
>> really needed in two places - dquot_initalize() and dquot_transfer() and
>> creating variants of these functions that just take an array of ids and use
>> them in ext4 is simple enough.
> OK, agreed.
After searching dquot_initalize() and dquot_transfer(), I found changing these
two functions envolves too many file systems. Is there any good reason not to
add kprojid_t field in inode structure?
>>
>> 3) I see no way how to get / set project ID from userspace. Did I miss
>> something?
> Yeah, you are right. I didn't push the interface patch. We implemeted
> two kinds of interfaces. One is based on extended attribute interfaces which
> simulate project ID as an extended attribute of the inode. The other implemented
> a new ioctl command for ext4. Personally, I think ioctl is better. What is your
> opinion?
>>
>> 4) The ext4 change is changing on-disk format. You definitely need a
>> feature flag for that so that kernels that don't understand project quotas
>> don't corrupt the filesystem. Also you need a support for this in
>> e2fsprogs.
> Sure. I will add feature flag for project quota of ext4. And there are
> quite some
> related tools need updates, including e2fsprogs and quota-tools. We will push
> those patches as soon as they are ready.
>>
>> 5) You make the feature configurable both in quota code and ext4. I don't
>> think the footprint of the feature warrants that.
> Sure. Good point.
>
> Thank you for the advices, I will refresh the patches and send the mails to all
> of the related addresses soon.
>
> Regards,
> Li Xi
On Fri 01-08-14 23:48:31, Li Xi wrote:
> 2014-08-01 23:28 GMT+08:00 Li Xi <[email protected]>:
> > 2014-08-01 20:40 GMT+08:00 Jan Kara <[email protected]>:
> >>
> >> 1) It should have been also posted to [email protected], Al Viro
> >> <[email protected]>, Christoph Hellwig <[email protected]> because
> >> you are changing core VFS inode and infrastructure as well. For quota
> >> changes you should have also CCed me as a quota maintainer.
> > Sure. Thanks for reminding me. I will add these addresses next time.
> >>
> >> 2) I'm not convinced we actually want project ID in the core inode - so far
> >> only XFS has this. For everyone else it's just extra bloat so we could just
> >> put it in ext4_inode_info. Granted we'd need to somewhat change quota
> >> interface so that it sees all the ids (uid, gid, projid) but they are
> >> really needed in two places - dquot_initalize() and dquot_transfer() and
> >> creating variants of these functions that just take an array of ids and use
> >> them in ext4 is simple enough.
> > OK, agreed.
> After searching dquot_initalize() and dquot_transfer(), I found changing these
> two functions envolves too many file systems. Is there any good reason not to
> add kprojid_t field in inode structure?
Yes. It grows struct inode which is used by *all* filesystems and only
ext4 (and possibly xfs) would use it. That's why I suggested you create
something like:
struct inode_ids {
kuid_t uid;
kgid_t gid;
kprojid_t projid;
};
void dquot_initialize_ids(struct inode *inode, struct inode_ids *ids)
{
...
}
and
static inline void dquot_initialize(struct inode *inode)
{
struct inode_ids ids = {
.uid = inode->i_uid,
.gid = inode->i_gid,
.projid = INVALID_PROJID,
};
dquot_initialize_ids(inode, &ids);
}
Then filesystems not using project ids remain untouched and filesystems
with project ids (i.e. ext4) can use dquot_initialize_ids() - probably you
should create something like:
static inline void ext4_dquot_initialize(struct inode *inode)
{
struct inode_ids ids = {
.uid = inode->i_uid,
.gid = inode->i_gid,
.projid = EXT4_I(inode)->i_projid,
};
dquot_initialize_ids(inode, &ids);
}
and use ext4_dquot_initialize() throughout ext4.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
2014-08-02 4:17 GMT+08:00 Jan Kara <[email protected]>:
> On Fri 01-08-14 23:48:31, Li Xi wrote:
>> 2014-08-01 23:28 GMT+08:00 Li Xi <[email protected]>:
>> > 2014-08-01 20:40 GMT+08:00 Jan Kara <[email protected]>:
>> >>
>> >> 1) It should have been also posted to [email protected], Al Viro
>> >> <[email protected]>, Christoph Hellwig <[email protected]> because
>> >> you are changing core VFS inode and infrastructure as well. For quota
>> >> changes you should have also CCed me as a quota maintainer.
>> > Sure. Thanks for reminding me. I will add these addresses next time.
>> >>
>> >> 2) I'm not convinced we actually want project ID in the core inode - so far
>> >> only XFS has this. For everyone else it's just extra bloat so we could just
>> >> put it in ext4_inode_info. Granted we'd need to somewhat change quota
>> >> interface so that it sees all the ids (uid, gid, projid) but they are
>> >> really needed in two places - dquot_initalize() and dquot_transfer() and
>> >> creating variants of these functions that just take an array of ids and use
>> >> them in ext4 is simple enough.
>> > OK, agreed.
>> After searching dquot_initalize() and dquot_transfer(), I found changing these
>> two functions envolves too many file systems. Is there any good reason not to
>> add kprojid_t field in inode structure?
> Yes. It grows struct inode which is used by *all* filesystems and only
> ext4 (and possibly xfs) would use it. That's why I suggested you create
> something like:
>
> struct inode_ids {
> kuid_t uid;
> kgid_t gid;
> kprojid_t projid;
> };
>
> void dquot_initialize_ids(struct inode *inode, struct inode_ids *ids)
> {
> ...
> }
>
> and
>
> static inline void dquot_initialize(struct inode *inode)
> {
> struct inode_ids ids = {
> .uid = inode->i_uid,
> .gid = inode->i_gid,
> .projid = INVALID_PROJID,
> };
> dquot_initialize_ids(inode, &ids);
> }
>
> Then filesystems not using project ids remain untouched and filesystems
> with project ids (i.e. ext4) can use dquot_initialize_ids() - probably you
> should create something like:
>
> static inline void ext4_dquot_initialize(struct inode *inode)
> {
> struct inode_ids ids = {
> .uid = inode->i_uid,
> .gid = inode->i_gid,
> .projid = EXT4_I(inode)->i_projid,
> };
> dquot_initialize_ids(inode, &ids);
> }
>
> and use ext4_dquot_initialize() throughout ext4.
>
I tried to change it in this way, but there ia another problem. add_dquot_ref()
calls __dquot_initialize() for each inode in the list of super block. That means
there is no way to pass projid as an argument of __dquot_initialize(). The
solution is to add a get_projid() method (and maybe set_projid too) in the
inode_operations structure. Personally, I perfer to add projid in the inode
stucture, since projid looks like uid and gid of an inode.
get_projid()/setprojid()
looks duplicated with getattr()/setattr() or getxattr()/setxattr(). Is there any
performance impact of increasing size of inode structure, e.g. cache
line problem? I will add get_projid() method if so.
Thanks,
Li Xi
On Sat 02-08-14 09:35:58, Li Xi wrote:
> 2014-08-02 4:17 GMT+08:00 Jan Kara <[email protected]>:
> > On Fri 01-08-14 23:48:31, Li Xi wrote:
> >> 2014-08-01 23:28 GMT+08:00 Li Xi <[email protected]>:
> >> > 2014-08-01 20:40 GMT+08:00 Jan Kara <[email protected]>:
> >> >>
> >> >> 1) It should have been also posted to [email protected], Al Viro
> >> >> <[email protected]>, Christoph Hellwig <[email protected]> because
> >> >> you are changing core VFS inode and infrastructure as well. For quota
> >> >> changes you should have also CCed me as a quota maintainer.
> >> > Sure. Thanks for reminding me. I will add these addresses next time.
> >> >>
> >> >> 2) I'm not convinced we actually want project ID in the core inode - so far
> >> >> only XFS has this. For everyone else it's just extra bloat so we could just
> >> >> put it in ext4_inode_info. Granted we'd need to somewhat change quota
> >> >> interface so that it sees all the ids (uid, gid, projid) but they are
> >> >> really needed in two places - dquot_initalize() and dquot_transfer() and
> >> >> creating variants of these functions that just take an array of ids and use
> >> >> them in ext4 is simple enough.
> >> > OK, agreed.
> >> After searching dquot_initalize() and dquot_transfer(), I found changing these
> >> two functions envolves too many file systems. Is there any good reason not to
> >> add kprojid_t field in inode structure?
> > Yes. It grows struct inode which is used by *all* filesystems and only
> > ext4 (and possibly xfs) would use it. That's why I suggested you create
> > something like:
> >
> > struct inode_ids {
> > kuid_t uid;
> > kgid_t gid;
> > kprojid_t projid;
> > };
> >
> > void dquot_initialize_ids(struct inode *inode, struct inode_ids *ids)
> > {
> > ...
> > }
> >
> > and
> >
> > static inline void dquot_initialize(struct inode *inode)
> > {
> > struct inode_ids ids = {
> > .uid = inode->i_uid,
> > .gid = inode->i_gid,
> > .projid = INVALID_PROJID,
> > };
> > dquot_initialize_ids(inode, &ids);
> > }
> >
> > Then filesystems not using project ids remain untouched and filesystems
> > with project ids (i.e. ext4) can use dquot_initialize_ids() - probably you
> > should create something like:
> >
> > static inline void ext4_dquot_initialize(struct inode *inode)
> > {
> > struct inode_ids ids = {
> > .uid = inode->i_uid,
> > .gid = inode->i_gid,
> > .projid = EXT4_I(inode)->i_projid,
> > };
> > dquot_initialize_ids(inode, &ids);
> > }
> >
> > and use ext4_dquot_initialize() throughout ext4.
> >
> I tried to change it in this way, but there ia another problem. add_dquot_ref()
> calls __dquot_initialize() for each inode in the list of super block. That means
> there is no way to pass projid as an argument of __dquot_initialize(). The
> solution is to add a get_projid() method (and maybe set_projid too) in the
> inode_operations structure.
Hum, you are right. add_dquot_ref() is an issue. So probably we'll have
add get_projid() method to struct dquot_operations (similar to
get_reserved_space() we have there).
> Personally, I perfer to add projid in the inode
> stucture, since projid looks like uid and gid of an inode.
> get_projid()/setprojid()
> looks duplicated with getattr()/setattr() or getxattr()/setxattr(). Is there any
> performance impact of increasing size of inode structure, e.g. cache
> line problem? I will add get_projid() method if so.
I agree projid looks like uid or gid. But uid & gid are in POSIX so
everyone needs them. projid isn't so we'll just waste space for lots of
inodes (not too much but still some and this way struct inode gets slowly
bloated and we can have lots of inodes around). And it's not too hard to
live without project ID in core inode... Once significant portion of
filesystems start to support project ID, situation is different and we'll
move project ID into core inode.
Regarding getting & setting project ID from userspace I suppose you'll have
to resort to fs-specific ioctl like XFS does and then setting project ID in
fs-specific part of the inode is trivial.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR
> Hum, you are right. add_dquot_ref() is an issue. So probably we'll have
> add get_projid() method to struct dquot_operations (similar to
> get_reserved_space() we have there).
>
>> Personally, I perfer to add projid in the inode
>> stucture, since projid looks like uid and gid of an inode.
>> get_projid()/setprojid()
>> looks duplicated with getattr()/setattr() or getxattr()/setxattr(). Is there any
>> performance impact of increasing size of inode structure, e.g. cache
>> line problem? I will add get_projid() method if so.
> I agree projid looks like uid or gid. But uid & gid are in POSIX so
> everyone needs them. projid isn't so we'll just waste space for lots of
> inodes (not too much but still some and this way struct inode gets slowly
> bloated and we can have lots of inodes around). And it's not too hard to
> live without project ID in core inode... Once significant portion of
> filesystems start to support project ID, situation is different and we'll
> move project ID into core inode.
>
> Regarding getting & setting project ID from userspace I suppose you'll have
> to resort to fs-specific ioctl like XFS does and then setting project ID in
> fs-specific part of the inode is trivial.
OK. Thank you very much for your detailed advices. I will refresh the patches
soon.
Regards,
Li Xi