Messages in this thread | | | From | Eric Dumazet <> | Date | Tue, 24 May 2022 13:22:49 -0700 | Subject | Re: [RFC] tcp_bbr2: use correct 64-bit division |
| |
On Tue, May 24, 2022 at 1:06 PM Neal Cardwell <ncardwell@google.com> wrote: > > On Tue, May 24, 2022 at 4:01 AM David Laight <David.Laight@aculab.com> wrote: > > > > From: Oleksandr Natalenko > > > Sent: 22 May 2022 23:30 > > > To: Neal Cardwell <ncardwell@google.com> > > > > > > Hello Neal. > > > > > > It was reported to me [1] by Konstantin (in Cc) that BBRv2 code suffers from integer division issue on > > > 32 bit systems. > > > > Do any of these divisions ever actually have 64bit operands? > > Even on x86-64 64bit divide is significantly slower than 32bit divide. > > > > It is quite clear that x * 8 / 1000 is the same as x / (1000 / 8). > > So promoting to 64bit cannot be needed. > > > > David > > The sk->sk_pacing_rate can definitely be bigger than 32 bits if the > network path can support more than 34 Gbit/sec (a pacing rate of 2^32 > bytes per sec is roughly 34 Gibt/sec). This definitely happens. > > So this one seems reasonable to me (and is only in debug code, so the > performance is probably fine): > - (u64)sk->sk_pacing_rate * 8 / 1000, > + div_u64((u64)sk->sk_pacing_rate * 8, 1000), > > For the other two I agree we should rework them to avoid the 64-bit > divide, since we don't need it. > > There is similar logic in mainline Linux in tcp_tso_autosize(), which > is currently using "unsigned long" for bytes. >
Not sure I follow.
sk_pacing_rate is also 'unsigned long'
So tcp_tso_autosize() is correct on 32bit and 64bit arches. There is no forced 64bit operation there.
> Eric, what do you advise? > > thanks, > neal
| |