Re: [Question] printk_safe: Do you want synch./barriers in raw_spin_is_locked()

Re: [Question] printk_safe: Do you want synch./barriers in raw_spin_is_locked()

[Sergey Senozhatsky]

Hello,

I'll Cc linux-kernel

On (03/06/18 17:21), Steven Rostedt wrote:
> On Tue, 6 Mar 2018 15:58:46 +0100
> Andrea Parri <parri.andrea@gmail.com> wrote:
> 
> > Dear PRINTK maintainers,
> > 
> > Following a recent discussion on LKML[1], I started auditing callsites
> > of spin_is_locked() and the "implicit" assumptions these sites made on
> > the memory ordering enforced by this primitive (not a first attempt[2],
> > FWIW).  As it turns out, this primitive is mostly used (40+ calls) for
> > debugging purposes (WARN_ON(!spin_is_locked()) or such), but the calls
> > to raw_spin_is_locked() in printk_safe seem to escape this usage.
> > 
> > Which assumptions are you relying on (if any) for raw_spin_is_locked()
> > enforced memory ordering?
> 
> We don't care about other CPUs. The use case here is to make sure that
> the printk in an NMI does not take the lock when the current CPU has
> it. Memory ordering is fine when dealing with only one CPU. If other
> CPUs mess with the result, then the worse that will happen is that we
> go to the "safe" mode when we didn't have to.

Right, thanks Steven.

A side note,
I think the only reason we have that raw_spin_is_locked() is because we
call console drivers in printk-safe mode, so we don't have a 1:1 mapping:

		printk-safe == raw_spin_is_locked(logbuf)

I sort of think we can stop calling console drivers in printk-safe,
there seems to be no real reasons to do so (kind of). And then we will
have that missing printk-safe == raw_spin_is_locked(logbuf) so we will
be able to drop raw_spin_is_locked() from printk-safe and make logbuf
spin_lock static again.

> BTW, next time when asking a question, don't do it off list. This may
> be something others would like to know. I usually don't answer
> questions like this when they don't include a Cc to a mailing list.

Agreed.

	-ss