Re: [hpoj-devel] kernel drivers broken with SMP

[Burkhard K. <bu...@bu...>]

Gerhard Fuernkranz
> Gerhard Fuernkranz wrote:
> > 
> >    There is one more thing, which gives me a strange feeling in my stomach,
> >    and that's the fact, that somebody had reported that start_bh_atomic()
> >    and friends do no longer exist in newer 2.3 kernels. Actually I had used
> >    them to prevent races between the top and bottom halves - so I've no
> >    idea what will happen, if they are just skipped.
> 
> After a very short look at the current SuSE 6.4 (-> 2.2.14) sources it
> appears, that ret_from_intr in entry.S does *NOT* run the bottom halfes -
> except it reschedules the current process (in this case schedule() runs them).
> But as long as the top half is running in the kernel, the process should not
> be rescheduled. Therefore I think, that an interrupt should not be able
> trigger a bottom half handler, which can interrupt the top half running in
> kernel mode (-> in an UP(!!!) kernel). But I've taken only a very short look
> and I could have overlooked something ...
> 
> So IMHO there is a chance, that explicit locking between the top and bottom
> halfes is no longer required (for UP kernels) and is just a relict which was
> required in some older kernel version (maybe 2.0???). I cannot remember ...
> Probably they also have removed start_bh_atomic() and friends from recent 2.3
> kernels, because it is no longer required? (in UP kernels and SMP kernels need
> different locking methods anyway).

There is a guide for kernel-locking issues - the kernel-locking-HOWTO. 
It use to be on http://netfilter.kernelnotes.org/unreliable-guides
but seems to be vanished from there. I found it mirrored on 
	pusa.uv.es/~ulisses/netfilter.kernelnotes.org/unreliable-guides

> 
> Nevertheless all the locking stuff in the drivers should be investigated very
> carefully, especially SMP locking, which is currently definitely not supported
> (i.e. missing) in ieee*.c.
> E.g. if the start_bh_atomic() go away, then there are no locks at all, which
> will prevent a top half from running on one processor while a bottom half is
> running on a different processor simultaneouly. So spinlocks must be added to
> protect critical regions in order to handle such situations properly.
The issue of BH is described in Chapter 7. Essentially BH are now 
deprecated due to their limitations with respect to SMP. From a quick 
glance I learned that BH were reimplemented underneath softirqs. There
are now spinlock primitives for "BHs": spin_lock_bh() and spin_unlock_bh().
Other primitives exist: spin_lock_irq, read_lock_irq/bh, write_lock_irq/bh
and their counterparts. For non-SMP systems they collapse to a 
local_xx_disable() call.

If I compare the slip driver from 2.2.14 to 2.4.0 I see that calls to
start_bh_atomic have been replaced by spin_lock_bh. I have to apologize
for not noting this in my former posting where I had only grepped for 
bh_atomic and found some pieces of code where it had vanished without 
replacement.

Burkhard

-- 
Burkhard Kohl	buk at/auf buks.ipn.de