linux-kernel - Re: [PATCH v8 6/7] apei/ghes: Use unrcu

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAMj1kXENTJVZU27PYqwWGod4ESkTxgJ8+2vdqWjR5DVRSbNGDg@mail.gmail.com>
Date:   Fri, 14 Oct 2022 17:24:02 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Justin He <Justin.He@....com>, Borislav Petkov <bp@...en8.de>,
        Len Brown <lenb@...nel.org>, James Morse <James.Morse@....com>,
        Tony Luck <tony.luck@...el.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Robert Richter <rric@...nel.org>,
        Robert Moore <robert.moore@...el.com>,
        Qiuxu Zhuo <qiuxu.zhuo@...el.com>,
        Yazen Ghannam <yazen.ghannam@....com>,
        Jan Luebbe <jlu@...gutronix.de>,
        Khuong Dinh <khuong@...amperecomputing.com>,
        Kani Toshi <toshi.kani@....com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "devel@...ica.org" <devel@...ica.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Shuai Xue <xueshuai@...ux.alibaba.com>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v8 6/7] apei/ghes: Use unrcu_pointer for cmpxchg

On Fri, 14 Oct 2022 at 17:11, Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Fri, Oct 14, 2022 at 04:31:37PM +0200, Ard Biesheuvel wrote:
> > +       if (slot != -1) {
> > +               /*
> > +                * Use release semantics to ensure that ghes_estatus_cached()
> > +                * running on another CPU will see the updated cache fields if
> > +                * it can see the new value of the pointer.
> > +                */
> > +               victim = xchg_release(ghes_estatus_caches + slot,
> > +                                     RCU_INITIALIZER(new_cache));
> > +
> > +               /*
> > +                * At this point, victim may point to a cached item different
> > +                * from the one based on which we selected the slot. Instead of
> > +                * going to the loop again to pick another slot, let's just
> > +                * drop the other item anyway: this may cause a false cache
> > +                * miss later on, but that won't cause any problems.
> > +                */
> > +               if (victim) {
> > +                       call_rcu(&rcu_dereference(victim)->rcu,
> > +                                ghes_estatus_cache_rcu_free);
>                 }
>
> I think you can use unrcu_pointer() here instead, there should not be a
> data dependency since the ->rcu member itself should be otherwise unused
> (and if it were, we wouldn't care about its previous content anyway).
>
> But only Alpha cares about that distinction anyway, so *shrug*.
>

Ah yeah good point - and we are not actually dereferencing the pointer
at all here, just adding an offset to get at the address of the rcu
member.

So we can take this block out of the rcu_read_lock() section as well.


> While I much like the xchg() variant; I still don't really fancy the
> verbage the sparse nonsense makes us do.
>
>                 victim = xchg_release(&ghes_estatus_caches[slot], new_cache);
>                 if (victim)
>                         call_rcu(&victim->rcu, ghes_estatus_cache_rcu_free);
>
> is much nicer code.
>
> Over all; I'd simply ignore sparse (I often do).
>

No disagreement there.