xfs
[Top] [All Lists]

Re: [PATCH 1/3] xfs: reset inode per-lifetime state when recycling it

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/3] xfs: reset inode per-lifetime state when recycling it
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 23 Jun 2011 17:21:15 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1308792901-22880-2-git-send-email-david@xxxxxxxxxxxxx>
References: <1308792901-22880-1-git-send-email-david@xxxxxxxxxxxxx> <1308792901-22880-2-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Jun 23, 2011 at 11:34:59AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> XFS inodes has several per-lifetime state fields that determine the
> behaviour of the inode. These state fields are not all reset when an
> inode is reused from the reclaimable state.
> 
> This can lead to unexpected behaviour of the new inode such as
> speculative preallocation not being truncated away in the expected
> manner for local files until the inode is subsequently truncated,
> freed or cycles out of the cache. It can also lead to an inode being
> considered to be a filestream inode or having been truncated when
> that is not the case.
> 
> Rework the reinitialisation of the inode when it is recycled to
> ensure that it is pristine before it is reused. While there, also
> fix the resetting of state flags in the recycling error paths so the
> inode does not become unreclaimable.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_iget.c  |   13 +++++++++----
>  fs/xfs/xfs_inode.h |   10 ++++++++++
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_iget.c b/fs/xfs/xfs_iget.c
> index cb9b6d1..3631783 100644
> --- a/fs/xfs/xfs_iget.c
> +++ b/fs/xfs/xfs_iget.c
> @@ -253,16 +253,21 @@ xfs_iget_cache_hit(
>                       rcu_read_lock();
>                       spin_lock(&ip->i_flags_lock);
>  
> -                     ip->i_flags &= ~XFS_INEW;
> -                     ip->i_flags |= XFS_IRECLAIMABLE;
> -                     __xfs_inode_set_reclaim_tag(pag, ip);
> +                     ip->i_flags &= ~(XFS_INEW | XFS_IRECLAIM);
> +                     ASSERT(ip->i_flags & XFS_IRECLAIMABLE);

Looking over this again XFS_INEW can't be set here, as we return early
a few lines above.

Otherwise looks good,

Reviewed-by: Christoph Hellwig <hch@xxxxxx>

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