[ENBD] 2.4.32

[Peter T. Breuer]

"Also sprach Anders Blomdell:"
> Minor note:
> 
> enbd_clr_kernel_queue:
> 
> shouldn't "schedule_timeout(1);" be preceded by a 
> "set_current_state(TASK_INTERRUPTIBLE);"
> 
> (io_request_lock is my current suspect).

Oh - I see, you think there is a real lock contention! No - one can't
lock on the io lock, because one never sleeps when holding it (and I
don't!).  So no thread will swap out of the processor when holding that
lock. Hence no contention, unless you are suggesting we do an infinite
loop while holding the lock (nope).

The deadlock you see is almost certainly the kernel free-buffer
algorithm doing "I want to free buffers so lets flush dirty buffers to
enbd ...", both sides at once. It would be ok if it chose some other
device candidate, but unfortunately it's deterministic!

Mind you, your theory is interesting.  But why would we want to allow an
interrupt of our thread just before sending it out of the processor?
When it's out, it can't be interrupted. And clr kernel queue, even if
it deadlocked, would only deadlock _after_ device failure (because it
is run when we try and go and kill all requests aimed at us, after
having done an "echo 0 > /proc/nbdinfo", for example).  So it can't be
that function.  Or does TASK_INTERRUPIBLE mean something else?  My vague
understanding of TASK_INTERRUPTIBLE is that this is low-level jiggery
pokery that we do not want to be playing with, and also merely tells the
scheduler how to treat us when we are in the procesor, not when we are
out.

There is another schedule_timeout, in wait_for_completion_timeout. 
But that's used to delay a client that is waiting for a remote ioctl to
be handled until the request that carried the ioctl has come back from
the server. There is a completion event signalled when the request
is end_io'ed (and we are careful not to destroy the request data), and
that wakes the wait_for_completion_timeout, and then we go on to
retrieve the data in the request and send the now free request back to
the heap.

Hmmm ... I just noticed __wait_event(wq, condition) in sched.h,  which
seems to do what some of the code in the enbd driver does. Thanks for
making me look!

Peter