Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-11 Thread Jarek Poplawski
On Thu, Jan 10, 2008 at 03:51:11PM -0800, Paul E. McKenney wrote: > On Fri, Jan 11, 2008 at 12:10:42AM +0100, Jarek Poplawski wrote: > > Eric Dumazet wrote, On 01/09/2008 11:37 AM: > > ... > > > [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache > > .

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-11 Thread Jarek Poplawski
On Fri, Jan 11, 2008 at 09:37:42PM +1100, Herbert Xu wrote: > On Fri, Jan 11, 2008 at 09:30:10AM +0100, Jarek Poplawski wrote: > > > > It looks like I'm really too lazy and/or these selfdocumenting features > > of RCU are a bit overrated: one can never be sure which pointer is > > really RCU prote

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-11 Thread Jarek Poplawski
On Fri, Jan 11, 2008 at 09:38:52PM +1100, Herbert Xu wrote: > On Fri, Jan 11, 2008 at 10:11:40AM +0100, Jarek Poplawski wrote: > > > > So, IOW: strictly speaking you are right, r can't change here, but I > > meant r vs. the returned value! Before the patch the returned value > > couldn't be NULL u

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-11 Thread Herbert Xu
On Fri, Jan 11, 2008 at 09:30:10AM +0100, Jarek Poplawski wrote: > > It looks like I'm really too lazy and/or these selfdocumenting features > of RCU are a bit overrated: one can never be sure which pointer is > really RCU protected without checking a few places?! So, after looking > at this rt_ca

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-11 Thread Herbert Xu
On Fri, Jan 11, 2008 at 10:11:40AM +0100, Jarek Poplawski wrote: > > So, IOW: strictly speaking you are right, r can't change here, but I > meant r vs. the returned value! Before the patch the returned value > couldn't be NULL unless all elements of the list were looped. After > this patch it seem

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-11 Thread Jarek Poplawski
On Fri, Jan 11, 2008 at 10:11:40AM +0100, Jarek Poplawski wrote: ... > So, IOW: strictly speaking you are right, r can't change here, but I > meant r vs. the returned value! Before the patch the returned value > couldn't be NULL unless all elements of the list were looped. After ...even more stric

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-11 Thread Jarek Poplawski
On Fri, Jan 11, 2008 at 09:30:10AM +0100, Jarek Poplawski wrote: > On Fri, Jan 11, 2008 at 11:00:20AM +1100, Herbert Xu wrote: > > On Fri, Jan 11, 2008 at 12:10:42AM +0100, Jarek Poplawski wrote: > > > > > > It seems this optimization could've a side effect: if during such a > > > loop updates are

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-11 Thread Jarek Poplawski
On Fri, Jan 11, 2008 at 11:00:20AM +1100, Herbert Xu wrote: > On Fri, Jan 11, 2008 at 12:10:42AM +0100, Jarek Poplawski wrote: > > > > It seems this optimization could've a side effect: if during such a > > loop updates are done, and r is seen !NULL during while() check, but > > NULL after rcu_dere

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-10 Thread Herbert Xu
On Fri, Jan 11, 2008 at 12:10:42AM +0100, Jarek Poplawski wrote: > > It seems this optimization could've a side effect: if during such a > loop updates are done, and r is seen !NULL during while() check, but > NULL after rcu_dereference(), the listing/counting could stop too > soon. So, IMHO, proba

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-10 Thread Paul E. McKenney
On Fri, Jan 11, 2008 at 12:10:42AM +0100, Jarek Poplawski wrote: > Eric Dumazet wrote, On 01/09/2008 11:37 AM: > ... > > [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache > ... > > diff --git a/net/ipv4/route.c b/net/ipv4/route.c > > index d337706..28484f

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-10 Thread Jarek Poplawski
Eric Dumazet wrote, On 01/09/2008 11:37 AM: ... > [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache ... > diff --git a/net/ipv4/route.c b/net/ipv4/route.c > index d337706..28484f3 100644 > --- a/net/ipv4/route.c > +++ b/net/ipv4/route.c > @@ -283,12 +283,12 @@ st

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-10 Thread David Miller
From: Eric Dumazet <[EMAIL PROTECTED]> Date: Wed, 9 Jan 2008 11:37:27 +0100 > [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache > > In rt_cache_get_next(), no need to guard seq->private by a rcu_dereference() > since seq is private to the thread running t

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-09 Thread Eric Dumazet
David Miller a écrit : From: "Paul E. McKenney" <[EMAIL PROTECTED]> Date: Wed, 9 Jan 2008 06:22:58 -0800 On Wed, Jan 09, 2008 at 11:37:27AM +0100, Eric Dumazet wrote: On Wed, 9 Jan 2008 20:46:37 +1100 Herbert Xu <[EMAIL PROTECTED]> wrote: diff --git a/net/ipv4/route.c b/net/ipv4/route

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-09 Thread Paul E. McKenney
On Wed, Jan 09, 2008 at 06:31:26AM -0800, David Miller wrote: > From: "Paul E. McKenney" <[EMAIL PROTECTED]> > Date: Wed, 9 Jan 2008 06:22:58 -0800 > > > On Wed, Jan 09, 2008 at 11:37:27AM +0100, Eric Dumazet wrote: > > > On Wed, 9 Jan 2008 20:46:37 +1100 > > > Herbert Xu <[EMAIL PROTECTED]> wrote

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-09 Thread David Miller
From: "Paul E. McKenney" <[EMAIL PROTECTED]> Date: Wed, 9 Jan 2008 06:22:58 -0800 > On Wed, Jan 09, 2008 at 11:37:27AM +0100, Eric Dumazet wrote: > > On Wed, 9 Jan 2008 20:46:37 +1100 > > Herbert Xu <[EMAIL PROTECTED]> wrote: > > > > diff --git a/net/ipv4/route.c b/net/ipv4/route.c > > index d337

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-09 Thread Paul E. McKenney
ng the > > dereference. Agreed -- as long as you don't try to dereference the pointer before passing it through rcu_dereference(), and as long as both the initial fetch of the pointer, the rcu_dereference(), and the actual dereferencing of the pointer are all within the same RCU

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-09 Thread Eric Dumazet
read the pointer value before the barrier is > irrelevant to the effectiveness of the barrier preceding the > dereference. You are absolutely right Herbert, so I changed the patch to : [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache In rt_cache_get_next(), no need to guard s

Re: [NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-09 Thread Herbert Xu
On Wed, Jan 09, 2008 at 08:38:56AM +0100, Eric Dumazet wrote: > > I am not sure this is valid, since it will do this : > > r = rt_hash_table[st->bucket].chain; > if (r) > return rcu_dereference(r); > > So compiler might be dumb enough do dereference > &rt_hash_table[st->bucket].chain two ti

[NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

2008-01-08 Thread Eric Dumazet
n; if (r) return rcu_dereference(r); So compiler might be dumb enough do dereference &rt_hash_table[st->bucket].chain two times. It seems Dipankar is busy at this moment, so I will post again the patch and ask a comment from Paul :) Thank you [NET] ROUTE: fix rcu_dereference() use