Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2014-01-03 Thread Dan Williams
On Thu, Jan 2, 2014 at 2:31 AM, Suresh Thiagarajan suresh.thiagara...@pmcs.com wrote: On Fri, Dec 27, 2013 at 9:48 PM, Oleg Nesterov o...@redhat.com wrote: On 12/24, Suresh Thiagarajan wrote: Below is a small pseudo code on protecting/serializing the flag for global access. struct temp {

RE: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2014-01-02 Thread Suresh Thiagarajan
On Fri, Dec 27, 2013 at 9:48 PM, Oleg Nesterov o...@redhat.com wrote: On 12/24, Suresh Thiagarajan wrote: Below is a small pseudo code on protecting/serializing the flag for global access. struct temp { ... spinlock_t lock; unsigned long lock_flags; }; void

Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-27 Thread Oleg Nesterov
On 12/24, Suresh Thiagarajan wrote: Below is a small pseudo code on protecting/serializing the flag for global access. struct temp { ... spinlock_t lock; unsigned long lock_flags; }; void my_lock(struct temp *t) { unsigned long flag; // thread-private

Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-24 Thread Ingo Molnar
* Oleg Nesterov o...@redhat.com wrote: On 12/23, Ingo Molnar wrote: * Oleg Nesterov o...@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

RE: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-24 Thread Suresh Thiagarajan
On Tue, Dec 24, 2013 at 1:59 PM, Ingo Molnar mi...@kernel.org wrote: * Oleg Nesterov o...@redhat.com wrote: On 12/23, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: Initially I thought that this is obviously wrong, irqsave/irqrestore assume that flags is owned by the

Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-24 Thread James Bottomley
On Tue, 2013-12-24 at 09:13 +, Suresh Thiagarajan wrote: On Tue, Dec 24, 2013 at 1:59 PM, Ingo Molnar mi...@kernel.org wrote: * Oleg Nesterov o...@redhat.com wrote: On 12/23, Ingo Molnar wrote: * Oleg Nesterov o...@redhat.com wrote: Initially I thought that this is

Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-23 Thread Linus Torvalds
On Mon, Dec 23, 2013 at 9:27 AM, Oleg Nesterov o...@redhat.com wrote: In short, is this code spinlock_t LOCK; unsigned long FLAGS; void my_lock(void) { spin_lock_irqsave(LOCK, FLAGS); } void my_unlock(void) {

Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-23 Thread Oleg Nesterov
On 12/23, Linus Torvalds wrote: On Mon, Dec 23, 2013 at 9:27 AM, Oleg Nesterov o...@redhat.com wrote: In short, is this code spinlock_t LOCK; unsigned long FLAGS; void my_lock(void) { spin_lock_irqsave(LOCK, FLAGS); }

Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-23 Thread Ingo Molnar
* Oleg Nesterov o...@redhat.com wrote: On 12/23, Oleg Nesterov wrote: Perhaps we should ask the maintainers upstream? Even if this works, I am not sure this is _supposed_ to work. I mean, in theory spin_lock_irqave() can be changed as, say #define spin_lock_irqsave(lock, flags)

Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-23 Thread Oleg Nesterov
On 12/23, Ingo Molnar wrote: * Oleg Nesterov o...@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

Re: spinlock_irqsave() flags (Was: pm80xx: Spinlock fix)

2013-12-23 Thread Linus Torvalds
On Mon, Dec 23, 2013 at 10:24 AM, Oleg Nesterov o...@redhat.com wrote: However, the code above already has the users. Do you think it makes sense to add something like No. I think it makes sense to put a big warning on any users you find, and fart in the general direction of any developer who