Re: [PATCH v3 2/4] quota: add project quota support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sep 10, 2014, at 1:47 PM, Jan Kara <jack@xxxxxxx> wrote:
> On Wed 10-09-14 11:54:07, Li Xi wrote:
>> Adds project ID support for ext4
>> 
>> This patch adds a new internal field of ext4 inode to save project
>> identifier. Also a new flag EXT4_INODE_PROJINHERIT is added for
>> inheriting project ID from parent directory.
>> 
>> Signed-off-by: Li Xi <lixi <at> ddn.com>
>> ---
>> Index: linux.git/fs/ext4/ialloc.c
>> ===================================================================
>> --- linux.git.orig/fs/ext4/ialloc.c
>> +++ linux.git/fs/ext4/ialloc.c
>> @@ -756,6 +756,16 @@ struct inode *__ext4_new_inode(handle_t
>>         inode->i_gid = dir->i_gid;
>>     } else
>>         inode_init_owner(inode, dir, mode);
>> +#ifdef CONFIG_QUOTA_PROJECT
>> +    if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT)) {
>> +        if (ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT)) {
>> +            ei->i_projid = EXT4_I(dir)->i_projid;
>> +        } else {
>> +            ei->i_projid =
>> +                make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
>> +        }
>> +    }
>> +#endif
>  Hum, I'd just always store the default project ID in the inode (i.e., you
> can remove the feature test). Then ext4_get_projid() also doesn't have to
> bother checking the feature and can just return whatever is in i_projid.

Makes sense.

>>     dquot_initialize(inode);
>> 
>>     if (!goal)
>> Index: linux.git/fs/ext4/ext4.h
>> ===================================================================
>> --- linux.git.orig/fs/ext4/ext4.h
>> +++ linux.git/fs/ext4/ext4.h
>> @@ -386,16 +386,18 @@ struct flex_groups {
>> #define EXT4_EA_INODE_FL            0x00200000 /* Inode used for large EA */
>> #define EXT4_EOFBLOCKS_FL        0x00400000 /* Blocks allocated beyond EOF */
>> #define EXT4_INLINE_DATA_FL        0x10000000 /* Inode has inline data. */
>> +#define EXT4_PROJINHERIT_FL        0x20000000 /* Create with parents projid */
>  Why not use the next available flag - 0x00800000?

Because Btrfs is using 0x00800000 for FS_NOCOW_FL, and until such a
time that we need to implement conflicting flags it would be better
to keep them non-overlapping.  It seems reasonable that since both
XFS and ext4 have a "PROJINHERIT" flag that this should also be
defined as FS_PROJINHERIT_FL to avoid other conflicts.

Please see lib/ext2fs/ext2_fs.h in newer e2fsprogs.

>> #define EXT4_RESERVED_FL        0x80000000 /* reserved for ext4 lib */
>> 
>> -#define EXT4_FL_USER_VISIBLE        0x004BDFFF /* User visible flags */
>> -#define EXT4_FL_USER_MODIFIABLE        0x004380FF /* User modifiable flags */
>> +#define EXT4_FL_USER_VISIBLE        0x204BDFFF /* User visible flags */
>> +#define EXT4_FL_USER_MODIFIABLE        0x204380FF /* User modifiable flags */
>> 
>> /* Flags that should be inherited by new inodes from their parent. */
>> #define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
>>                EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
>>                EXT4_NOCOMPR_FL | EXT4_JOURNAL_DATA_FL |\
>> -               EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL)
>> +               EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL |\
>> +               EXT4_PROJINHERIT_FL)
>> 
>> /* Flags that are appropriate for regular files (all but dir-specific ones). */
>> #define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL))
>> @@ -443,6 +445,7 @@ enum {
>>     EXT4_INODE_EA_INODE    = 21,    /* Inode used for large EA */
>>     EXT4_INODE_EOFBLOCKS    = 22,    /* Blocks allocated beyond EOF */
>>     EXT4_INODE_INLINE_DATA    = 28,    /* Data in inode. */
>> +    EXT4_INODE_PROJINHERIT    = 29,    /* Create with parents projid */
>  This will need updating as well.

Or not...

>>     EXT4_INODE_RESERVED    = 31,    /* reserved for ext4 lib */
>> };
>> 
>> @@ -695,6 +698,7 @@ struct ext4_inode {
>>     __le32  i_crtime;       /* File Creation time */
>>     __le32  i_crtime_extra; /* extra FileCreationtime (nsec << 2 | epoch) */
>>     __le32  i_version_hi;    /* high 32 bits for 64-bit version */
>> +    __le32  i_projid;    /* Project ID */
>> };
>> 
>> struct move_extent {
>> @@ -943,6 +947,9 @@ struct ext4_inode_info {
>> 
>>     /* Precomputed uuid+inum+igen checksum for seeding inode checksums */
>>     __u32 i_csum_seed;
>> +#ifdef CONFIG_QUOTA_PROJECT
>> +    kprojid_t i_projid;
>> +#endif
>> };
>> 
>> /*
>> @@ -1525,6 +1532,7 @@ static inline void ext4_clear_state_flag
>>  * GDT_CSUM bits are mutually exclusive.
>>  */
>> #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM    0x0400
>> +#define EXT4_FEATURE_RO_COMPAT_PROJECT        0x1000 /* Project quota */
>  Why not use the next bit available - i.e. 0x0800?

That is also already in use in e2fsprogs:

    #define EXT4_FEATURE_RO_COMPAT_REPLICA          0x0800

>> #define EXT4_FEATURE_INCOMPAT_COMPRESSION    0x0001
>> #define EXT4_FEATURE_INCOMPAT_FILETYPE        0x0002
>> @@ -1574,7 +1582,8 @@ static inline void ext4_clear_state_flag
>>                      EXT4_FEATURE_RO_COMPAT_HUGE_FILE |\
>>                      EXT4_FEATURE_RO_COMPAT_BIGALLOC |\
>>                      EXT4_FEATURE_RO_COMPAT_METADATA_CSUM|\
>> -                     EXT4_FEATURE_RO_COMPAT_QUOTA)
>> +                     EXT4_FEATURE_RO_COMPAT_QUOTA |\
>> +                     EXT4_FEATURE_RO_COMPAT_PROJECT)
>> 
>> /*
>>  * Default values for user and/or group using reserved blocks
>> @@ -1582,6 +1591,11 @@ static inline void ext4_clear_state_flag
>> #define    EXT4_DEF_RESUID        0
>> #define    EXT4_DEF_RESGID        0
>> 
>> +/*
>> + * Default project ID
>> + */
>> +#define    EXT4_DEF_PROJID        0
>> +
>> #define EXT4_DEF_INODE_READAHEAD_BLKS    32
>> 
>> /*
>> @@ -2133,6 +2147,9 @@ extern int ext4_zero_partial_blocks(hand
>>                  loff_t lstart, loff_t lend);
>> extern int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf);
>> extern qsize_t *ext4_get_reserved_space(struct inode *inode);
>> +#ifdef CONFIG_QUOTA_PROJECT
>> +extern int ext4_get_projid(struct inode *inode, kprojid_t *projid);
>> +#endif
>> extern void ext4_da_update_reserve_space(struct inode *inode,
>>                     int used, int quota_claim);
>> 
>> Index: linux.git/fs/ext4/inode.c
>> ===================================================================
>> --- linux.git.orig/fs/ext4/inode.c
>> +++ linux.git/fs/ext4/inode.c
>> @@ -4026,6 +4026,47 @@ static inline void ext4_iget_extra_inode
>>         EXT4_I(inode)->i_inline_off = 0;
>> }
>> 
>> +#ifdef CONFIG_QUOTA_PROJECT
>> +static int ext4_inode_projid_get(struct inode *inode, struct ext4_inode *raw,
>> +                 struct ext4_inode_info *ei, projid_t *projid)
>> +{
>> +    BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
>> +            EXT4_FEATURE_RO_COMPAT_PROJECT));
>> +
>> +    if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
>> +        EXT4_FITS_IN_INODE(raw, ei, i_projid)) {
>> +        *projid = (projid_t)le32_to_cpu(raw->i_projid);
>> +        return 0;
>> +    }
>> +    return -EFBIG;
>> +}
>  Maybe you should just return the default project ID for inodes that
> cannot store one?

Sure, on the "get" side that would probably be OK.

>> +static int ext4_inode_projid_set(struct inode *inode, struct ext4_inode *raw,
>> +                 struct ext4_inode_info *ei, projid_t projid)
>> +{
>> +    BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
>> +            EXT4_FEATURE_RO_COMPAT_PROJECT));
>> +
>> +    if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE &&
>> +        EXT4_FITS_IN_INODE(raw, ei, i_projid)) {
>> +        raw->i_projid = cpu_to_le32(projid);
>> +        return 0;
>> +    }
>> +    return -EFBIG;
>> +}
> 
>  The test whether project ID fits into an inode should be done when
> userspace tries to set project ID for the inode in the ioctl. Thus when
> we are trying to write the inode and projid doesn't fit into the inode, we
> know projid is the default one and we can just avoid storing it.
> ext4_inode_projid_get() will then properly interpret this as the default
> projid. This way we can make project ids work reasonably for filesystems
> where inode needn't have enough space.

We also have patches for e2fsprogs to allow e2fsck to resize the
i_extra_isize on existing inodes to ensure that there is always
enough space in the inode to store the project quota.

Cheers, Andreas

>> +int ext4_get_projid(struct inode *inode, kprojid_t *projid)
>> +{
>> +    if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
>> +            EXT4_FEATURE_RO_COMPAT_PROJECT)) {
>> +        return -ENOTSUPP;
>> +    }
>> +
>> +    *projid = EXT4_I(inode)->i_projid;
>> +    return 0;
>> +}
>> +#endif
>> +
> ...
> 
> 								Honza
> -- 
> Jan Kara <jack@xxxxxxx>
> SUSE Labs, CR


Cheers, Andreas





Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux