Re: [PATCH v2] rcu: fix a race in hlist_nulls_for_each_entry_rcumacro

From: Roman Gushchin
Date: Wed May 22 2013 - 09:07:31 EST

On 22.05.2013 16:30, Eric Dumazet wrote:
On Wed, 2013-05-22 at 15:58 +0400, Roman Gushchin wrote:

+ * Same as ACCESS_ONCE(), but used for accessing field of a structure.
+ * The main goal is preventing compiler to store &ptr->field in a register.

But &ptr->field is a constant during the whole duration of
udp4_lib_lookup2() and could be in a register, in my case field is at
offset 0, and ptr is a parameter (so could be in a 'register')

The bug you found is that compiler caches the indirection (ptr->field)
into a register, not that compiler stores &ptr->field into a register.

+ */
+#define ACCESS_FIELD_ONCE(PTR, FIELD) (((volatile typeof(*PTR) *)PTR)->FIELD)

Here we force the compiler to consider ptr as volatile, but semantically
it is not required in rcu_dereference(ptr->field)

Actually, we need to mark an "address of a place" where the field value is
located as volatile before dereferencing. I have no idea how to do it in another way,
except using multiple casts and offsetof's, but, IMHO, it will be even more complex:
ACCESS_ONCE(typeof(&ptr->field)((char*)ptr + offsetof(typeof(*ptr), field)))

We want field to be reloaded, not ptr.

So yes, the patch appears to fix the bug, but it sounds not logical to

May be we can enhance it by providing better/more detailed comments here?
Have you any suggestions?

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at