RE: [PATCH net-next 3/9] net: Remove e_nobufs label from ip_route_input_slow

[David Laight]

From: Eric W. Biederman
> Sent: 23 September 2015 03:15
> David Ahern  writes:
> 
> > e_nobufs has 1 user. Move setting err to -ENOBUFS for the 1 user and
> > use the goto out label instead of e_nobufs. Stepping stone patch; next
> > one moves rth code into a helper function.
> 
> Ick you are pessimizing the code.
> 
> You will almost certainly have better code generation if you hoist
> the assignment of "err = -ENOBUFS" above the rt_dst_alloc then you
> don't need to do anything in your error path except "goto out;"

Unlikely to make a difference.
The compiler is very likely to set 'err' before the conditional
branch anyway. It will use an extra register to make it all work.

David

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 3/9] net: Remove e_nobufs label from ip_route_input_slow

[Eric W. Biederman]

David Ahern  writes:

> e_nobufs has 1 user. Move setting err to -ENOBUFS for the 1 user and
> use the goto out label instead of e_nobufs. Stepping stone patch; next
> one moves rth code into a helper function.

Ick you are pessimizing the code.

You will almost certainly have better code generation if you hoist
the assignment of "err = -ENOBUFS" above the rt_dst_alloc then you
don't need to do anything in your error path except "goto out;"

Eric

> Signed-off-by: David Ahern 
> ---
>  net/ipv4/route.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 3c5000db5972..e3b18cc1952f 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1805,8 +1805,10 @@ out:   return err;
>  
>   rth = rt_dst_alloc(net->loopback_dev, flags | RTCF_LOCAL, res.type,
>  IN_DEV_CONF_GET(in_dev, NOPOLICY), false, do_cache);
> - if (!rth)
> - goto e_nobufs;
> + if (!rth) {
> + err = -ENOBUFS;
> + goto out;
> + }
>  
>   rth->dst.output= ip_rt_bug;
>  #ifdef CONFIG_IP_ROUTE_CLASSID
> @@ -1852,10 +1854,6 @@ out:   return err;
>  #endif
>   goto out;
>  
> -e_nobufs:
> - err = -ENOBUFS;
> - goto out;
> -
>  martian_source:
>   ip_handle_martian_source(dev, in_dev, skb, daddr, saddr);
>   goto out;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next 3/9] net: Remove e_nobufs label from ip_route_input_slow

[David Ahern]

On 9/22/15 8:15 PM, Eric W. Biederman wrote:

David Ahern  writes:


e_nobufs has 1 user. Move setting err to -ENOBUFS for the 1 user and
use the goto out label instead of e_nobufs. Stepping stone patch; next
one moves rth code into a helper function.


Ick you are pessimizing the code.

You will almost certainly have better code generation if you hoist
the assignment of "err = -ENOBUFS" above the rt_dst_alloc then you
don't need to do anything in your error path except "goto out;"


Can't do that here because the current value of err is used later in:
rth->dst.error = -err;

Besides, as mentioned above this is a stepping stone patch all of this 
is moved to a helper in patch 4. Where I could do the assignment before 
the rt_dst_alloc since I put the original err as an input arg rth_err.


David


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html