2.6.12-rc2 in_atomic() picks up preempt_disable()

2.6.12-rc2 in_atomic() picks up preempt_disable()

[Keith Owens]

2.6.12-rc2, with CONFIG_PREEMPT and CONFIG_PREEMPT_DEBUG.  The
in_atomic() macro thinks that preempt_disable() indicates an atomic
region so calls to __might_sleep() result in a stack trace.
preempt_count() returns 1, no soft or hard irqs are running and no
spinlocks are held.  It looks like there is no way to distinguish
between the use of preempt_disable() in the lock functions (atomic) and
preempt_disable() outside the lock functions (do nothing that might
migrate me).

Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

[Arjan van de Ven]

On Thu, 2005-04-07 at 20:10 +1000, Keith Owens wrote:
> 2.6.12-rc2, with CONFIG_PREEMPT and CONFIG_PREEMPT_DEBUG.  The
> in_atomic() macro thinks that preempt_disable() indicates an atomic
> region so calls to __might_sleep() result in a stack trace.

but you're not allowed to schedule when preempt is disabled!

> preempt_count() returns 1, no soft or hard irqs are running and no
> spinlocks are held.  It looks like there is no way to distinguish
> between the use of preempt_disable() in the lock functions (atomic) and
> preempt_disable() outside the lock functions (do nothing that might
> migrate me).

in what code are you seeing this?

Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

[Romano Giannetti]

On Thu, Apr 07, 2005 at 12:17:37PM +0200, Arjan van de Ven wrote:
> On Thu, 2005-04-07 at 20:10 +1000, Keith Owens wrote:
> > 2.6.12-rc2, with CONFIG_PREEMPT and CONFIG_PREEMPT_DEBUG.  The
> > in_atomic() macro thinks that preempt_disable() indicates an atomic
> > region so calls to __might_sleep() result in a stack trace.
> 
> but you're not allowed to schedule when preempt is disabled!
> 

Could it be related to this: 

http://marc.theaimsgroup.com/?l=linux-kernel&m=111277325629959&w=2



-- 
Romano Giannetti             -  Univ. Pontificia Comillas (Madrid, Spain)
Electronic Engineer - phone +34 915 422 800 ext 2416  fax +34 915 596 569

Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

[Andrew Morton]

Keith Owens <kaos@sgi.com> wrote:
>
> 2.6.12-rc2, with CONFIG_PREEMPT and CONFIG_PREEMPT_DEBUG.  The
> in_atomic() macro thinks that preempt_disable() indicates an atomic
> region so calls to __might_sleep() result in a stack trace.
> preempt_count() returns 1, no soft or hard irqs are running and no
> spinlocks are held.  It looks like there is no way to distinguish
> between the use of preempt_disable() in the lock functions (atomic) and
> preempt_disable() outside the lock functions (do nothing that might
> migrate me).

Is this new behaviour?

It sounds correct to me:

	preempt_disable();
	do_something_which_might_sleep();
	preempt_enable();

Is buggy?

Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

[Ingo Molnar]

* Keith Owens <kaos@sgi.com> wrote:

> 2.6.12-rc2, with CONFIG_PREEMPT and CONFIG_PREEMPT_DEBUG.  The 
> in_atomic() macro thinks that preempt_disable() indicates an atomic 
> region so calls to __might_sleep() result in a stack trace. 
> preempt_count() returns 1, no soft or hard irqs are running and no 
> spinlocks are held.  It looks like there is no way to distinguish 
> between the use of preempt_disable() in the lock functions (atomic) 
> and preempt_disable() outside the lock functions (do nothing that 
> might migrate me).

preempt_disable() sections are just as much atomic as spinlocked 
regions. Like the name suggests it.

	Ingo

Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

[Mikael Pettersson]

On Thu, 07 Apr 2005 12:17:37 +0200, Arjan van de Ven wrote:
>On Thu, 2005-04-07 at 20:10 +1000, Keith Owens wrote:
>> 2.6.12-rc2, with CONFIG_PREEMPT and CONFIG_PREEMPT_DEBUG.  The
>> in_atomic() macro thinks that preempt_disable() indicates an atomic
>> region so calls to __might_sleep() result in a stack trace.
>
>but you're not allowed to schedule when preempt is disabled!

That sounds draconian. Where is that requirement stated?

A preempt-disabled region ought to have the same semantics
as in a CONFIG_PREEMPT=n kernel, and since schedule is Ok
in the latter case it should be Ok in the former too.

All that preempt_disable() should do is prevent involuntary
schedules. But the conditional schedules introduced by may-sleep
functions are _voluntary_, so there's no reason to forbid them.

Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

[Ingo Molnar]

* Mikael Pettersson <mikpe@csd.uu.se> wrote:

> On Thu, 07 Apr 2005 12:17:37 +0200, Arjan van de Ven wrote:
> >On Thu, 2005-04-07 at 20:10 +1000, Keith Owens wrote:
> >> 2.6.12-rc2, with CONFIG_PREEMPT and CONFIG_PREEMPT_DEBUG.  The
> >> in_atomic() macro thinks that preempt_disable() indicates an atomic
> >> region so calls to __might_sleep() result in a stack trace.
> >
> >but you're not allowed to schedule when preempt is disabled!
> 
> That sounds draconian. Where is that requirement stated?

(in the code, and in lkml discussions, as usual. There's tons of code 
that correctly handled it and continues to handle it. Let me be clear, 
this isnt some obscure side-effect, this is one of the cornerstones, 
preempt_disable()/enable() always had these semantics, and this is very 
much being relied on in a number of areas.)

> A preempt-disabled region ought to have the same semantics as in a 
> CONFIG_PREEMPT=n kernel, and since schedule is Ok in the latter case 
> it should be Ok in the former too.
> 
> All that preempt_disable() should do is prevent involuntary schedules.  
> But the conditional schedules introduced by may-sleep functions are 
> _voluntary_, so there's no reason to forbid them.

this just hides bugs and introduces bugs. From a critical section POV a 
voluntary preemption is almost the same thing as a voluntary preemption 
- the task may wander to another CPU, and smp_processor_id() might 
become different. If it's not a problem for your code to preempt then 
just enable preemption before calling it. Anyway, preempt_disable() / 
preempt_enable() is pretty much an internal interface and shouldnt be 
used lightly.

	Ingo

Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

[Arjan van de Ven]

On Thu, 2005-04-07 at 20:40 +0200, Mikael Pettersson wrote:
> On Thu, 07 Apr 2005 12:17:37 +0200, Arjan van de Ven wrote:
> >On Thu, 2005-04-07 at 20:10 +1000, Keith Owens wrote:
> >> 2.6.12-rc2, with CONFIG_PREEMPT and CONFIG_PREEMPT_DEBUG.  The
> >> in_atomic() macro thinks that preempt_disable() indicates an atomic
> >> region so calls to __might_sleep() result in a stack trace.
> >
> >but you're not allowed to schedule when preempt is disabled!
> 
> That sounds draconian. Where is that requirement stated?
> 
> A preempt-disabled region ought to have the same semantics
> as in a CONFIG_PREEMPT=n kernel, and since schedule is Ok
> in the latter case it should be Ok in the former too.
> 
> All that preempt_disable() should do is prevent involuntary
> schedules. But the conditional schedules introduced by may-sleep
> functions are _voluntary_, so there's no reason to forbid them.

but that implies you need to remember this after schedule. all in all it
starts to smell more and more like the local irq disable flag, and I at
least thing of it in a very similar way as well.