On Tue, 2011-05-31 at 14:20 +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> XFs inodes have several per-lifetime state flags that determine heaviour of the
> inode. These state flags are not reset when an inode is reallocated and reused
> from the reclaimable state.
>
> This can lead to specualtive 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.
>
> Clear the state flags when the inode is recycled to avoid these problems.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Other than simple typo's, this looks good to me.
Comment below.
Reviewed-by: Alex Elder <aelder@xxxxxxx>
> ---
> fs/xfs/xfs_iget.c | 12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/fs/xfs/xfs_iget.c b/fs/xfs/xfs_iget.c
> index cb9b6d1..36467f1 100644
> --- a/fs/xfs/xfs_iget.c
> +++ b/fs/xfs/xfs_iget.c
> @@ -262,7 +262,17 @@ xfs_iget_cache_hit(
>
> spin_lock(&pag->pag_ici_lock);
> spin_lock(&ip->i_flags_lock);
> - ip->i_flags &= ~(XFS_IRECLAIMABLE | XFS_IRECLAIM);
> +
> + /*
> + * Clear the relcaim and per-lifetime state flags as we are now
reclaim
> + * effectively a new inode and so we need to reset to the
> + * initial state.
> + *
> + * XXX(dgc): should the XFS_ISTALE flag only be cleared here?
> + */
> + ip->i_flags &= ~(XFS_IRECLAIMABLE | XFS_IRECLAIM |
> + XFS_IDIRTY_RELEASE | XFS_ITRUNCATED |
> + XFS_IFILESTREAM);
> ip->i_flags |= XFS_INEW;
If you clear XFS_ISTALE here you could simply re-phrase the
above comment to emphasize the "reset to initial state" and
change it to a simple assignment:
ip->i_flags = XFS_INEW;
(Though that makes clearing the rest of the flags
implicit, which I think is undesirable from the point
of view of code searching. XFS_IDIRTY_RELEASE is
already cleared only implicitly.)
Otherwise I was going to suggest you define a symbol
representing the subset of XFS inode flags that are
per-lifetime state flags, so if another one gets
invented along the way it could get added to the set.
> __xfs_inode_clear_reclaim_tag(mp, pag, ip);
> inode->i_state = I_NEW;
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
