On Fri, 24 Jul 2009, Andi Kleen wrote: > > The part I don't understand is what actually happens to pages after the > > error flag set. They're still uptodate, but no longer dirty? And can be > > overwritten/redirtied? There's also an error flag on the address_space. > > Are there any guidelines as far as which should be used? > > Ideally both. The Page error flag prevents the data from being > consumed and the address space error flag makes sure errors are > getting reported on fsync()/close() etc. Also AS error is useful when > you don't have a clear page to assign the error to, e.g. if you > get an error indication that's not tied to a read operation. > > BTW the upcoming hwpoison code can set such errors asynchronously > under you. I looked through the various callers, and these look correct: readpages -- return error code. Doesn't matter, we'll get a call to ->readpage later. writepages (async) -- return error code if we fail during request setup, or call mapping_set_error() on completion. If wait_on_page_writeback_range() is any indication, that's correct. These I'm unsure about: readpage -- currently calls SetPageError(page) _and_ returns error code. I don't see any page error checks outside the writeout paths, so a simple return value looks sufficient for a synchronous read. OTOH mpage_end_io_read() and end_buffer_async_read() both do SetPageError() for async readpage (although I don't see where that is checked). writepage -- SetPageError(page) _and_ return error code. I see that wait_on_page_writeback_range() and write_one_page() check the page error after wait_on_page_writeback(), so that looks correct for an async writepage. My writepage() is currently synchronous (i.e. does wait_on_page_writeback itself), so worry the page error bit is superfluous? I should make it async, regardless. Also, mpage_end_io_write() sets both PageError and AS_EIO on the mapping, so an async writepage should also set the mapping bit in the async case. I'm guessing on the (??) items, but this at least looks consistent (as far as the sync vs async implementations go): readpage (sync) -- return error and SetPageError (??) readpage (async) -- SetPageError readpages -- doesn't matter writepage (sync) -- return error and SetPageError (__writepage() sets mapping error) (??) writepage (async) -- SetPageError AND mapping_set_error writepages (sync) -- return error AND mapping_set_error (??) writepages (async) -- mapping_set_error Does that sound right? If so, I can update Documentation/filesystems/vfs.txt appropriately. Thanks- sage -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
