ACCESS_ONCE usage inside llist_add_batch function
Reading the lib/llist.c file in the kernel sources, I came across the llist_add_bach function defined like this;
bool llist_add_batch(struct llist_node *new_first, struct llist_node *new_last,
struct llist_head *head)
{
struct llist_node *first;
do {
new_last->next = first = ACCESS_ONCE(head->first);
} while (cmpxchg(&head->first, first, new_first) != first);
return !first;
}One thing bugging my mind is the ACCESS_ONCE macro. Is it really needed here ? I mean I would write this function with ACCES_ONCE moved outside the loop like as follows;
bool llist_add_batch(struct llist_node *new_first, struct llist_node *new_last,
struct llist_head *head)
{
struct llist_node *first, *old;
old = ACCESS_ONCE(head->first);
for (;;) {
first = old;
new_last->next = old;
old = cmpxchg(&head->first, first, new_first);
if (old == first)
break;
}
return !first;
}I think that it may be faster to just use the return value of cmpxchg. But I am not sure about this.
Is my understanding correct ?
Quoted 15 lines · Cihangir Akturk
Reading the
lib/llist.cfile in the kernel sources, I came across thellist_add_bachfunction defined like this;bool llist_add_batch(struct
llist_node*new_first, structllist_node*new_last,
structllist_head*head){ struct llist_node *first; do { new_last->next = first = ACCESS_ONCE(head->first); } while (cmpxchg(&head->first, first, new_first) != first); return !first; }
Quoted 4 lines
One thing bugging my mind is the
ACCESS_ONCEmacro. Is it really needed here ? I mean I would write this function withACCES_ONCEmoved outside the loop like as follows;
Do you know what ACCESS_ONCE does?
I found this comment:
"The compiler
* is also forbidden from reordering successive instances of ACCESS_ONCE()"
from: include/linux/compiler.h
This might be something that does need to be called (to tell the compiler something) on every loop iteration.
Quoted 13 lines · John de la Garza
Quoted 12 lines · Cihangir Akturk
Reading the
lib/llist.cfile in the kernel sources, I came across thellist_add_bachfunction defined like this;bool llist_add_batch(struct
llist_node*new_first, structllist_node*new_last,
structllist_head*head){ struct llist_node *first; do { new_last->next = first = ACCESS_ONCE(head->first); } while (cmpxchg(&head->first, first, new_first) != first);
Quoted 6 lines
Quoted 6 lines
return !first; }One thing bugging my mind is the
ACCESS_ONCEmacro. Is it really needed here ? I mean I would write this function withACCES_ONCEmoved outside the loop like as follows;
Replace ACCESS_ONCE() with volatile and you would most likely understand that it is not what it looks like.
A very unfortunate consequence of what the macro is named. A better name probably would have been - REALLY_REALLY_ACCESS_ONCE()
Checkout http://lwn.net/Articles/624126/ for some obscure bugs and future plans for this.
Quoted 17 lines · Chinmay V S
Quoted 16 lines · John de la Garza
Quoted 15 lines · Cihangir Akturk
Reading the
lib/llist.cfile in the kernel sources, I came across thellist_add_bachfunction defined like this;bool llist_add_batch(struct
llist_node*new_first, structllist_node*new_last,
structllist_head*head){ struct llist_node *first; do { new_last->next = first = ACCESS_ONCE(head->first); } while (cmpxchg(&head->first, first, new_first) != first); return !first; }
Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
One thing bugging my mind is the
ACCESS_ONCEmacro. Is it really needed here ? I mean I would write this function withACCES_ONCEmoved outside the loop like as follows;Replace
ACCESS_ONCE()with volatile and you would most likely understand that it is not what it looks like.
A very unfortunate consequence of what the macro is named. A better name probably would have been -REALLY_REALLY_ACCESS_ONCE()
Thanks for the reply but this isn't the question I asked.
Quoted 2 lines
Checkout http://lwn.net/Articles/624126/ for some obscure bugs and future plans for this.
I didn't know the non-scalar type related issue and there is a READ_ONCE macro out there but again there is nothing to do with my question because as you can see there isn't any non-scalar access being done in the code.
So my question is this; do we really need to use ACCESS_ONCE(head->first) on every iteration instead of just using the return value of cmpxchg function like as follows ? [1]
[1] sample code
struct llist_node *first, *old_first;
old_first = ACCESS_ONCE(head->first);
for (;;) {
first = old_first;
new_last->next = old_first;
old_first = cmpxchg(&head->first, first, new_first);
if (old_first == first)
break;
}Hi Akturk,
Quoted 16 lines · Cihangir Akturk
Reading the
lib/llist.cfile in the kernel sources, I came across thellist_add_bachfunction defined like this;bool llist_add_batch(struct
llist_node*new_first, structllist_node*new_last,
structllist_head*head){ struct llist_node *first; do { new_last->next = first = ACCESS_ONCE(head->first); } while (cmpxchg(&head->first, first, new_first) != first); return !first; }
Quoted 4 lines
One thing bugging my mind is the
ACCESS_ONCEmacro. Is it really needed here ? I mean I would write this function withACCES_ONCEmoved outside the loop like as follows;
ACCESS_ONCE avoids compiler optimization. A kind of compiler optimization can be caching a value in a register and reusing it.
Why compiler does this? Its his job to create optimized code for speed by default.
Now llist is lock less linked list, where multiple consumers and producers can work simultaneously.
Hence during each iteration of the while loop, you want to access the current value of head->first.
And if you remove ACCESS_ONCE, compiler can keep a copy of head->list in a register during first iteration and reuse it from register for the subsequent iterations.
Thanks,
Arun
Quoted 22 lines
bool llist_add_batch(struct
llist_node*new_first, structllist_node*new_last,
structllist_head*head){ struct llist_node *first, *old; old = ACCESS_ONCE(head->first); for (;;) { first = old; new_last->next = old; old = cmpxchg(&head->first, first, new_first); if (old == first) break; } return !first; }I think that it may be faster to just use the return value of cmpxchg. But I am not sure about this.
Quoted 7 lines
Is my understanding correct ?
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20150304/5d78b097/attachment.html