Re: [PATCH] 2.5.x use list_head to handle scsi starved request queues

[Luben Tuikov <luben@splentec.com>]

Patrick Mansfield wrote:
> On Mon, Mar 24, 2003 at 12:12:07PM -0500, Luben Tuikov wrote:
> 
>>If scsi_queue_next_request(q, cmd) is running on more than one CPU
>>and q != q1 then you have a problem with the starved devices list.
> 
> 
> Not with the current locking: we lock queue_lock (equal to host_lock)
> at the start of scsi_queue_next_request, and unlock at the end of
> scsi_queue_next_request.

Are you sure that all device's request queues will use
the same lock (the host lock) in the future?

My problem is that your patch code for scsi_queue_next_request()
behaves as if q->queue_lock is the same for all devices' request_queues
of the host.  And this is a _special case_, valid only at the present!!!!

I absolutely _detest_ the fact that the host_lock is used to lock LLDD's
entry points, SCSI Core's data, *and* the request queues for
all devices of the host... Talk about serializaion!

But okay, if you want to leave it like it is, holding the q->lock
all over the place, _*PLEASE*_, put a comment saying that
the q->lock is actually the host_lock and locks all the
queues for all devices on the host... similar to my ``detest''
paragraph above.  And if this were to be ever changed then
the individual locks will have to be attained and released...
and lots of code _rewritten_.

This of course forbids you to touch the starved_list in ANY other
context, than that of holding _a_ q->queue_lock, and this is _again_ wrong...
(because if the queues were to get their own locks, then you've
got the starved list mangled badly...) (BTDT)

If at least the infrastructre is wrong in principle, the code doesn't
have to be.  I'll try to mock up a patch for ``show and tell''. :-)

(Oh, well...)
-- 
Luben
P.S. Ideally, I'd rather have individual q->locks, no host lock (except for
LLDD's sake), and SCSI Core completely multithread safe.