Re: [PATCH] kernel/timer.c: using spin_lock_irqsave instead of spin_lock + local_irq_save, especially when CONFIG_LOCKDEP not defined

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 20 Jun 2013, Chen Gang wrote:
> On 06/19/2013 06:53 PM, Thomas Gleixner wrote:
> > So
> > 	local_irq_save(flags);
> > 	spin_lock(&lock);
> > 
> > is semantically the same as 
> > 
> > 	spin_lock_irqsave(&lock, flags);
> > 
> 
> Yes (but reverse is NO).
> 
> > And this is completely independent of LOCKDEP.
> 
> NO.
> 
>  	spin_lock_irqsave(&lock, flags);
> 
>  is not semantically the same as
> 
>  	local_irq_save(flags);
>  	spin_lock(&lock);

If A is semantically the same as B, then B is semantically the same as
A. At least that's the common understanding.

You seem to have a different definition of semantics, but I prefer the
common one.
 
> It depend on the spin_lock_irqsave() implementation, if the parameters
> has no relation ship with each other, semantically the same.

Yes, it depends on the implementation, but all implementations do:

     local_irq_save(flags);
     arch_spin_lock_flags(l, flags);

And whether that maps to a reenable interrupts while spinning or not,
has nothing to do with the spinlock semantics.

If you find a single architecture specific implementation, which is
wrong, then fix it and send a patch for it.

The core implementation _IS_ correct. Period.

Thanks,

	tglx