ACCESS_ONCE usage inside llist_add_batch function

[Cihangir Akturk]

On Tue, Mar 03, 2015 at 12:08:41PM +0530, Chinmay V S wrote:
> On Tue, Mar 3, 2015 at 11:51 AM, John de la Garza <john at jjdev.com> wrote:
> > On Sat, Feb 28, 2015 at 10:12:23PM +0200, Cihangir Akturk wrote:
> >> 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;
> 
> 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.

> 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;
}