On Tue, 21 Oct 2008, Nick Piggin wrote:
> IO error handling in the core mm/fs still doesn't seem perfect, but with
> the recent round of patches and this one, it should be getting on the
> right track.
>
> I kind of get the feeling some people would rather forget about all this
> and brush it under the carpet. Hopefully I'm mistaken, but if anybody
> disagrees with my assertion that error handling, and data integrity
> semantics are first-class correctness issues, and therefore are more
> important than all other non-correctness problems... speak now and let's
> discuss that, please.
I agree that error handling is important. But careful: some
filesystems (NFS I know) don't set PG_error on async read errors, and
changing the semantics of ->readpage() from returning EIO to retrying
will potentially cause infinite loops. And no casual testing will
reveal those because peristent read errors are extremely rare.
So I think a better aproach would be to do
error = lock_page_killable(page);
if (unlikely(error))
goto readpage_error;
if (PageError(page) || !PageUptodate(page)) {
unlock_page(page);
shrink_readahead_size_eio(filp, ra);
error = -EIO;
goto readpage_error;
}
if (!page->mapping) {
unlock_page(page);
page_cache_release(page);
goto find_page;
}
etc...
Is there a case where retrying in case of !PageUptodate() makes any
sense?
Thanks,
Miklos
--
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
