lkml.org 
[lkml]   [1999]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
Subjectreview of i_op->readpage error handling (fwd)
i sent this yesterday afternoon, but apparently it didn't get through.

- Chuck Lever
--
corporate: <chuckl@netscape.com>
personal: <chucklever@netscape.net> or <cel@monkey.org>

The Linux Scalability project:
http://www.citi.umich.edu/projects/linux-scalability/

---------- Forwarded message ----------
Date: Mon, 23 Aug 1999 16:51:22 -0400 (EDT)
From: Chuck Lever <cel@monkey.org>
To: linux-kernel@vger.rutgers.edu
Subject: review of i_op->readpage error handling

i've done a simple survey of the i_op->readpage method to see how each
file system handles errors in terms of page locking and properly setting
the PG_Error and PG_Uptodate flags. all comments welcome!

there are six implementations i could find:

block_read_full_page
coda_readpage
nfs_readpage
qnx4_readpage
romfs_readpage
smb_readpage


the upshot of this is:
1. PG_Error is not set reliably. either it should be fixed in every file
system, or no-one should use it.

2. synchronous readpage's always unlock the page on return, whether
successful or an error occurs.

3. asynch readpage's can unlock on error, but don't always.

4. mainline (that is, non-error) logic generally seems to handle page
locking correctly.

the following recommendations are based on the behavior in 2.3.15-pre2,
where filemap_nopage assumes that a page is always unlocked if an error
occurs. my own opinion is that it would be easier everywhere if readpage
leaves a page locked (essentially in the same state as when readpage was
called) on error.


block_read_full_page
review:
never returns an error, so it doesn't touch PG_Error.
if all buffers for a page are up to date, it will set
PG_Uptdate, unlock the page, and return.
otherwise, it schedules I/O.
so, on return, either:
the page is locked, not uptodate, and not error, or
the page is unlocked, uptodate, and not error
return code is always zero.
recommendation:
perhaps it should clear PG_Error before starting.


coda_readpage
review:
can return ENXIO -- doesn't unlock the page or set PG_Error
can return 0, after invoking block_read_full_page
recommendation:
perhaps it should return code from block_read_full_page
instead of zero. if ENXIO is returned, it should set PG_Error,
and unlock the page.


nfs_readpage
review:
oy.
if a page write-back error occurs, it unlocks the page,
but doesn't set PG_Error.
if it decides to try async read, it leaves the page
locked, and the callback will set PG_Error/PG_Uptodate,
and unlock the page. if an RPC error occurs, the page
is left locked.
if it decides to try sync read, it unlocks the page,
but PG_Error is not set if an error occurs.
recommendation:
page write-back error handling should set PG_Error.
nfs_readpage uses PG_Error to determine whether to use
sync or async read -- if filemap_nopage clears PG_Error
during retry, that causes the retry to be async, which is
not what we want. sync read should set PG_Error properly.
async read should unlock the page if an RPC error occurs.
otherwise, this leaks locked pages when network
connectivity is poor.


qnx4_readpage
review:
doesn't use lock_page or get_page, uses old bread interface.
synchronous read only; it sets PG_Error and PG_Uptodate
correctly, and unlocks the page when it's done.
recommendation:
this file system hasn't been upgraded to use the page
cache yet. should probably check value of bits
before simply clearing them.


romfs_readpage
review:
synchronous operation, always unlocks page when done.
sets PG_Error and PG_Uptodate correctly.
recommendation:
no change.


smb_readpage
review:
synchronous operation, always unlocks page when done.
clears PG_Error, but never sets it. sets PG_Uptodate
when successful.
recommendation:
should probably set PG_Error if an error occurs.


- Chuck Lever
--
corporate: <chuckl@netscape.com>
personal: <chucklever@netscape.net> or <cel@monkey.org>

The Linux Scalability project:
http://www.citi.umich.edu/projects/linux-scalability/



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:53    [W:0.028 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site