linux-kernel - Re: [RFC/PATCH] doc: volatile considered evil

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: <a781481a0705091234w613f7e59ud3968a7143ed62ac@mail.gmail.com>
Date:	Thu, 10 May 2007 01:04:41 +0530
From:	"Satyam Sharma" <satyam.sharma@...il.com>
To:	"Johannes Stezenbach" <js@...uxtv.org>
Cc:	"Jonathan Corbet" <corbet@....net>,
	"Randy Dunlap" <randy.dunlap@...cle.com>,
	"Paul Sokolovsky" <pmiscml@...il.com>,
	linux-kernel@...r.kernel.org,
	"Andrew Morton" <akpm@...ux-foundation.org>
Subject: Re: [RFC/PATCH] doc: volatile considered evil

On 5/9/07, Johannes Stezenbach <js@...uxtv.org> wrote:
> On Tue, May 08, 2007, Jonathan Corbet wrote:
> >
> > I just took a shot at turning this into something more like a normal
> > document:
> >
> >       http://lwn.net/Articles/233479/
>
> I think the "jiffies variable is special" part misses the
> "for stupid legacy reasons" explanation.
>
> According to the other volatile rules one should use
> something like that:
>
> extern unsigned long __jiffies;
> static inline unsigned long read_ulong(unsigned long *addr)
> {
>         return *(volatile unsigned long *)addr;
> }
> static inline unsigned long get_jiffies(void)
> {
>         return read_ulong(&__jiffies);
> }
>
> But of course changing all references to jiffies in the kernel would
> be insane, thus jiffies is special "for stupid legacy reasons".
>
> Right?

Right. jiffies comes with no locking protection by default (and adding
one today in a patch that is not invasive / disruptive could be
difficult). If it had a spinlock or something for itself then the
above discussion would have been void, everybody would've just grabbed
the lock before accessing jiffies, and the spinlock implementation
would have done the Right Thing by itself (using barriers, obviously,
and so "volatile" would've been unnecessary even then).

Anyway, as things stand, jiffies comes without locks for itself. But
using a volatile "access cast" to read jiffies whenever you might need
it is _still_ not what I would personally prefer. IMO, it's *much*
better to use something like barrier() if / where you're sitting in a
tight loop comparing jiffies to whatever (a timeout expiry for
example).

So, if you _really_ need / want to do something of this sort, I'd much
rather see:

while (time_before(jiffies, expiry))
	barrier();

in code instead of:

while (time_before((*(volatile unsigned long *)&jiffies), expiry))
	;

But of course,

while (time_before(jiffies, expiry))
	cpu_relax();

would be better still.

A last word from Linus himself here would be obviously best, but I'm
not so sure it makes sense to allow the "volatile" type qualifier
_even_ for the jiffies case.
-
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/