2005-04-07 10:11:20

by Keith Owens

[permalink] [raw]
Subject: 2.6.12-rc2 in_atomic() picks up preempt_disable()

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).


2005-04-07 10:17:53

by Arjan van de Ven

[permalink] [raw]
Subject: Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

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?


2005-04-07 10:22:32

by Andrew Morton

[permalink] [raw]
Subject: Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

Keith Owens <[email protected]> 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?

2005-04-07 12:24:44

by Romano Giannetti

[permalink] [raw]
Subject: Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

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

2005-04-07 14:55:07

by Ingo Molnar

[permalink] [raw]
Subject: Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()


* Keith Owens <[email protected]> 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

2005-04-07 18:41:06

by Mikael Pettersson

[permalink] [raw]
Subject: Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

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.

2005-04-07 19:00:39

by Ingo Molnar

[permalink] [raw]
Subject: Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()


* Mikael Pettersson <[email protected]> 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

2005-04-07 20:17:35

by Arjan van de Ven

[permalink] [raw]
Subject: Re: 2.6.12-rc2 in_atomic() picks up preempt_disable()

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.