On Fri, Jul 21, 2023 at 11:59 AM David Laight [off-list ref] wrote:

....

quoted

Right, it shouldn't need to cache. To Eric's point it might be risky to remove
the barrier() and someone needs to explain that issue first (or IMO there needs
to be another tangible reason like performance etc). Anyway, FWIW I wrote a
simple program and I am not seeing the head->first cached with the pattern you
shared above:

#include <stdlib.h>

#define READ_ONCE(x) (*(volatile typeof(x) *)&(x))
#define barrier() __asm__ __volatile__("": : :"memory")

typedef struct list_head {
     int first;
     struct list_head *next;
} list_head;

int main() {
     list_head *head = (list_head *)malloc(sizeof(list_head));
     head->first = 1;
     head->next = 0;

     READ_ONCE(head->first);
     barrier();
     READ_ONCE(head->first);

     free(head);
     return 0;
}

You probably need to try harder to generate the error.
It probably has something to do code surrounding the
sk_nulls_for_each_rcu() in the ca065d0c^ version of udp.c.

That patch removes the retry loop - and probably breaks udp receive.
The issue is that sockets can be moved between the 'hash2' chains
(eg by connect()) without being freed.

I was just replying to Alan's question on the behavior of READ_ONCE()
since I myself recently got surprised by compiler optimizations
related to it. I haven't looked into the actual UDP code.

 - Joel