netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarek Poplawski <jarkao2@o2.pl>
To: Patrick McHardy <kaber@trash.net>
Cc: Ranko Zivojnovic <ranko@spidernet.net>,
	akpm@linux-foundation.org, netdev@vger.kernel.org
Subject: Re: + gen_estimator-fix-locking-and-timer-related-bugs.patch added to -mm tree
Date: Tue, 10 Jul 2007 15:10:34 +0200	[thread overview]
Message-ID: <20070710131033.GC3130@ff.dom.local> (raw)
In-Reply-To: <4693797C.1020408@trash.net>

On Tue, Jul 10, 2007 at 02:20:12PM +0200, Patrick McHardy wrote:
> Jarek Poplawski wrote:
> > On Tue, Jul 10, 2007 at 01:09:07PM +0300, Ranko Zivojnovic wrote:
> > 
> >>However I decided not to use _rcu based iteration neither the
> >>rcu_read_lock() after going through the RCU documentation and a bunch of
> >>examples in kernel that iterate through the lists using non _rcu macros
> >>and do list_del_rcu() just fine.
> >>
> >>For readability, the reference to list_del_rcu as well as call_rcu, I
> >>believe, should be enough of the indication. Please do correct me if I
> >>am wrong here.
> > 
> > 
> > It's only my opinion, and it's probably not very popular at least
> > at net/ code, so it's more about general policy and not this
> > particular code. But:
> > - if somebody is looking after some rcu related problems, why can't
> > he/she spare some time by omitting lists without _rcu and not
> > analyzing why/how such lists are used and locked?
> 
> 
> RCU is used for the read-side, using it on the write-side just makes
> things *less* understandable IMO. It will look like the read-side
> but still do modifications.
> 

>From Documentation/RCU/checklist:

"9.      All RCU list-traversal primitives, which include
        list_for_each_rcu(), list_for_each_entry_rcu(),
        list_for_each_continue_rcu(), and list_for_each_safe_rcu(),
        must be within an RCU read-side critical section.  RCU
        read-side critical sections are delimited by rcu_read_lock()
        and rcu_read_unlock(), or by similar primitives such as
        rcu_read_lock_bh() and rcu_read_unlock_bh().

        Use of the _rcu() list-traversal primitives outside of an
        RCU read-side critical section causes no harm other than
        a slight performance degradation on Alpha CPUs.  It can
        also be quite helpful in reducing code bloat when common
        code is shared between readers and updaters."

So, it seems, it's a question of taste about what is this bloat.

I'd certainly agree with you if the write-side looks like in
examples from documentation: there is always a few lines of code
with some spinlocks. But here we see no locks and no comments,
so each time we have to guess which of the supposed here locks
is needed for RCU or maybe for the lists without "rcu".

Sorry, for messing here with my private opinions: I really
shouldn't start this theme.

Jarek P.

  reply	other threads:[~2007-07-10 13:01 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200706271921.l5RJLgCC003910@imap1.linux-foundation.org>
     [not found] ` <1183642800.3789.11.camel@ranko-fc2.spidernet.net>
     [not found]   ` <20070705142135.GG4759@ff.dom.local>
     [not found]     ` <1183646029.4069.11.camel@ranko-fc2.spidernet.net>
     [not found]       ` <1183651165.4069.26.camel@ranko-fc2.spidernet.net>
2007-07-06  6:14         ` + gen_estimator-fix-locking-and-timer-related-bugs.patch added to -mm tree Jarek Poplawski
2007-07-06  6:20           ` Fwd: " Jarek Poplawski
2007-07-06  6:26           ` Jarek Poplawski
2007-07-06  6:45             ` Jarek Poplawski
2007-07-06 12:47               ` Jarek Poplawski
2007-07-06 13:16                 ` Ranko Zivojnovic
2007-07-09  8:25                   ` Jarek Poplawski
2007-07-06 13:14               ` Ranko Zivojnovic
2007-07-06 13:27                 ` Patrick McHardy
2007-07-06 13:59                   ` Ranko Zivojnovic
2007-07-06 14:21                 ` Patrick McHardy
2007-07-06 14:55                   ` Ranko Zivojnovic
2007-07-07  7:55                     ` Ranko Zivojnovic
2007-07-07 15:10                       ` Patrick McHardy
2007-07-09  7:47                         ` Jarek Poplawski
2007-07-09 12:41                         ` Ranko Zivojnovic
2007-07-09 13:52                           ` Patrick McHardy
2007-07-09 16:43                             ` Ranko Zivojnovic
2007-07-09 16:54                               ` Patrick McHardy
2007-07-10  7:34                               ` Jarek Poplawski
2007-07-10 10:09                                 ` Ranko Zivojnovic
2007-07-10 12:17                                   ` Jarek Poplawski
2007-07-10 12:20                                     ` Patrick McHardy
2007-07-10 13:10                                       ` Jarek Poplawski [this message]
2007-07-10 13:51                                         ` Jarek Poplawski

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=20070710131033.GC3130@ff.dom.local \
    --to=jarkao2@o2.pl \
    --cc=akpm@linux-foundation.org \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    --cc=ranko@spidernet.net \
    /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).