netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
To: Jarek Poplawski <jarkao2@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>,
	David Miller <davem@davemloft.net>,
	netdev@vger.kernel.org, mbizon@freebox.fr, dada1@cosmosbay.com,
	pekkas@netcore.fi, jmorris@namei.org, yoshfuji@linux-ipv6.org
Subject: Re: [PATCH] fix NULL pointer + success return in route lookup path
Date: Mon, 22 Jun 2009 15:29:56 +0400	[thread overview]
Message-ID: <20090622112956.GA22313@ms2.inr.ac.ru> (raw)
In-Reply-To: <20090622110023.GA6994@ff.dom.local>

On Mon, Jun 22, 2009 at 11:00:23AM +0000, Jarek Poplawski wrote:
> One little doubt would be RCU: if it's currently used in rt_free, and
> some code depends on it, there would be a change: rt could be freed
> just after atomic dec, without waiting for RCU yet.

Is this bad 'could', or "would be good 'could'"? :-)

Such route always goes through RCU: nobody can destroy it,
destruction happens only if route is on gc list or when RCU callback
is called and refcnt is already 0.

BTW RCU on such routes is redundant, they never were in hash table.
This means that rt_free() could be safely replaced with straight dst_free()
in this place.


>					 The second one is
> about timing: freeing this always from a workqueue could probably
> make a problem if softirqs are often disabled.

Do you mean dst_gc_task?

Yes, it could be optimized out, route can be destroyed when
refcnt hits zero, but this will require checking refcnt in dst_release().
Old days it was well visible both on code size and performance.
Probably, nowadays it is not a problem anymore.

Alexey

  parent reply	other threads:[~2009-06-22 11:30 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-19 17:18 [PATCH] fix NULL pointer + success return in route lookup path Neil Horman
2009-06-20  8:15 ` David Miller
2009-06-20 12:37 ` Jarek Poplawski
2009-06-20 16:39   ` Jarek Poplawski
2009-06-20 17:11     ` Neil Horman
2009-06-20 17:23       ` Jarek Poplawski
2009-06-20 23:47     ` David Miller
2009-06-21 17:11       ` Neil Horman
2009-06-22  5:43         ` Jarek Poplawski
2009-06-22  8:59           ` Alexey Kuznetsov
2009-06-22  9:42             ` David Miller
2009-06-22 10:56               ` Neil Horman
2009-06-22 11:00             ` Jarek Poplawski
2009-06-22 11:08               ` Neil Horman
2009-06-22 12:18                 ` Jarek Poplawski
2009-06-22 15:10                   ` Neil Horman
2009-06-22 11:29               ` Alexey Kuznetsov [this message]
2009-06-22 12:04                 ` Jarek Poplawski
2009-06-20 16:44   ` Neil Horman

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=20090622112956.GA22313@ms2.inr.ac.ru \
    --to=kuznet@ms2.inr.ac.ru \
    --cc=dada1@cosmosbay.com \
    --cc=davem@davemloft.net \
    --cc=jarkao2@gmail.com \
    --cc=jmorris@namei.org \
    --cc=mbizon@freebox.fr \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=pekkas@netcore.fi \
    --cc=yoshfuji@linux-ipv6.org \
    /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).