netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: James Chapman <jchapman@katalix.com>,
	Stephen Hemminger <shemminger@vyatta.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH v3 10/12] l2tp: Add L2TP ethernet pseudowire support
Date: Fri, 2 Apr 2010 09:21:07 -0700	[thread overview]
Message-ID: <20100402162107.GF2549@linux.vnet.ibm.com> (raw)
In-Reply-To: <1270223931.11099.7.camel@edumazet-laptop>

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

  reply	other threads:[~2010-04-02 16:21 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-30 16:17 [PATCH v3 00/12] l2tp: Introduce L2TPv3 support James Chapman
2010-03-30 16:17 ` [PATCH v3 01/12] l2tp: Relocate pppol2tp driver to new net/l2tp directory James Chapman
2010-03-30 16:17 ` [PATCH v3 02/12] l2tp: Split pppol2tp patch into separate l2tp and ppp parts James Chapman
2010-03-30 16:17 ` [PATCH v3 03/12] ppp: Add ppp_dev_name() exported function James Chapman
2010-03-30 16:17 ` [PATCH v3 04/12] l2tp: Add ppp device name to L2TP ppp session data James Chapman
2010-03-30 16:29   ` Stephen Hemminger
2010-03-31  7:43     ` James Chapman
2010-03-31  8:46       ` David Miller
2010-03-31 23:08       ` Stephen Hemminger
2010-04-01  7:19         ` James Chapman
2010-04-01  7:30           ` Eric Dumazet
2010-04-01  8:55             ` James Chapman
2010-04-01  7:34           ` David Miller
2010-04-01  8:59             ` James Chapman
2010-04-01  9:04               ` David Miller
2010-03-30 16:17 ` [PATCH v3 05/12] l2tp: Add L2TPv3 protocol support James Chapman
2010-03-30 16:17 ` [PATCH v3 06/12] l2tp: Update PPP-over-L2TP driver to work over L2TPv3 James Chapman
2010-03-30 16:18 ` [PATCH v3 07/12] l2tp: Add L2TPv3 IP encapsulation (no UDP) support James Chapman
2010-03-30 16:18 ` [PATCH v3 08/12] netlink: Export genl_lock() API for use by modules James Chapman
2010-03-30 16:18 ` [PATCH v3 09/12] l2tp: Add netlink control API for L2TP James Chapman
2010-03-31  8:59   ` Patrick McHardy
2010-03-30 16:18 ` [PATCH v3 10/12] l2tp: Add L2TP ethernet pseudowire support James Chapman
2010-03-30 16:30   ` Stephen Hemminger
2010-03-30 16:30   ` Stephen Hemminger
2010-03-30 16:32   ` Stephen Hemminger
2010-03-31  9:35     ` James Chapman
2010-03-31 17:38       ` Paul E. McKenney
2010-04-02 15:24         ` Paul E. McKenney
2010-04-02 15:58           ` Eric Dumazet
2010-04-02 16:21             ` Paul E. McKenney [this message]
2010-03-30 16:18 ` [PATCH v3 11/12] l2tp: Add support for static unmanaged L2TPv3 tunnels James Chapman
2010-03-30 16:18 ` [PATCH v3 12/12] l2tp: Update documentation James Chapman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100402162107.GF2549@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=eric.dumazet@gmail.com \
    --cc=jchapman@katalix.com \
    --cc=netdev@vger.kernel.org \
    --cc=shemminger@vyatta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).