linux-kernel - Re: 2.6.25-git2: BUG: unable to handle kernel paging request at ffffffffffffffff

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: <20080421011855.GA6243@gondor.apana.org.au>
Date:	Mon, 21 Apr 2008 09:18:55 +0800
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>,
	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-ext4@...r.kernel.org, "Paul E. McKenney" <paulmck@...ibm.com>
Subject: Re: 2.6.25-git2: BUG: unable to handle kernel paging request at ffffffffffffffff

Hi Linus:

On Sun, Apr 20, 2008 at 02:31:48PM -0700, Linus Torvalds wrote:
>
> Talking about RCU I also think that whoever did those "rcu_dereference()" 
> macros in <linux/list.h> was insane. It's totally pointless to do 
> "rcu_dereference()" on a local variable. It simply *cannot* make sense. 
> Herbert, Paul, you guys should look at it.

Since I made the macros look this way I'm obliged to defend it :)

>  #define list_for_each_rcu(pos, head) \
> -	for (pos = (head)->next; \
> -		prefetch(rcu_dereference(pos)->next), pos != (head); \
> -        	pos = pos->next)
> +	for (pos = rcu_dereference((head)->next); \
> +		prefetch(pos->next), pos != (head); \
> +        	pos = rcu_dereference(pos->next))

Semantically there should be no difference between the two versions.
The purpose of rcu_dereference is really similar to smp_rmb, i.e.,
it adds a (conditional) read barrier between what has been read so
far (including its argument), and what will be read subsequently.

So if we expand out the current code it would look like

	fetch (head)->next
	store into pos
again:
	smp_read_barrier_depends()
	prefetch(pos->next)
	pos != (head)

	...loop body...

	fetch pos->next
	store into pos
	goto again

Yours looks like

	fetch (head)->next
	smp_read_barrier_depends()
	store into pos
again:
	prefetch(pos->next)
	pos != (head)

	...loop body...

	fetch pos->next
	smp_read_barrier_depends()
	store into pos
	goto again

As the objective here is to insert a barrier before dereferencing
pos (e.g., reading pos->next or using it in the loop body), these
two should be identical.

But I do concede that your version looks clearer, and has the
benefit that should prefetch ever be optimised out with no side-
effects, yours would still be correct while the current one will
lose the barrier completely.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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/