Re: [PATCH] fix NULL pointer + success return in route lookup path

[Jarek Poplawski <jarkao2@gmail.com>]

On Mon, Jun 22, 2009 at 03:29:56PM +0400, Alexey Kuznetsov wrote:
> 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'"? :-)

Hmm... 'would' could be 'could', I guess... ;-) 

> 
> 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.

Righ, but now there is some RCU period after refcount goes 0, not
before.

> 
> 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.

If it's like this then of course there is no problem.

> 
> >					 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.

I mean checking refcnts on some list in rt_intern_hash while adding
a new uncached dst.

Jarek P.