linux-kernel - Re: [RFC][PATCH 5/5] powerpc: Remove SYNC from

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: <877f0mere1.fsf@concordia.ellerman.id.au>
Date:   Thu, 08 Jun 2017 19:54:30 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Peter Zijlstra <peterz@...radead.org>,
        Nicholas Piggin <npiggin@...il.com>
Cc:     torvalds@...ux-foundation.org, will.deacon@....com,
        oleg@...hat.com, paulmck@...ux.vnet.ibm.com,
        benh@...nel.crashing.org, linux-kernel@...r.kernel.org,
        mingo@...nel.org, stern@...land.harvard.edu,
        linuxppc-dev <linuxppc-dev@...abs.org>
Subject: Re: [RFC][PATCH 5/5] powerpc: Remove SYNC from _switch

Peter Zijlstra <peterz@...radead.org> writes:
> On Thu, Jun 08, 2017 at 05:29:38PM +1000, Nicholas Piggin wrote:
>> On Thu, 8 Jun 2017 08:54:00 +0200
>> Peter Zijlstra <peterz@...radead.org> wrote:
>> > 
>> > Right, so this patch relies on the smp_mb__before_spinlock ->
>> > smp_mb__after_spinlock conversion that makes the rq->lock RCsc and
>> > should thus provide the required SYNC for migrations.
>> 
>> AFAIKS either one will do, so long as there is a hwsync there. The
>> point is just that I have added some commentary in the generic and
>> powerpc parts to make it clear we're relying on that behavior of
>> the primitive. smp_mb* is not guaranteed to order MMIO, it's just
>> that it does on powerpc.
>
> I'm not particularly happy with the generic comment; I don't feel we
> should care that PPC is special here.

I think it'd be nice if there was *some* comment on the two uses of
smp_mb__after_spinlock(), it's fairly subtle, but I don't think it needs
to mention PPC specifically.


If we have:

arch/powerpc/include/asm/barrier.h:
+/*
+ * This must resolve to hwsync on SMP for the context switch path. See
+ * _switch.
+ */
 #define smp_mb__after_spinlock()   smp_mb()


And then something in _switch() that says "we rely on the
smp_mb__after_spinlock() in the scheduler core being a hwsync", that
should probably be sufficient.

cheers