netdev - Re: [PATCH v3 10/12] l2tp: Add L2TP ethernet pseudowire support

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] [day] [month] [year] [list]

Message-ID: <20100402162107.GF2549@linux.vnet.ibm.com>
Date:	Fri, 2 Apr 2010 09:21:07 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	James Chapman <jchapman@...alix.com>,
	Stephen Hemminger <shemminger@...tta.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH v3 10/12] l2tp: Add L2TP ethernet pseudowire support

On Fri, Apr 02, 2010 at 05:58:51PM +0200, Eric Dumazet wrote:
> Le vendredi 02 avril 2010 à 08:24 -0700, Paul E. McKenney a écrit :
> > On Wed, Mar 31, 2010 at 10:38:36AM -0700, Paul E. McKenney wrote:
> 
> > > The list_is_last() is RCU-safe already, since the ->next pointer is
> > > only compared, never dereferenced.  I suggest adding a comment to its
> > > header stating that it is RCU-safe.
> > > 
> > > Feel free to create a list_for_each_entry_safe_rcu(), and I will be
> > > happy to review it.
> > 
> > Right.  When using RCU, list_for_each_entry_safe_rcu() is not needed,
> > because destruction of the element you removed doesn't happen until the
> > end of a subsequent grace period.  By which time you will be done with
> > your RCU read-side critical section.
> > 
> > So you can just use list_for_each_entry_rcu(), you do not need a new
> > list_for_each_entry_safe_rcu().
> > 
> > But you did have me going for a bit!!!  ;-)
> 
> Just wondering Paul if you need to rest :)

In this case, I most certainly needed to have engaged my brain more
fully before hitting "send".  :-/

> list_for_each_entry_safe_rcu() is not needed not because of various (and
> very intersting) reasons you gave !!!
> 
> But because list_for_each_entry_safe() is OK, since if we are in a
> destroy phase, we are a writer, and can use regular list primitive.
> 
> Remember _rcu() versions of list iterators are for readers, and readers
> are NOT supposed to delete an item from the list !

Yes, RCU readers had better have acquired the update-side lock before
modifying the list.  And then there would be some debate as to whether
they were still readers.  For example, while reading one list under RCU
protection, you might be modifying a subordinate list while holding the
corresponding lock.

And in the networking code, you would also need to convince the usual
suspects, including Eric, that upgrading to writer in the middle of an
RCU read-side critical section was the right thing to be doing in the
first place!

							Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html