Re: [PATCH] tcp: documents timewait refcnt tricks

[Randy Dunlap <rdunlap@xenotime.net>]

Eric Dumazet wrote:
> Evgeniy Polyakov a écrit :
>> Hi Eric.
>>
>> On Fri, Dec 04, 2009 at 02:46:54PM +0100, Eric Dumazet (eric.dumazet@gmail.com) wrote:
>>> First patch changes __inet_hash_nolisten() and __inet6_hash()
>>> to get a timewait parameter to be able to unhash it from ehash
>>> at same time the new socket is inserted in hash.
>>>
>>> This makes sure timewait socket wont be found by a concurrent
>>> writer in __inet_check_established()
>> Both patches look good, although trick with returning reference counter
>> may look like a hack especially when only viewing into ip code and not
>> hashtable itself. Can you please cook up a documentation update for hash
>> function that it is supposed to return refcnt when socket was in hash
>> table.
>>
> 
> Sure, here it is :
> 
> Thanks !
> 
> [PATCH] tcp: documents timewait refcnt tricks 
> 
> Adds kerneldoc for inet_twsk_unhash() & inet_twsk_bind_unhash().
> 
> Suggested-by: Evgeniy Polyakov <zbr@ioremap.net>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> 
> diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> index 1958cf5..cf719c2 100644
> --- a/net/ipv4/inet_timewait_sock.c
> +++ b/net/ipv4/inet_timewait_sock.c
> @@ -15,9 +15,13 @@
>  #include <net/ip.h>
>  
>  
> -/*
> - * unhash a timewait socket from established hash
> - * lock must be hold by caller
> +/**
> + *	inet_twsk_unhash - unhash a timewait socket from established hash
> + *	@tw: timewait socket
> + *
> + *	unhash a timewait socket from established hash, if hashed.
> + *	ehash lock must be hold by caller.

	                   held

> + *	Returns 1 if caller should call inet_twsk_put() after lock release.
>   */
>  int inet_twsk_unhash(struct inet_timewait_sock *tw)
>  {
> @@ -26,12 +30,21 @@ int inet_twsk_unhash(struct inet_timewait_sock *tw)
>  
>  	hlist_nulls_del_rcu(&tw->tw_node);
>  	sk_nulls_node_init(&tw->tw_node);
> +	/*
> +	 * We cannot call inet_twsk_put() ourself under lock,
> +	 * caller must call it for us.
> +	 */
>  	return 1;
>  }
>  
> -/*
> - * unhash a timewait socket from bind hash
> - * lock must be hold by caller
> +/**
> + *	inet_twsk_bind_unhash - unhash a timewait socket from bind hash
> + *	@tw: timewait socket
> + *	@hashinfo: hashinfo pointer
> + *
> + *	unhash a timewait socket from bind hash, if hashed.
> + *	bind hash lock must be hold by caller.

	                       held

> + *	Returns 1 if caller should call inet_twsk_put() after lock release.
>   */
>  int inet_twsk_bind_unhash(struct inet_timewait_sock *tw,
>  			  struct inet_hashinfo *hashinfo)


thanks.
-- 
~Randy