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

[Lachlan McIlroy <lachlan@sgi.com>]

Dave Chinner wrote:
> 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....

Yes, I agree.  I just don't have the time to hunt it down.  I see
there's a call to xfs_idestroy_fork() in xfs_ireclaim() for directories
but xfs_ireclaim() gets called after xfs_iflush() in xfs_reclaim_inode().
Might also need something like:

@@ -2445,6 +2447,7 @@ xfs_idestroy_fork(
                         kmem_free(ifp->if_u1.if_data);
                         ifp->if_u1.if_data = NULL;
                         ifp->if_real_bytes = 0;
+                       ifp->if_bytes = 0;
                 }
         } else if ((ifp->if_flags & XFS_IFEXTENTS) &&
                    ((ifp->if_flags & XFS_IFEXTIREC) ||

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs