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. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
