linux-kernel - Re: use of preempt_count instead of in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080325113942.06fe388b@hyperion.delvare>
Date:	Tue, 25 Mar 2008 11:39:42 +0100
From:	Jean Delvare <khali@...ux-fr.org>
To:	Junio C Hamano <gitster@...ox.com>
Cc:	corbet@....net (Jonathan Corbet),
	Andrew Morton <akpm@...ux-foundation.org>,
	stern@...land.harvard.edu, mb@...sch.de, hmh@....eng.br,
	david-b@...bell.net, rpurdie@...ys.net,
	linux-kernel@...r.kernel.org, mingo@...e.hu, geert@...ux-m68k.org
Subject: Re: use of preempt_count instead of in_atomic() at leds-gpio.c

On Tue, 25 Mar 2008 01:52:58 -0700, Junio C Hamano wrote:
> corbet@....net (Jonathan Corbet) writes:
> 
> > diff --git a/include/linux/hardirq.h b/include/linux/hardirq.h
> > index 4982998..63a7782 100644
> > --- a/include/linux/hardirq.h
> > +++ b/include/linux/hardirq.h
> > @@ -72,6 +72,13 @@
> >  #define in_softirq()		(softirq_count())
> >  #define in_interrupt()		(irq_count())
> >  
> > +/*
> > + * Are we running in atomic context?  WARNING: this macro cannot
> > + * always detect atomic context; in particular, it cannot know about
> > + * held spinlocks in non-preemptible kernels.  Thus it should not be
> > + * used in the general case to determine whether sleeping is possible.
> > + * Do not use in_atomic() in driver code.
> > + */
> >  #define in_atomic()		((preempt_count() & ~PREEMPT_ACTIVE) != 0)
> >  
> >  #ifdef CONFIG_PREEMPT
> 
> Is it just me who feels this comment that says "in_atomic() is not a way
> to tell if we are in atomic reliably and cannot be used for such and such"
> very reader-unfriendly?  Ok, maybe the macro is not reliable and is not
> meant to be used for the purpose its name seems to suggest (at least to a
> non-kernel person).  An inevitable question is, then what is it good for?
> What's the right situation to use this macro?
> 
> I guess an additional comment "even if this says no, you could still be in
> atomic, but if this says yes, then you definitely are in atomic and cannot
> sleep" may help unconfuse a clueless reader like myself.

Andrew explained that in_atomic() could deadlock if called in a
condition where it is unreliable (although I did not understand the
details). Documenting that a "yes" from in_atomic() can always be
trusted, would invite driver authors to still use it, when my
understanding is that they still shouldn't.

If drivers shouldn't use in_atomic() at all then I think that the
long-term solution is to move its definition out of <linux/hardirq.h>.
But of course this means fixing all the drivers that still use it first.

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