xfs
[Top] [All Lists]

Re: [PATCH] Don't reset di_format in xfs_ifree()

To: Lachlan McIlroy <lachlan@xxxxxxx>
Subject: Re: [PATCH] Don't reset di_format in xfs_ifree()
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 11 Feb 2009 20:20:20 +1100
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <49921B3E.8040406@xxxxxxx>
Mail-followup-to: Lachlan McIlroy <lachlan@xxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
References: <49921B3E.8040406@xxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Wed, Feb 11, 2009 at 11:26:38AM +1100, Lachlan McIlroy wrote:
> I hit a panic while flushing a reclaimed inode that is fairly
> reproducible under load.
>
> In xfs_iflush_fork() we're led to believe that there are extents
> on this inode but there aren't any.  Actually the inode was a
> directory.  I added some debugging to xfs_ifree() and found
> that di_format was XFS_DINODE_FMT_LOCAL and got reset to
> XFS_DINODE_FMT_EXTENTS and this has confused the code in
> xfs_iflush_fork().

Wow. I wonder why we've never seen this before - it's not a new
problem AFAICT.

A freed inode is supposed to have both forks in extent format
with zero extents - it means the fork is empty. Changing it
to local format means that it is not in the expected state
for a subsequent create.

I think the problem may be that the size of the fork has not been
reset to zero, not that format has been changed. If it was in local
format, the truncates prior to freeing would not have done anything
and the size of the data/attr fork would still be non-zero.  Hence
if the fork is then changed to extent format, xfs_iextents_copy()
will be triggered from xfs_iflush_fork() and you'd see something
like the confusion you are seeing.

Hence I think we should be ensuring the fork size is set to zero for
both the attr/data fork when the format is changed, not removing
the change of type....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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