> 2023年7月22日 22:06，David Laight <David.Laight@xxxxxxxxxx> 写道： > > .... >>> Found a related discussion: >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102714 >>> >>> Looks like GCC 10, 11 have been backported, not sure whether GCC 8 has been backported. >>> >>> So, I have the following questions: >>> >>> Given that some people might not update their GCC, do they need to be notified? >>> >>> Do we need to CC Linus? >> >> No need. >> >> I put the following code into a kernel module: >> >> typedef struct list_head_shit { >> int next; >> struct list_head *first; >> } list_head_shit; >> >> static void noinline so_shit(void) { >> list_head_shit *head = (list_head_shit *)kmalloc(sizeof(list_head_shit), GFP_KERNEL); >> head->first = 0; >> head->next = 1; >> >> READ_ONCE(head->first); >> READ_ONCE(head->first); >> >> kfree(head); >> } >> >> x86_64-linux-gnu-gcc-11 generate the following code: >> >> 0000000000000000 <so_shit>: >> 0: 48 8b 3d 00 00 00 00 mov 0x0(%rip),%rdi # 7 <so_shit+0x7> >> 7: ba 10 00 00 00 mov $0x10,%edx >> c: be c0 0c 00 00 mov $0xcc0,%esi >> 11: e8 00 00 00 00 call 16 <so_shit+0x16> >> 16: 48 c7 40 08 00 00 00 movq $0x0,0x8(%rax) >> 1d: 00 >> 1e: 48 89 c7 mov %rax,%rdi >> 21: c7 00 01 00 00 00 movl $0x1,(%rax) >> 27: 48 8b 47 08 mov 0x8(%rdi),%rax # READ_ONCE here >> 2b: 48 8b 47 08 mov 0x8(%rdi),%rax # READ_ONCE here >> 2f: e9 00 00 00 00 jmp 34 <so_shit+0x34> >> 34: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1) >> 3b: 00 00 00 00 >> 3f: 90 nop >> >> The conclusion is that we can rely on READ_ONCE when writing kernel code. >> >> The kernel’s READ_ONCE is different with the one Joel wrote yesterday. (Joel’s is the same as the old >> ACCESS_ONCE) > > You do need to reproduce the error with code that looks like > the loop in the (old) udp.c code. > > Then see if changing the implementation of READ_ONCE() from > a simple 'volatile' access the newer variant makes a difference. > > You also need to check with the oldest version of gcc that is > still supported - that is much older than gcc 11. > > In the udp code the volatile access was on a pointer (which should > qualify as a scaler type) so it may well be the inlining bug you > mentioned earlier, not the 'volatile on non-scaler' feature that > READ_ONCE() fixed. > That fix hasn't been back-ported to all the versions of gcc > that the kernel build supports. Well, the same compiler, the kernel’s READ_ONCE: static int noinline foo(int a, int b, int c) { b = a + 1; c = READ_ONCE(b) + 1; a = c + 1; return a; } 0000000000000040 <foo.constprop.0>: 40: b8 04 00 00 00 mov $0x4,%eax 45: c3 ret Example from: https://stackoverflow.com/questions/70380510/non-conforming-optimizations-of-volatile-in-gcc-11-1 Change the code to: static int noinline foo(int a, volatile int b, int c) { b = a + 1; c = b + 1; a = c + 1; return a; } Doesn’t help. BTW, Clang works fine, even the function is inlined. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
