[ENBD] ENBD on 2.4.x

[Peter T. Breuer]

"Jon Arney wrote:"
> > Are you mounting the fs sync?
> No, I'm not, but I'll give it a shot.

You need to do that on all drivers which have some kind of backup usage
as a question of design. But on a driver which has a userspace
component, you need to also do it to avoid deadlocking buffers against
userspace - nobody really quite understands exactly where the problem is
(but I believe I do :-). I believe it's simply: full VFS, need to
flush, call client to take buffer contents, wups, client needs some
buffers for something (what?).

So we don't let things get to that point. Keep VFS empty, eh.

> > Probably vfs is full. File bigger than memory, etc. Is the fs mounted
> > sync? If not, mount it sync. If that doesn't help, load the module
> > with
> > sync_intvl=1. That will flush VFS frequently, before it can ever fill
> > up (well, small ram, fast cpu will still let it block, but we hope
> > not).
> When I say "Blocks indefinitely", I mean that it blocks and NEVER
> flushes. It's dead, jim! :)

The technical term is "deadlock", mr. sulu. You really can't get out of
it. I suspect that if you can manage to echo -n 0 > /proc/nbdinfo,
you may manage to  squeek out before the whole system blows.

> As to the "merge_requests=0" option, I suppose I should have
> used that before jumping to conclusions, but you should see
> the VAST changes between 2.4.0 and 2.4.1 in the 'elevator.c' code.
> It just makes me a little scared that whatever's there isn't stable,

I'm just asking you to check, since you have the code handy. You can do
some experiments rather more easily than I can at this moment.

> so I'd rather not try to muck with it until it is.  I agree that we
> should try to clean up the merge code and I'll try your
> suggestion:
>   int nbd = MINOR(bh->b_rdev) >> NBD_SHIFT;
>   if (bh->b_size != blksize_size[NBD_MAJOR][nbd])
>     NBD_ALERT ...

That was just a passing thought. Really, I don't see what's wrong with
the nbd_fron/back_merge functions returning 0, which they will do if
merge_requests=0 is set. Is this not the right answer anymore?
What do the default kernel functions (presumably in ll_rw_blk.c) return?

> and see what happens, but I have a sneaking suspicion that we should
> wait until like 2.4.5 or something before we believe that the elevator
> stuff is stable.
> Also, I stand corrected on the 'd[ei]spatch' spelling.

Nice that I win at least the odd argument. To me "dispatch" has
slightly the connotation of "getting rid of it", while "despatch" seems
to me to tell me that I'm sending the stuff to a destination. Probably
no basis for that, though!

I believe that with the kernel defaults, and the device permitting
plugging, that the kernel will always merge requests as they stack up
on the plugged queue. Is that not the case? Wrapping the kernel
functions and setting merge_requests=0 simply stops the merges.
How could that be affected by the elevator code?


Peter