linux-kernel - Re: [PATCH] spinlock: __raw_spin_is_locked() should return true for UP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.01.0908181640120.3158@localhost.localdomain>
Date:	Tue, 18 Aug 2009 16:52:20 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Steven Rostedt <rostedt@...dmis.org>
cc:	Kumar Gala <galak@...nel.crashing.org>,
	linux-kernel@...r.kernel.org, mingo@...e.hu, tglx@...utronix.de,
	linuxppc-dev@...abs.org, peterz@...radead.org
Subject: Re: [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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/