On Thu, Apr 19, 2018 at 01:14:04PM -0400, Jeff Layton wrote: > On Thu, 2018-04-19 at 10:44 +1000, Dave Chinner wrote: > > On Wed, Apr 18, 2018 at 12:00:10PM -0400, Jeff Layton wrote: > > > > > 4) syncfs doesn't currently report an error when a single inode fails > > > > > writeback, only when syncing out the block device. Should it report > > > > > errors in that case as well? > > > > > > > > Yes. > > > > > > I have a small patch that implements this that I posted a few days ago. > > > I meant to mark it as an RFC, but didn't, fwiw. I'm not convinced that > > > it's the right approach. > > > > > > Instead of growing struct file to accommodate a second errseq cursor, > > > this only implements that behavior when the file is opened with O_PATH > > > (as we know that that will have the fsync op set to NULL). Maybe we can > > > do this better somehow though. > > > > No idea whether this is possible, or even a good idea, but could we > > just have syncfs() create a temporary struct file for the duration > > of the syncfs call, so it works on any fd passed in? (sorta like an > > internal dupfd() call?) > > > > No, we need something that will persist the errseq_t cursor between > syncfs calls. > > If we did what you're suggesting, once your temporary file goes away, > you'd lose your place in the error stream and you'd end up reporting > the same errors more than once on subsequent calls to syncfs. Which, IMO, is correct behaviour. If there is a persistent data writeback errors, then syncfs() should report it *every time it is called* because that syncfs() call did not result in the filesystem being fully committed to stable storage. I don't care whether the error has been reported before - the context of syncfs() is "commit the entire filesystem to stable storage". If any IO failed then we have not acheived what the application asked us to do and so we should be returning an error on every call that sees a data writeback error. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
