Re: SMP in SCSI code.

Linus Torvalds (torvalds@transmeta.com)
Sun, 12 Apr 1998 11:01:33 -0700 (PDT)


On Sun, 12 Apr 1998, Gerard Roudier wrote:
>
> However, looking at the current SCSI code, it seems to me that my changes
> are essentially broken because the current SCSI code does very weird
> things with spin_locking.
> In brief, the way the io_request_lock is handled seems to me a lot ugly
> and I think that it disallows to fine-grain low-level drivers.

It doesn't disable fine-grained low-level drivers, but it makes the
drivers have to be _aware_ of SMP issues if they want to be fine-grained.

Think of it as defaulting to a safe mode, and the driver writers can
become unsafe if they want to. But I'd seriously suggest against it at
this point: don't even _try_ to be clever until everything has proven
itself to be stable (I have a lot of success reports right now, so I know
things are getting better, but I'd prefer threading experimentation to
happen in 2.3 rather than at this point).

> On the other hand, some internal routines do unlocking and so are silently
> assumed to be called with some lock set. Bad news for maintainance!

No.

ALL routines are called with the lock set. That's GOOD news for
maintainers. The only thing that the maintainers need to worry about is
when they get entered from hardware interrupts (including timers) that
they have set up themselves, in which case the first thing they need to
get the lock immediately.

Most of that work has already been done - I'll make a 2.1.96 that should
be SMP safe with just about every SCSI driver. Doing that change was
fairly simple (and thanks to Doug Ledford for the work).

The OLD code was really bad for maintenance, because there was no easy way
to guarantee that things were ok. Old drivers had lots of random "cli+sti"
things to protect certain regions.

> It is indeed not optimal but not broken to call low-level drivers
> entry points with the io_request_lock set, but assuming that callbacks
> needs this lock to be held is ugly requirement IMHO.

Why? 95% of all driver writers are blissfully unaware of SMP locking
issues. Even for the good ones, SMP tends to be something of a black art:
they may know the issues, but it is _so_ easy to make a mistake that you
should be _really_ really careful.

The new setup just means that the driver writer doesn't _have_ to be
careful.

> Each driver level should be responsible of its resources threading
> strategy and must not rely _explicitaly_ on other level spinlock stuff.

Wrong. That might be true if everybody had an IQ of 150, but that's not
how the real world works. In the real world, the new strategy where no
driver has to know is the right one, just trust me on this one.

The new strategy _allows_ fine-frained threading, so it's not a question
of being fascist about this. But I'll say once more that I'd strongly
discourage driver writers from trying to be clever, especially if they
don't have SMP hardware to test things on (and only a small fraction of
driver writers do have SMP machines at all).

> Could you confirm me that I just have to spinlock the io_request_lock
> before calling 'scsi_done' in order not to break the current SMP SCSI
> requirements ? Thanks.

No. You should _never_ need to spinlock at all (except for the interrupt
part, and I already have that done. I'll make a pre-96 on ftp.kernel.org
shortly so that you and others can see what's up).

Linus

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