On Fri, Apr 13, 2018 at 09:18:56AM -0400, Jeff Layton wrote: > On Fri, 2018-04-13 at 08:44 +1000, Dave Chinner wrote: > > To save you looking, XFS will trash the page contents completely on > > a filesystem level ->writepage error. It doesn't mark them "clean", > > doesn't attempt to redirty and rewrite them - it clears the uptodate > > state and may invalidate it completely. IOWs, the data written > > "sucessfully" to the cached page is now gone. It will be re-read > > from disk on the next read() call, in direct violation of the above > > POSIX requirements. > > > > This is my point: we've done that in XFS knowing that we violate > > POSIX specifications in this specific corner case - it's the lesser > > of many evils we have to chose between. Hence if we chose to encode > > that behaviour as the general writeback IO error handling algorithm, > > then it needs to done with the knowledge it is a specification > > violation. Not to mention be documented as a POSIX violation in the > > various relevant man pages and that this is how all filesystems will > > behave on async writeback error..... > > > > Got it, thanks. > > Yes, I think we ought to probably do the same thing globally. It's nice > to know that xfs has already been doing this. That makes me feel better > about making this behavior the gold standard for Linux filesystems. > > So to summarize, at this point in the discussion, I think we want to > consider doing the following: > > * better reporting from syncfs (report an error when even one inode > failed to be written back since last syncfs call). We'll probably > implement this via a per-sb errseq_t in some fashion, though there are > some implementation issues to work out. > > * invalidate or clear uptodate flag on pages that experience writeback > errors, across filesystems. Encourage this as standard behavior for > filesystems and maybe add helpers to make it easier to do this. > > Did I miss anything? Would that be enough to help the Pg usecase? > > I don't see us ever being able to reasonably support its current > expectation that writeback errors will be seen on fd's that were opened > after the error occurred. That's a really thorny problem from an object > lifetime perspective. I think we can do better than XFS is currently doing (but I agree that we should have the same behaviour across all Linux filesystems!) 1. If we get an error while wbc->for_background is true, we should not clear uptodate on the page, rather SetPageError and SetPageDirty. 2. Background writebacks should skip pages which are PageError. 3. for_sync writebacks should attempt one last write. Maybe it'll succeed this time. If it does, just ClearPageError. If not, we have somebody to report this writeback error to, and ClearPageUptodate. I think kupdate writes are the same as for_background writes. for_reclaim is tougher. I don't want to see us getting into OOM because we're hanging onto stale data, but we don't necessarily have an open fd to report the error on. I think I'm leaning towards behaving the same for for_reclaim as for_sync, but this is probably a subject on which reasonable people can disagree. And this logic all needs to be on one place, although invoked from each filesystem.
