Re: [PATCH 4/6 net-next] inet: don't check for bind conflicts twice when searching for a port

[David Miller <davem@davemloft.net>]

From: Josef Bacik <jbacik@fb.com>
Date: Thu, 22 Dec 2016 16:26:36 -0500

> This is just wasted time, we've already found a tb that doesn't have a bind
> conflict, and we don't drop the head lock so scanning again isn't going to give
> us a different answer.  Instead move the tb->reuse setting logic outside of the
> found_tb path and put it in the success: path.  Then make it so that we don't
> goto again if we find a bind conflict in the found_tb path as we won't reach
> this anymore when we are scanning for an ephemeral port.
> 
> Signed-off-by: Josef Bacik <jbacik@fb.com>
 ...
> @@ -220,8 +219,10 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
>  		spin_lock_bh(&head->lock);
>  		inet_bind_bucket_for_each(tb, &head->chain)
>  			if (net_eq(ib_net(tb), net) && tb->port == port) {
> +				if (hlist_empty(&tb->owners))
> +					goto success;

I am not sure that this condition can ever trigger.

The only time we will see an alive 'tb' with an empty owners list
is when we allocate one in this function.  And when that allocation
succeeds, we go straight through the rest of this function without
ever jumping out for errors or anything like that.

So we should never drop the allocated fresh 'tb', leave it with
an empty owners list, and branch back up to this part of the
function.