Re: [PATCH 4/5] xfs: use xfs_ilock_map_shared in xfs_attr_get

Dave Chinner
2013-12-05T20:59:19Z

On Thu, Dec 05, 2013 at 07:58:34AM -0800, Christoph Hellwig wrote:
> We might not have read in the extent list at this point, so make sure we
> take the ilock exclusively if we have to do so.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> diff --git a/fs/xfs/xfs_attr.c b/fs/xfs/xfs_attr.c
> index b861270..5343034 100644
> --- a/fs/xfs/xfs_attr.c
> +++ b/fs/xfs/xfs_attr.c
> @@ -164,6 +164,7 @@ xfs_attr_get(
> {
> int error;
> struct xfs_name xname;
> + uint lock_mode;
>
> XFS_STATS_INC(xs_attr_get);
>
> @@ -174,9 +175,9 @@ xfs_attr_get(
> if (error)
> return error;
>
> - xfs_ilock(ip, XFS_ILOCK_SHARED);
> + lock_mode = xfs_ilock_map_shared(ip);
> error = xfs_attr_get_int(ip, &xname, value, valuelenp, flags);
> - xfs_iunlock(ip, XFS_ILOCK_SHARED);
> + xfs_iunlock_map_shared(ip, lock_mode);
> return(error);
> }

I think the locking here should be moved inside xfs_attr_get_int()
so that it uses the same locking pattern as xfs_attr_set() and
xfs_attr_remove().

Also, xfs_attr_list() needs this treatment (the attr version of
readdir) as well (and it has the locking inside xfs_attr_list_int(),
too ;).

It looks like xfs_readlink needs fixing, too.

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com