lkml.org 
[lkml]   [2009]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] spinlock: __raw_spin_is_locked() should return true for UP


On Tue, 18 Aug 2009, Steven Rostedt wrote:
>
> > The thing is, some people may assert that a lock is held, but others could
> > easily be looping until it's not held using something like
> >
> > while (spin_is_locked(lock))
> > cpu_relax();
>
> Wouldn't something like that be really racey? And anyone doing such a
> thing had better have that code within an #ifdef CONFIG_SMP.

Sure, it's hopefully inside a #ifdef CONFIG_SMP.

And no, it's not necessarily racy. Sure, it's race in itself if that's all
you are doing, but I could imagine writing that kind of code if I knew
some lock was likely held, and I wanted to avoid doing a "try_lock()"
until it got released.

The point is, "spin_is_locked()" is simply not a well-defined operation in
this case. It could go either way.

And for the original case, we actually have a function for that:

assert_spin_locked(x)

which goes away on UP. Exactly because

BUG_ON(!spin_is_locked(x))

is not a good thing to do!

> > so it's hard to tell whether it should return true or false in the case
> > where spin-locking simply doesn't exist.
>
> Actually, I did have a case where I would use it and would expect a return
> of 0. That was in the experimental printk code to see if it was safe to
> wakeup the klogd. I once had a check of the current cpu runqueue lock is
> locked, and if it was, not to wake up klogd. I'm sure there's other cases
> like this as well.

Yeah, "spin_is_locked()" can be useful for those kinds of things. A
heuristic for whether we should do something based on whether some other
CPU holds it (or we migth have recursion).

Exactly like it can be useful for doing the BUG_ON thing. But in both
cases it's a bit iffy.

> Thinking about it, UP probably should have spin_is_locked always return
> false, but if you want to make sure you are not in a critical section
> with the lock not held, then use assert_spin_locked, which on UP should be
> a nop.

That's what we do. That said, I also think we should generally try to
avoid the kind of code that depends on spin_is_locked always returning
false, for the same reason we should try to avoid any code that depends on
it always returning true.

Linus


\
 
 \ /
  Last update: 2009-10-18 23:28    [W:0.070 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site