[Top] [All Lists]

Re: [PATCH 1/5] [PATCH] xfs: fix attr2 vs large data fork assert

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/5] [PATCH] xfs: fix attr2 vs large data fork assert
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 17 Nov 2011 02:30:04 -0500
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20111116231517.GA7046@dastard>
References: <20111115201407.038216766@xxxxxxxxxxxxxxxxxxxxxx> <20111115201426.498870090@xxxxxxxxxxxxxxxxxxxxxx> <20111116231517.GA7046@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Nov 17, 2011 at 10:15:17AM +1100, Dave Chinner wrote:
> On Tue, Nov 15, 2011 at 03:14:08PM -0500, Christoph Hellwig wrote:
> > With Dmitry fsstress updates I've seen very reproducible crashes in
> > xfs_attr_shortform_remove because xfs_attr_shortform_bytesfit claims that
> > the attributes would not fit inline into the inode after removing an
> > attribute.  It turns out that we were operating on an inode with lots
> > of delalloc extents, and thus an if_bytes values for the data fork that
> > is larger than biggest possible on-disk storage for it which utterly
> > confuses the code near the end of xfs_attr_shortform_bytesfit.
> We have a test that stresses allocated extents vs attributes in the
> xfs_fsr swapext test (227), but that does not take into account
> delalloc extents. It sounds like it would be relatively easy to
> write a regression test for this particular case - create a file
> with a bunch of attributes, then create a number of delalloc data
> extents, then remove the attributes to trigger the condition in
> xfs_attr_shortform_remove()....

Test 117 with Dmitries new fsstress changes hit it 100% reliably

        xfstests: freeze fsstress options for 117'th

I was planning on adding a copy of the test using an explicit
combination of fsstress seeds that reproduce the issue.

> While you are touching that function, can you fix all the whitespace
> damage as well? (lots of trailing whitespace). There's a couple of
> typos I noticed in your changes (below), but otherwise looks good.

I'll fix the tpos and whitespace issues.

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