xfs
[Top] [All Lists]

Re: [PATCH VER 4] Extend project quotas to support 32bit project identif

To: Arkadiusz Mi??kiewicz <arekm@xxxxxxxx>
Subject: Re: [PATCH VER 4] Extend project quotas to support 32bit project identificators.
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 22 Sep 2010 14:42:42 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1285177343-11108-1-git-send-email-arekm@xxxxxxxx>
References: <1285177343-11108-1-git-send-email-arekm@xxxxxxxx>
User-agent: Mutt/1.5.20 (2009-08-17)
On Wed, Sep 22, 2010 at 07:42:23PM +0200, Arkadiusz Mi??kiewicz wrote:
>       if (mask & FSX_PROJID) {
> +             /*
> +              * Switch on the PROJID32BIT superblock bit when needed
> +              * (implies also FEATURES2)
> +              */
> +             if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb) &&
> +                             fa->fsx_projid > (__uint16_t)-1)
> +                     xfs_addprojid32bit(tp, ip);

Didn't we agree that we want to enable this feature explicitly via
xfs_admin (or mkfs.xfs)?

> diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h
> index 87c2e9d..c03c752 100644
> --- a/fs/xfs/xfs_fs.h
> +++ b/fs/xfs/xfs_fs.h
> @@ -293,9 +293,10 @@ typedef struct xfs_bstat {
>       __s32           bs_extsize;     /* extent size                  */
>       __s32           bs_extents;     /* number of extents            */
>       __u32           bs_gen;         /* generation count             */
> -     __u16           bs_projid;      /* project id                   */
> +     __u16           bs_projid_lo;   /* lower part of project id     */
>       __u16           bs_forkoff;     /* inode fork offset in bytes   */
> -     unsigned char   bs_pad[12];     /* pad space, unused            */
> +     __u16           bs_projid_hi;   /* higher part of project id    */
> +     unsigned char   bs_pad[10];     /* pad space, unused            */

Unlike in the inode we can't just rename the lo field here - that would
break the compilation of existing applications.

>  /*
> + * Project quota id helpers
> + */

Maybe add a little comment here that the split is because the projid
historically was just 16 bits on disk?

> +static inline prid_t
> +xfs_get_projid(xfs_inode_t *ip)

Please always use struct xfs_foo instead of xfs_foo_t for new code.

>       return 0;
>  }
> +
> +/*
> + * Switches on the PROJID32BIT superblock bit
> + * (implies also FEATURES2).
> + */
> +
> +int

No need for a space after the comment.

> +{
> +     spin_lock(&ip->i_mount->m_sb_lock);
> +     if (!xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb)) {

Wedon't need the lock around the check, it's enough if it's inside the
conditional.

> +             xfs_sb_version_addprojid32bit(&ip->i_mount->m_sb);
> +             spin_unlock(&ip->i_mount->m_sb_lock);
> +             xfs_mod_sb(tp,
> +                             XFS_SB_VERSIONNUM | XFS_SB_FEATURES2);

Weird formatting?

Otherwise this looks good to me.

<Prev in Thread] Current Thread [Next in Thread>