Messages in this thread | | | Date | Mon, 6 Mar 2000 15:17:27 +0100 (CET) | From | Andrea Arcangeli <> | Subject | Re: Nbd is broken |
| |
On Mon, 6 Mar 2000, Alexander Viro wrote:
>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.
This make no sense IMHO. Why don't you also update the rest? Without updating the rest only updating such two fields is useless. You lose synchrony with the fields updating them in different places.
head_active drivers could have no knowledge at all about merging an end_request could do all the work as it does when there's an error.
> c) if you got an error you _skip_ the rest of segment. Thus the >code in end_request().
No, if there's an error you don't skip the rest of segment because end_that_request_first returns zero if there are more bhs to process in the request. This also make sense since if there's a defective sector it doesn't mean the following sector is defective too.
> d) if you got more segments - just reinsert the thing into queue.
That is a special implementation case of loop. Normal headactive devices left the req into the queue. Smart devices like scsi removes it forever and never reinserts it.
>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.
That problem has anything to do with plugging logic. If the request_fn blocks dropping the lock even if you avoid plugging you can have merged requests in the queue.
If we want to provide backwards compatibility we must add a "merging" field in the blk_dev_struct and skip the merging path if it's set to zero.
>> 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.
It's very insane IMHO (see first paragraph).
Andrea
- 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/
| |