Re: Recent change in tcp_output.c is surely wrong

From: David S. Miller (davem@redhat.com)
Date: Mon Jan 17 2000 - 16:18:45 EST


   Date: Mon, 17 Jan 2000 15:53:04 -0500
   From: Matthew Wilcox <willy@thepuffingroup.com>

   On Mon, Jan 17, 2000 at 05:49:43PM +0100, Jamie Lokier wrote:
> I noticed this in pre-patch-2.3.40-4.gz:
>
>
> /* Stay within the limit we were given */
> - timeout = tp->ato;
> + timeout = (tp->ato << 1) >> 1;
> if (timeout > max_timeout)
> timeout = max_timeout;
>
> What is the point of this?
>
> - If tp->ato has bit 31 set, then the effect depends on the width of `int'.
> (Because all arithmetic operations in C extend to `int' width first).
>

Not true, ato is a u32 and I just double checked that the computation
is kept unsigned by the compiler on both sparc64 and ix86.

> - If tp->ato is bit 31 clear, this has no effect.

This is intentional, the goal of the code is to make sure bit
31 is clear. It might be set, it might not, we don't know
at the time the code is run.

> If the intention is to clear bit 31, `&= 0x7fffffff' is the thing which
> works and is probably more efficient.

Not true on all RISC machines I am familiar with. It's 2 instructions
either way. On x86 you'll end up using a larger opcode and one of
x86's most notable performance advantages is it's code density.

   and `&= (1U<<31)' is more obvious. Using a symbolic constant would be even
   better.

You mean '&= ~(1UL<<31)'

Later,
David S. Miller
davem@redhat.com

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sun Jan 23 2000 - 21:00:16 EST