Messages in this thread | | | Date | Mon, 6 Mar 2000 03:29:08 -0500 (EST) | From | Alexander Viro <> | Subject | Re: 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/
| |