lkml.org 
[lkml]   [2000]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Nbd is broken


On Sat, 4 Mar 2000, Andrea Arcangeli wrote:

> I also know why probably you got in troubles. As Al Viro pointed out to me
> lots of blockdevices wasn't ready to handle merged requests. Lots of them
> (the one with the head active) could be just ready if end_request wouldn't
> be so badly designed :(. end_request acts correctly when there's been an
> I/O error. If no IO error happened, then end_request forgets to advance
> the ->sector field and to decrease nr_sectors in the request and so it
> corrupts everything if you don't add the dirty hack before calling it.

AFAICS the original logics looked so:
a) you update ->sector and ->nr_sectors in the same place where
you do the IO. E.g. in the interrupt handler.
b) normally you are supposed to have those fields properly updated
when the segment is done with.
c) if you got an error you _skip_ the rest of segment. Thus the
code in end_request().
d) if you got more segments - just reinsert the thing into queue.

Now, it worked fine for almost everything. Two exceptions were loop and
nbd (monkied up from loop). Here the thing didn't do (a) (since there was
no obvious place for that? Hell knows) and was explicitly protected from
multi-segment requests. When that protection went away the shit had hit
the fan. Badly. Especially since it got (d), so it overwrote the first
segment with latter ones instead of skipping them. It became really bad
when plugging logics changed (couple of releases after the first change),
because now requests got merged much more often.

Notice that everything except loop and nbd kept working (OK, working wrt
that stuff) since they used to update ->sector and ->nr_sectors inside the
IO code from the very beginning. I've fixed loop adding (a) into
do_lo_request(). Nbd remained b0rken, apparently.

BTW, nbd really ought to do request merging in decent way - here we have
the equivalent of scatter-gather, so using it is worth trying. What about
using sock_readv() and sock_writev()?

> end_request should really hide the merging trasparently. Fixing that is
> trivial but all the blockdevices that was wokarounding that design bug
> will have to be ported to the new semantic and that's some time not
> obvious. It's certainly a cleanup to do in the long term IMHO.

I'm not sure. Updating the ->sector and ->nr_sectors at the actual IO time
looks perfectly sane to me. Code in end_request() is rather a "OK, we got
an error, so let's skip the rest of this segment" thing.
Al


-
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:56    [W:0.063 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site