netdev - Re: [PATCH net-next] tcp/dccp: remove twchain

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

Message-ID: <1380832260.19002.230.camel@edumazet-glaptop.roam.corp.google.com>
Date:	Thu, 03 Oct 2013 13:31:00 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] tcp/dccp: remove twchain

On Thu, 2013-10-03 at 21:04 +0100, Ben Hutchings wrote:
> On Thu, 2013-10-03 at 00:22 -0700, Eric Dumazet wrote:
> [...]
> > @@ -146,26 +150,21 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
> >  	spin_lock(lock);
> >  
> >  	/*
> > -	 * Step 2: Hash TW into TIMEWAIT chain.
> > -	 * Should be done before removing sk from established chain
> > -	 * because readers are lockless and search established first.
> > +	 * Step 2: Hash TW into tcp ehash chain.
> > +	 * Notes :
> > +	 * - tw_refcnt is set to 3 because :
> > +	 * - We have one reference from bhash chain.
> > +	 * - We have one reference from ehash chain.
> > +	 * We can use atomic_set() because prior spin_lock()/spin_unlock()
> > +	 * committed into memory all tw fields.
> >  	*/
> > -	inet_twsk_add_node_rcu(tw, &ehead->twchain);
> > +	atomic_set(&tw->tw_refcnt, 1 + 1 + 1);
> > +	inet_twsk_add_node_rcu(tw, &ehead->chain);
> [...]
> 
> 'will commit' rather than 'committed'?

Nope, I am referring to prior spinlock/unlock as in :

spin_lock(&bhead->lock);
spin_unlock(&bhead->lock); 

> 
> But isn't this also a separate optimisation?

Not really, value is 0 by definition, and setting 3 value before the
add_node_rcu() avoid that a lookup see this 0 value.

Not a big deal either way, lookups are under rcu and a concurrent lookup
might not see the socket anyway.




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