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

From: Mathieu Desnoyers
Date: Mon Sep 18 2017 - 15:29:28 EST


----- On Sep 18, 2017, at 2:46 PM, Alan Stern stern@xxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxxxx>
>
>> +++ 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