[Top] [All Lists]

Re: [PATCH 6/9] xfs: kill xfs_bioerror_relse

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 6/9] xfs: kill xfs_bioerror_relse
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sat, 30 Aug 2014 10:05:47 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20140829182610.GA28670@xxxxxxxxxxxxx>
References: <1408084747-4540-1-git-send-email-david@xxxxxxxxxxxxx> <1408084747-4540-7-git-send-email-david@xxxxxxxxxxxxx> <20140829003257.GF17502@xxxxxxxxxxxxx> <20140829011236.GB20518@dastard> <20140829182610.GA28670@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Fri, Aug 29, 2014 at 11:26:10AM -0700, Christoph Hellwig wrote:
> On Fri, Aug 29, 2014 at 11:12:36AM +1000, Dave Chinner wrote:
> > > This is a large change of behavior as it doesn't hit the error
> > > path after the xfs_buf_iowait anymore.  While I don't think that
> > > that path was entirely correct this version seems to be even less so
> > > by not releasing the buffer reference or forcing the shutdown.
> > 
> > The IO is synchronous, so the previous behaviour did not release
> > the buffer here. But, yes, it needs to because we're not running the
> > io wait on it anymore. And this happens only during a shutdown, so i
> > don't see any need to trigger a shutdown ;)
> > 
> > As it is, I think this gets properly fixed by the next patch....
> Do you have any scenario that might actually exercise this path?  I
> enabled xfs_trans_read_buf_io trace events and run xfstests as well
> as a few other workloads and couldn't hit it at all.  And when you
> think of it: when would be do a trans_get_buf, then not actually
> updating it with data and then recurse into a trans_read_buf in
> the same transaction?

*nod*. This path has been there is the Irix days, and I suspect that
it was there to handle some wierd corner case to do with async
buffer reads which IIRC, could once be done through this code...

> Maybe it's just time do a bit more of an audit and put this whole
> code path to rest.

Perhaps so.


Dave Chinner

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