xfs
[Top] [All Lists]

Re: [PATCH 05/16] xfs: don't truncate prealloc from frequently accessed

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 05/16] xfs: don't truncate prealloc from frequently accessed inodes
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 9 Nov 2010 10:56:50 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20101108113645.GA16418@xxxxxxxxxxxxx>
References: <1289206519-18377-1-git-send-email-david@xxxxxxxxxxxxx> <1289206519-18377-6-git-send-email-david@xxxxxxxxxxxxx> <20101108113645.GA16418@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Mon, Nov 08, 2010 at 06:36:45AM -0500, Christoph Hellwig wrote:
> I'd be much more happy about fixing this properly in nfsd. 

So would I, but we've been saying that for years and it still ain't
done....

> But I guess
> the fix is simple enough that we can put it into XFS for now.  Any
> reason you use up a whole int in the inode instead of using a flag in
> i_flags?

I wasn't sure how many dirty releases we wanted before triggering
the change of behaviour. A single dirty release seems to be fine in
my testing so far, and if that continues then I think that , like
you suggest, changing it to a flag in i_flags is the right thing to
do.

> > -
> > -           ASSERT(ip->i_delayed_blks == 0);
> > +           /*
> > +            * even after flushing the inode, there can still be delalloc
> > +            * blocks on the inode beyond EOF due to speculative
> > +            * preallocation. These are not removed until the release
> > +            * function is called or the inode is inactivated. Hence we
> > +            * cannot assert here that ip->i_delayed_blks == 0.
> > +            */
> 
> Shouldn't this be in a separate patch given that we can fail the flush
> due to iolock contention?  I think this and the swapext fix are .37
> material in fact.

Agreed. I should have noted in the series preamble that I thought these
probably need splitting out into separate bug fixing patches rather
than being lumped in here.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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