On Fri, Sep 20, 2019 at 8:11 PM Al Viro <v...@zeniv.linux.org.uk> wrote: > > My apologies ;-/ Correct diff follows:
This is similar to what we do for the regular list_add(), so I have no objections to the micro-optimization. Of course, for list_add() we do it by using a helper function and passing those prev/next pointers to it instead, so it _looks_ very different. But the logic is the same: do the loads of next/prev early and once, so that gcc doesn't think they might alias with the updates. However, I *really* don't like this syntax: struct hlist_node *p = n->next = prev->next; What, what? That's illegible. Both for the double assignment within a declaration, but also for the naming. Yeah, I assume you mean 'p' just for pointer. Fine. But when we are explicitly playing with multiple pointers, just give them a name. In this case, 'next'. So just do hlist_add_behind: struct hlist_node *next = prev->next; n->next = next; prev->next = n; n->pprev = &prev->next; if (next) next->pprev = &n->next; And honestly, I'd rename 'n' with 'new' too while at it. We're not using C++, so we can use sane names (and already do in other places). That way each statement makes sense on its own, rather than being a mess of "what does 'p' and 'n' mean?" Linus