xfs
[Top] [All Lists]

Re: [PATCH 1/3] xfs: push down inactive transaction mgmt for remote syml

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH 1/3] xfs: push down inactive transaction mgmt for remote symlinks
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Wed, 18 Sep 2013 14:06:26 -0400
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1379520960-22972-2-git-send-email-bfoster@xxxxxxxxxx>
References: <1379520960-22972-1-git-send-email-bfoster@xxxxxxxxxx> <1379520960-22972-2-git-send-email-bfoster@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130805 Thunderbird/17.0.8
On 09/18/2013 12:15 PM, Brian Foster wrote:
> Push down the transaction management for remote symlinks from
> xfs_inactive() down to xfs_inactive_symlink_rmt(). The latter is
> cleaned up to avoid transaction management intended for the
> calling context (i.e., trans duplication, reservation, item
> attachment).
> 
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_inode.c   | 15 ++++++------
>  fs/xfs/xfs_symlink.c | 64 
> ++++++++++++++++++----------------------------------
>  fs/xfs/xfs_symlink.h |  2 +-
>  3 files changed, 31 insertions(+), 50 deletions(-)
> 
...
> diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
> index f622a97..f85f6f2 100644
> --- a/fs/xfs/xfs_symlink.c
> +++ b/fs/xfs/xfs_symlink.c
> @@ -424,8 +424,7 @@ xfs_symlink(
>   */
...
>  
> @@ -563,16 +546,13 @@ xfs_inactive_symlink_rmt(
>   */
>  int
>  xfs_inactive_symlink(
> -     struct xfs_inode        *ip,
> -     struct xfs_trans        **tp)
> +     struct xfs_inode        *ip)
>  {
>       struct xfs_mount        *mp = ip->i_mount;
>       int                     pathlen;
>  
>       trace_xfs_inactive_symlink(ip);
>  
> -     ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> -

I just want to call out one thing here in case it isn't noticed on
review... the safety of this is something I was curious about.
Specifically, note that I've removed the inode locking from
xfs_inactive(), which previously covered xfs_inactive_symlink() (for
xfs_idata_realloc()), down into xfs_inactive_symlink_rmt().

My assumption was that this is currently ok since at this point we have
an inode with di_nlink == 0. If that's not accurate or not expected to
remain so after O_TMPFILE related work, I suppose I could pull the
locking back up into xfs_inactive_symlink().

Brian

>       if (XFS_FORCED_SHUTDOWN(mp))
>               return XFS_ERROR(EIO);
>  
> @@ -599,5 +579,5 @@ xfs_inactive_symlink(
>       }
>  
>       /* remove the remote symlink */
> -     return xfs_inactive_symlink_rmt(ip, tp);
> +     return xfs_inactive_symlink_rmt(ip);
>  }
> diff --git a/fs/xfs/xfs_symlink.h b/fs/xfs/xfs_symlink.h
> index 99338ba..e75245d 100644
> --- a/fs/xfs/xfs_symlink.h
> +++ b/fs/xfs/xfs_symlink.h
> @@ -22,6 +22,6 @@
>  int xfs_symlink(struct xfs_inode *dp, struct xfs_name *link_name,
>               const char *target_path, umode_t mode, struct xfs_inode **ipp);
>  int xfs_readlink(struct xfs_inode *ip, char *link);
> -int xfs_inactive_symlink(struct xfs_inode *ip, struct xfs_trans **tpp);
> +int xfs_inactive_symlink(struct xfs_inode *ip);
>  
>  #endif /* __XFS_SYMLINK_H */
> 

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