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

[Thomas Gleixner]

On Thu, 20 Jun 2013, Chen Gang wrote:
> On 06/20/2013 03:36 PM, Thomas Gleixner wrote:
> > On Thu, 20 Jun 2013, Chen Gang wrote:
> >> > On 06/19/2013 06:49 PM, Thomas Gleixner wrote:
> >>> > > We must do this because some architectures implement
> >>> > > do_raw_spin_lock_flags() in the following way:
> >>> > > 
> >>> > > do_raw_spin_lock_flags(l, flags)
> >>> > > {
> >>> > > 	while (!arch_spin_trylock(l)) {
> >>> > > 	      if (!irq_disabled_flags(flags)) {
> >>> > > 	      	      arch_irq_restore(flags);
> >>> > > 		      cpu_relax();
> >>> > > 		      arch_irq_disable();
> >>> > > 	      }
> >>> > > 	}
> >>> > > }
> >>> > > 
> >> > 
> >> > For mn10300 and sparc64 (not space32), it doesn't like your demo above.
> > Sigh. You're an sparc64 and mn10300 assembler expert, right?
> >  
> 
> No, do you mean: "only the related expert can discuss about it" ?

A discussion requires that the people who are discussing something are
familiar with the matter.

> >> > For API definition, it has no duty to make it correct if the user call
> >> > them with informal ways, especially, the implementation is related with
> >> > various architectures.
> > Nonsense.
> >
> 
> The word 'Nonsense' seems not quite polite.  ;-)

It might be not polite, but it's correct. And I really start to get
annoyed.
 
> At least, when some one see this usage below:
> 
>    spin_lock_irqsave(&l1, flags);
>    spin_unlock(&l1);
>    spin_lock(&l2);
>    spin_unlock_irqrestore(&l2, flags);
> 
> most of them will be amazing.

What's amazing about this?

It's the equivivalent to:

     local_irq_save(flags);
     spin_lock(&l1);
     spin_unlock(&l1);
     spin_lock(&l2);
     spin_unlock(&l2);
     local_irq_restore(flags);

The only difference is, that spin_lock_irqsave() implementations are
allowed to reenable interrupts while spinning, but again that's an
implementation detail which does not matter at all.

Thanks,

	tglx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/