xfs
[Top] [All Lists]

Re: [PATCH 2/5] xfs: fold xfs_attr_get_int into xfs_attr_get

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 2/5] xfs: fold xfs_attr_get_int into xfs_attr_get
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 5 May 2014 16:21:14 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1399130415-5382-3-git-send-email-hch@xxxxxx>
References: <1399130415-5382-1-git-send-email-hch@xxxxxx> <1399130415-5382-3-git-send-email-hch@xxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Sat, May 03, 2014 at 05:20:12PM +0200, Christoph Hellwig wrote:
> This allows doing an unlocked check if an attr for is present at all and
> slightly reduce the lock hold time if we actually do an attr get.
> 
> Plus various minor style fixes to the new xfs_attr_get.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>

>  fs/xfs/xfs_attr.c |   79 
> ++++++++++++++++++-----------------------------------
>  1 file changed, 27 insertions(+), 52 deletions(-)
> 
> diff --git a/fs/xfs/xfs_attr.c b/fs/xfs/xfs_attr.c
> index eb3ae8f..01f2267 100644
> --- a/fs/xfs/xfs_attr.c
> +++ b/fs/xfs/xfs_attr.c
> @@ -106,26 +106,34 @@ xfs_inode_hasattr(
>   * Overall external interface routines.
>   *========================================================================*/
>  
> -STATIC int
> -xfs_attr_get_int(
> +int
> +xfs_attr_get(
>       struct xfs_inode        *ip,
> -     struct xfs_name         *name,
> +     const unsigned char     *name,
>       unsigned char           *value,
>       int                     *valuelenp,
>       int                     flags)
>  {
> -     xfs_da_args_t   args;
> -     int             error;
> +     struct xfs_da_args      args;
> +     struct xfs_name         xname;
> +     uint                    lock_mode;
> +     int                     error;
> +
> +     XFS_STATS_INC(xs_attr_get);
> +
> +     if (XFS_FORCED_SHUTDOWN(ip->i_mount))
> +             return EIO;
>  
>       if (!xfs_inode_hasattr(ip))
>               return ENOATTR;
>  
> -     /*
> -      * Fill in the arg structure for this request.
> -      */
> -     memset((char *)&args, 0, sizeof(args));
> -     args.name = name->name;
> -     args.namelen = name->len;
> +     error = xfs_attr_name_to_xname(&xname, name);
> +     if (error)
> +             return error;
> +
> +     memset(&args, 0, sizeof(args));
> +     args.name = xname.name;
> +     args.namelen = xname.len;
>       args.value = value;
>       args.valuelen = *valuelenp;
>       args.flags = flags;
> @@ -133,52 +141,19 @@ xfs_attr_get_int(
>       args.dp = ip;
>       args.whichfork = XFS_ATTR_FORK;
>  
> -     /*
> -      * Decide on what work routines to call based on the inode size.
> -      */
> -     if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
> +     lock_mode = xfs_ilock_attr_map_shared(ip);
> +     if (!xfs_inode_hasattr(ip))
> +             error = ENOATTR;
> +     else if (ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL)
>               error = xfs_attr_shortform_getvalue(&args);
> -     } else if (xfs_bmap_one_block(ip, XFS_ATTR_FORK)) {
> +     else if (xfs_bmap_one_block(ip, XFS_ATTR_FORK))
>               error = xfs_attr_leaf_get(&args);
> -     } else {
> +     else
>               error = xfs_attr_node_get(&args);
> -     }
> +     xfs_iunlock(ip, lock_mode);
>  
> -     /*
> -      * Return the number of bytes in the value to the caller.
> -      */
>       *valuelenp = args.valuelen;
> -
> -     if (error == EEXIST)
> -             error = 0;
> -     return(error);
> -}
> -
> -int
> -xfs_attr_get(
> -     xfs_inode_t     *ip,
> -     const unsigned char *name,
> -     unsigned char   *value,
> -     int             *valuelenp,
> -     int             flags)
> -{
> -     int             error;
> -     struct xfs_name xname;
> -     uint            lock_mode;
> -
> -     XFS_STATS_INC(xs_attr_get);
> -
> -     if (XFS_FORCED_SHUTDOWN(ip->i_mount))
> -             return(EIO);
> -
> -     error = xfs_attr_name_to_xname(&xname, name);
> -     if (error)
> -             return error;
> -
> -     lock_mode = xfs_ilock_attr_map_shared(ip);
> -     error = xfs_attr_get_int(ip, &xname, value, valuelenp, flags);
> -     xfs_iunlock(ip, lock_mode);
> -     return(error);
> +     return error == EEXIST ? 0 : error;
>  }
>  
>  /*
> -- 
> 1.7.10.4
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

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