Re: [PATCH 4/17] kill the v_flag member in struct bhv_vnode

[David Chinner <dgc@xxxxxxx>]

On Thu, Aug 23, 2007 at 09:38:07PM +0200, Christoph Hellwig wrote:
> 
> All flags previous handled at the vnode level are not in the xfs_inode
> where we already have a flags mechanisms and free bits for flags
> previously in the vnode.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
....
> Index: linux-2.6-xfs/fs/xfs/xfs_vnodeops.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_vnodeops.c  2007-08-13 18:00:49.000000000 
> +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_vnodeops.c       2007-08-13 18:00:51.000000000 
> +0200
....
> @@ -1536,7 +1538,13 @@ xfs_release(
>                * significantly reducing the time window where we'd otherwise
>                * be exposed to that problem.
>                */
> -             if (VUNTRUNCATE(vp) && VN_DIRTY(vp) && ip->i_delayed_blks > 0)
> +             spin_lock(&ip->i_flags_lock);
> +             truncated = __xfs_iflags_test(ip, XFS_ITRUNCATED);
> +             if (truncated)
> +                     ip->i_flags &= ~XFS_ITRUNCATED;
> +             spin_unlock(&ip->i_flags_lock);
> +
> +             if (truncated && VN_DIRTY(vp) && ip->i_delayed_blks > 0)
>                       xfs_flush_pages(ip, 0, -1, XFS_B_ASYNC, FI_NONE);
>       }

This is kind of icky - doing an open coded flag clear instead of wrapping
it in a xfs_iflags_test_and_clear() type operation. Something like:

static inline int
xfs_iflags_test_and_clear(xfs_inode_t *ip, unsigned short flags)
{
        int     ret;

        spin_lock(&ip->i_flags_lock);
        ret = ip->i_flags & flags;
        if (ret)
                ip->i_flags &= ~flags;
        spin_unlock(&ip->i_flags_lock);
        return ret;
}

And then the code can become:

        if (xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED) &&
            VN_DIRTY(vp) && ip->i_delayed_blks > 0))
                xfs_flush_pages(ip, 0, -1, XFS_B_ASYNC, FI_NONE);

FWIW, for changes to this series I think incremental patches would
probably be the easiest way to handle it.

Cheers,

Dave.

-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group