linux-kernel - Re: [RFC PATCH 1/2] membarrier: Provide register expedited private command

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:   Mon, 18 Sep 2017 19:30:06 +0000 (UTC)
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Boqun Feng <boqun.feng@...il.com>,
        Andrew Hunter <ahh@...gle.com>,
        maged michael <maged.michael@...il.com>,
        gromer <gromer@...gle.com>, Avi Kivity <avi@...lladb.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Dave Watson <davejwatson@...com>,
        Will Deacon <will.deacon@....com>,
        Andy Lutomirski <luto@...nel.org>,
        linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [RFC PATCH 1/2] membarrier: Provide register expedited private
 command

----- On Sep 18, 2017, at 2:46 PM, Alan Stern stern@...land.harvard.edu wrote:

> On Mon, 18 Sep 2017, Mathieu Desnoyers wrote:
> 
>> Provide a new command allowing processes to register their intent to use
>> the private expedited command.
>> 
>> This allows PowerPC to skip the full memory barrier in switch_mm(), and
>> only issue the barrier when scheduling into a task belonging to a
>> process that has registered to use expedited private.
>> 
>> Processes are now required to register before using
>> MEMBARRIER_CMD_PRIVATE_EXPEDITED, otherwise that command returns EPERM.
>> 
>> [ Runtime testing on the PowerPC architecture would be welcome. ]
>> 
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
> 
>> +++ b/arch/powerpc/include/asm/membarrier.h
>> @@ -0,0 +1,37 @@
>> +#ifndef _ASM_POWERPC_MEMBARRIER_H
>> +#define _ASM_POWERPC_MEMBARRIER_H
>> +
>> +static inline void membarrier_arch_sched_in(struct task_struct *prev,
>> +		struct task_struct *next)
>> +{
>> +	/*
>> +	 * Only need the full barrier when switching between processes.
>> +	 */
>> +	if (likely(!test_thread_flag(TIF_MEMBARRIER_PRIVATE_EXPEDITED)
>> +			|| prev->mm == next->mm))

I think I should use:

test_ti_thread_flag(next, TIF_MEMBARRIER_PRIVATE_EXPEDITED)

instead of test_thread_flag above.

>> +		return;
>> +
>> +	/*
>> +	 * The membarrier system call requires a full memory barrier
>> +	 * after storing to rq->curr, before going back to user-space.
>> +	 */
>> +	smp_mb();
>> +}
>> +static inline void membarrier_arch_fork(struct task_struct *t,
>> +		unsigned long clone_flags)
>> +{
>> +	/*
>> +	 * Coherence of TIF_MEMBARRIER_PRIVATE_EXPEDITED against thread
>> +	 * fork is protected by siglock. membarrier_arch_fork is called
>> +	 * with siglock held.
>> +	 */
>> +	if (t->mm->membarrier_private_expedited)
>> +		set_ti_thread_flag(t, TIF_MEMBARRIER_PRIVATE_EXPEDITED);
>> +}
> 
> Why have two separate bitflags for the same thing?  Can't you just use
> the mm->membarrier_private_expedited flag everywhere and forget about
> TIF_MEMBARRIER_PRIVATE_EXPEDITED?

The goal here is to save chasing through the task->mm-> pointer in sched_in(),
and directly access the process registration state through a thread flag.

Thanks,

Mathieu


> 
> Alan Stern

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com