[7899] in linux-scsi channel archive

home help back first fref pref prev next nref lref last post

Comments requested related to SMP locking and low-level drivers.

daemon@ATHENA.MIT.EDU (Eric Youngdale)
Sun Jan 23 00:38:28 2000

Message-ID: <003601bf650c$f1377660$2017a8c0@eric.home>
From: "Eric Youngdale" <eric@andante.org>
To: <linux-scsi@vger.rutgers.edu>
Date:   Sat, 22 Jan 2000 14:14:44 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit


    One of the things that has been sticking in my craw for a while is the
way that all of the low-level drivers currently use the io_request_lock in
order to provide SMP safety.  There isn't anything wrong with making the
thing SMP safe, of course, the problem is that io_request_lock is
technically designed to protect the queue proper and not provide generic
protection for low-level drivers.  Severe misuse of the thing can lead to
severe system performance problems.  Before the queueing rewrite,
io_request_lock was already held by the mid-level when calling into the
low-level, and the low-level drivers were along for the ride.

    When I first started looking at the rewrite of queueing, I considered
trying to fix this at the same time, and quickly abandoned that plan, as I
needed to limit the scope of what I was doing so that the queueing work
would get completed in a timely fashion.   Nonetheless, currently the
mid-level and all of the upper level drivers are now fairly clean WRT
locking and using io_request_lock (with the exception of the old error
handling code).  In fact, if you look at the places in scsi.c and
scsi_error.c where calls are made to low-level drivers, you will notice that
these calls are now surrounded by calls to grab and release the lock.  In
other words, the mid-level doesn't need that particular lock to be held when
calling low-level drivers - it grab it merely because low-level drivers
currently expect it.

    I am proposing a means by which we can get all of the low-level drivers
to pry their grubby little fingers off the io_request_lock.  I am thinking
of adding a "smp_safe" flag to the host template.  If this flag is not set,
it indicates that the low-level driver is expecting the current behavior (in
other words, it assumes that io_request_lock is held when each of it't
entrypoints is called).  Defining it this way means that with no changes to
the low-level drivers it is possible for things to work correctly.

    If a low-level driver were to set "smp_safe" to 1, it would imply the
opposite.  The mid-layer would no longer grab io_request_lock when calling
down into the low-level driver.   It would be up to the low-level driver to
do whatever locking is appropriate in order to assure SMP safety - this
includes the interrupt handler routine, of course.

    We do have a choice here - I could easily fix it instead so that there
was a pointer to a lock structure in Scsi_Host, and the mid-level would lock
this instead of io_request_lock, but my gut feeling is that this is
architecturally incorrect.  The only advantage of doing it this way is that
it would make it a tad easier to retrofit all of the existing drivers.  To
me it just seems more correct to state that it becomes the complete
responsibility of all low-level drivers to have their own locks and to do
their own locking.

    The changes to the mid-layer to support this are probably < 10Kbyte
worth of diffs.  I have actually been playing with this together with the
appropriate changes to the 1542 driver to see if it holds together (so far
it does).  The motivation for doing this now is that I would like to try and
urge more driver maintainers to switch to using the new error handling code,
and it would probably be easier to tackle all of these things in one swell
foop.  In fact, I am kind of looking at conversion to the new error handling
code as a prerequisite for turning on "smp_safe" - the old error handling
code is such a rats nest that I am not sure if it is possible to support
smp_safe with old error handling.  Quite frankly, I would really rather not
touch the old error handling again unless I absolutely had to :-).

    Comments?

-Eric


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.rutgers.edu

home help back first fref pref prev next nref lref last post