Messages in this thread | | | Date | Mon, 23 Dec 2013 19:33:41 +0100 | From | Oleg Nesterov <> | Subject | Re: spinlock_irqsave() && flags (Was: pm80xx: Spinlock fix) |
| |
On 12/23, Ingo Molnar wrote: > > * Oleg Nesterov <oleg@redhat.com> wrote: > > > Initially I thought that this is obviously wrong, irqsave/irqrestore > > assume that "flags" is owned by the caller, not by the lock. And > > iirc this was certainly wrong in the past. > > > > But when I look at spinlock.c it seems that this code can actually > > work. _irqsave() writes to FLAGS after it takes the lock, and > > _irqrestore() has a copy of FLAGS before it drops this lock. > > I don't think that's true: if it was then the lock would not be > irqsave, a hardware-irq could come in after the lock has been taken > and before flags are saved+disabled.
I do agree that this pattern is not safe, that is why I decided to ask.
But, unless I missed something, with the current implementation spin_lock_irqsave(lock, global_flags) does:
unsigned long local_flags;
local_irq_save(local_flags); spin_lock(lock);
global_flags = local_flags;
so the access to global_flags is actually serialized by lock.
> So AFAICS this is an unsafe pattern, beyond being ugly as hell.
Yes, I think the same.
Oleg.
| |