On Tue, Sep 05, 2023 at 04:56:12PM -0400, Joel Fernandes wrote: > On Tue, Sep 5, 2023 at 4:52 PM Paul E. McKenney <paulmck@xxxxxxxxxx> wrote: > > > > On Tue, Sep 05, 2023 at 04:40:46PM -0400, Joel Fernandes wrote: > [...] > > > > > > > So this bug appears to have no effect for default RCU setups, even on very > > > > > > > large 64-bit systems, which is consistent with field experience. Even if > > > > > > > this is the case, it still should be fixed, to avoid confusion if nothing > > > > > > > else. Or just in case someone decides to set CONFIG_RCU_FANOUT_LEAF=32. > > > > > > > Which actually happened the other day due to someone trusting ChatGPT's > > > > > > > opinion about RCU Kconfig options... > > > > > > > > > > > > And I therefore queued Denis's v3 patch with an edited commit log. > > > > > > Of course, if anyone sees some other way that the bug could manifest > > > > > > other than in a 64-bit kernel built with CONFIG_RCU_FANOUT_LEAF greater > > > > > > than 30 on a system with at least 31 CPUs, please let me know so that > > > > > > I can adjust. > > > > > > > > > > > > Thanx, Paul > > > > > > > > > > > > ------------------------------------------------------------------------ > > > > > > > > > > > > commit ed083b0e22f1396dee3599896249a3f218845298 > > > > > > Author: Denis Arefev <arefev@xxxxxxxxx> > > > > > > Date: Mon Sep 4 15:21:14 2023 +0300 > > > > > > > > > > > > Fix srcu_struct node grpmask overflow on 64-bit systems > > > > > > The value of an arithmetic expression 1 << (cpu - sdp->mynode->grplo) > > > > > > > > > > AFAIU, the overflow resides in the "bitwise expression" and not > > > > > the arithmetic expression. > > > > > > > > Rather than quibble about exactly what constitutes arithmetic, I > > > > updated the first paragraph and added your Reviewed-by as shown > > > > below. ;-) > > > > > > > > > Other than this, please add my > > > > > > > > > > Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> > > > > > > > > Thank you all!!! > > > > > > > > ------------------------------------------------------------------------ > > > > > > > > commit 50477ff756ab99402b1523b7c6be8b5d790d05e7 > > > > Author: Denis Arefev <arefev@xxxxxxxxx> > > > > Date: Mon Sep 4 15:21:14 2023 +0300 > > > > > > > > Fix srcu_struct node grpmask overflow on 64-bit systems > > > > > > > > The value of a bitwise expression 1 << (cpu - sdp->mynode->grplo) > > > > is subject to overflow due to a failure to cast operands to a larger > > > > data type before performing the bitwise operation. > > > > > > > > The maximum result of this subtraction is defined by the RCU_FANOUT_LEAF > > > > Kconfig option, which on 64-bit systems defaults to 16 (resulting in a > > > > maximum shift of 15), but which can be set up as high as 64 (resulting > > > > in a maximum shift of 63). A value of 31 can result in sign extension, > > > > resulting in 0xffffffff80000000 instead of the desired 0x80000000. > > > > A value of 31 or greater triggers undefined behavior per the C standard. > > > > > > Do you mean here "A value of 32 or greater"? > > > > > > Only N >= 32 throws warning for: > > > unsigned long foo = (1 << N); > > > > > > N=31 does undesirable sign extension but no warning. > > > > Good catch, thank you, and I will update this on my next rebase. > > Thanks, and with that the patch looks good to me: > Reviewed-by: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx> Thank you very much, and I will apply this on the next rebase. Thanx, Paul
