Re: [PATCH] Linux 2.6.0-test8 __might_sleep warnings on boot

[Jesper Juhl]

On Sun, 19 Oct 2003, Roland Dreier wrote:

> >>>>> " " == Jesper Juhl <juhl-lkml@xxxxxx> writes:
>
>     > On Sun, 18 Oct 2003, Roland Dreier wrote:
>     >
>     > > -	if (in_atomic() || irqs_disabled()) {
>     > > +	/* Don't print warnings until system_running is set.  This avoids
>     > > +	   spurious warnings during boot before local_irq_enable() and
>     > > +	   init_idle(). */
>     > > +	if (system_running && (in_atomic() || irqs_disabled())) {
>     >
>     > Wouldn't this :
>     >
>     > if ((in_atomic() || irqs_disabled()) && system_running)
>     >
>     > be slightly more efficient?   The reason I say that is that I would assume
>     > that the chance of (in_atomic() || irqs_disabled()) being false is greater
>     > than the chance of !system_running - if that is so, then reordering the if
>     > will allow it to break out early more often...
>
> Yes, I think you're right about the efficiency.  However, I didn't
> think this was a performance-critical code path (especially since it
> is only turned on by a debugging config option),

No, I don't think it is critical in any way either. I just read the patch
and thought "why not do it the most efficient way even if it's not a
critical path?"...
But I'm probably just being a pedant here...

> and the way I wrote
> it matched the way I was thinking about the test ("If the system is
> running, then check if we're in atomic or have irqs disabled").
> But I don't think it matters much either way.
>
Probably not.


/Jesper Juhl

-
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/