lkml.org 
[lkml]   [2002]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] lock assertion macros for 2.5.30
On Wed, 7 Aug 2002, Jesse Barnes wrote:

> > > +#define MUST_HOLD(lock) BUG_ON(!spin_is_locked(lock))
> > > +#define MUST_NOT_HOLD(lock) BUG_ON(spin_is_locked(lock))
> >
> > Please tell me the MUST_NOT_HOLD thing is a joke.
>
> Nothing at all, but isn't that how the scsi ASSERT_LOCK(&lock, 0)
> macro worked before? I could just remove all those checks in the scsi
> code I guess.

That would be a better option.

> --- linux-2.5.30/drivers/scsi/scsi_lib.c Thu Aug 1 14:16:26 2002
> +++ linux-2.5.30-lockassert/drivers/scsi/scsi_lib.c Wed Aug 7 11:34:39 2002
> @@ -202,7 +202,7 @@
> Scsi_Device *SDpnt;
> struct Scsi_Host *SHpnt;
>
> - ASSERT_LOCK(q->queue_lock, 0);
> + MUST_NOT_HOLD(q->queue_lock);
>
> spin_lock_irqsave(q->queue_lock, flags);
> if (SCpnt != NULL) {

> After I posted the last patch, a few people asked for MUST_NOT_HOLD so
> I added it back in. Do you think it's a bad idea?

Just look at the above code (also from your patch).

The fact that we take the spin_lock_irqsave() at that point
means we want to protect ourselves from another CPU here.

The MUST_NOT_HOLD basically means the kernel will OOPS the
moment the lock is contended.

In effect, this debugging code makes lock contention fatal!


If you want to detect lock recursion on the same CPU, I'd
suggest the following:

1) add a 'cpu' member to spinlock_t

2) whenever we take a spinlock, assign the current CPU
id to the spinlock->cpu

3) in the spinlock slow path (ie. when the spinlock is
contended and we have to spin) check if the CPU holding
the spinlock is the current CPU ... if it is, BUG()

4) on spin_unlock, check that the CPU unlocking the spinlock
is the same one that's holding the spinlock

This will have the advantages that it'll actually work and
it will also debug spinlock recursion for ANY spinlock in
the system, without the need to insert special debugging
macros into the code...

regards,

Rik
--
Bravely reimplemented by the knights who say "NIH".

http://www.surriel.com/ http://distro.conectiva.com/

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:22    [W:0.177 / U:2.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site