On Sun, Jul 08, 2018 at 05:16:15PM +0200, Christoph Hellwig wrote: > On Tue, Jul 03, 2018 at 03:05:01PM -0700, Darrick J. Wong wrote: > > > So the buffer completion code clears the uptodate status of the buffer > > > on error. I assume that means the next read would replace the data we > > > failed to write with whatever was previously on disk. > > > > I've always found it a little weird that we basically throw away the > > newer page contents on error, but we shouldn't be changing the behavior > > in quite so subtle a way. > > > > Also, since we clear uptodate the next (buffered) write will reread the > > page contents. > > > > > I guess it's debatable whether that is the right thing to do in > > > general, but that seems like a higher level issue nonetheless (i.e., I > > > don't think we'd ever retry the writepage either?). > > > > AFAIK we don't retry failed writes unless userspace dirties the page. > > > > > So is there any reason not to do the analogous in the iomap completion > > > code? > > > > Will let Christoph answer that one. > > As far as I can tell the write path should never even touch the > uptodate bit, and the buffer head path is only doing so for very > old legacy reasons. I'd rather keep the iomap write path out of > the update bit manipulation business from the very beginning instead > of carry junk like this over. I'm more interested in preserving the expected behavior in the event of a write error as opposed to just copying whatever state the buffer head code sets. It looks to me that if the page itself isn't uptodate, we overwrite a block of that page and then the writepage fails, clearing the buffer uptodate status means that the next read would return what is on disk (not what was just written to the page). I'm not sure that's what happens if the page was already uptodate before the overwrite/writepage, however, I didn't notice anything that cleared page uptodate status on a writepage I/O error..? Brian
