linux-kernel - Re: [RFC] microoptimizing hlist_add

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sat, 21 Sep 2019 10:03:07 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] microoptimizing hlist_add_{before,behind}

On Fri, Sep 20, 2019 at 8:11 PM Al Viro <viro@...iv.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