Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1700043 > unrolled thread

Re: [RFC PATCH v2] membarrier: expedited private command

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v2] membarrier: expedited private command Michael Ellerman <mpe@ellerman.id.au> - 2017-07-31 15:30 +0200
    Re: [RFC PATCH v2] membarrier: expedited private command Peter Zijlstra <peterz@infradead.org> - 2017-07-31 15:40 +0200
    Re: [RFC PATCH v2] membarrier: expedited private command Nicholas Piggin <npiggin@gmail.com> - 2017-08-01 02:40 +0200
      Re: [RFC PATCH v2] membarrier: expedited private command Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-08-01 03:40 +0200
        Re: [RFC PATCH v2] membarrier: expedited private command Nicholas Piggin <npiggin@gmail.com> - 2017-08-01 04:10 +0200
          Re: [RFC PATCH v2] membarrier: expedited private command Peter Zijlstra <peterz@infradead.org> - 2017-08-01 10:20 +0200
            Re: [RFC PATCH v2] membarrier: expedited private command Nicholas Piggin <npiggin@gmail.com> - 2017-08-01 12:00 +0200
              Re: [RFC PATCH v2] membarrier: expedited private command Peter Zijlstra <peterz@infradead.org> - 2017-08-01 12:30 +0200
                Re: [RFC PATCH v2] membarrier: expedited private command Avi Kivity <avi@scylladb.com> - 2017-08-01 12:40 +0200
                  Re: [RFC PATCH v2] membarrier: expedited private command Peter Zijlstra <peterz@infradead.org> - 2017-08-01 12:50 +0200
                Re: [RFC PATCH v2] membarrier: expedited private command Nicholas Piggin <npiggin@gmail.com> - 2017-08-01 12:40 +0200
                  Re: [RFC PATCH v2] membarrier: expedited private command Peter Zijlstra <peterz@infradead.org> - 2017-08-01 13:10 +0200
                    Re: [RFC PATCH v2] membarrier: expedited private command Nicholas Piggin <npiggin@gmail.com> - 2017-08-01 14:00 +0200
                Re: [RFC PATCH v2] membarrier: expedited private command "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-08-01 15:30 +0200
                  Re: [RFC PATCH v2] membarrier: expedited private command Peter Zijlstra <peterz@infradead.org> - 2017-08-01 16:20 +0200
                    Re: [RFC PATCH v2] membarrier: expedited private command "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-08-02 01:40 +0200
                      Re: [RFC PATCH v2] membarrier: expedited private command Nicholas Piggin <npiggin@gmail.com> - 2017-08-02 02:50 +0200

#1700043 — Re: [RFC PATCH v2] membarrier: expedited private command

Peter Zijlstra <peterz@infradead.org> writes:

> On Fri, Jul 28, 2017 at 10:55:32AM +0200, Peter Zijlstra wrote:
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index e9785f7aed75..33f34a201255 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -2641,8 +2641,18 @@ static struct rq *finish_task_switch(struct task_struct *prev)
>>  	finish_arch_post_lock_switch();
>>  
>>  	fire_sched_in_preempt_notifiers(current);
>> +
>> +	/*
>> +	 * For CONFIG_MEMBARRIER we need a full memory barrier after the
>> +	 * rq->curr assignment. Not all architectures have one in either
>> +	 * switch_to() or switch_mm() so we use (and complement) the one
>> +	 * implied by mmdrop()'s atomic_dec_and_test().
>> +	 */
>>  	if (mm)
>>  		mmdrop(mm);
>> +	else if (IS_ENABLED(CONFIG_MEMBARRIER))
>> +		smp_mb();
>> +
>>  	if (unlikely(prev_state == TASK_DEAD)) {
>>  		if (prev->sched_class->task_dead)
>>  			prev->sched_class->task_dead(prev);
>> 
>> 
>
>> a whole bunch of architectures don't in fact need this extra barrier at all.
>
> In fact, I'm fairly sure its only PPC.
>
> Because only ARM64 and PPC actually implement ACQUIRE/RELEASE with
> anything other than smp_mb() (for now, Risc-V is in this same boat and
> MIPS could be if they ever sort out their fancy barriers).
>
> TSO archs use a regular STORE for RELEASE, but all their atomics imply a
> smp_mb() and there are enough around to make one happen (typically
> mm_cpumask updates).
>
> Everybody else, aside from ARM64 and PPC must use smp_mb() for
> ACQUIRE/RELEASE.
>
> ARM64 has a super duper barrier in switch_to().
>
> Which only leaves PPC stranded.. but the 'good' news is that mpe says
> they'll probably need a barrier in switch_mm() in any case.

I may have been sleep deprived. We have a patch, probably soon to be
merged, which will add a smp_mb() in switch_mm() but *only* when we add
a CPU to mm_cpumask, ie. when we run on a CPU we haven't run on before.

I'm not across membarrier enough to know if that's sufficient, but it
seems unlikely?

cheers

[toc] | [next] | [standalone]

#1700053

On Mon, Jul 31, 2017 at 11:20:59PM +1000, Michael Ellerman wrote:
> Peter Zijlstra <peterz@infradead.org> writes:

> > In fact, I'm fairly sure its only PPC.
> >
> > Because only ARM64 and PPC actually implement ACQUIRE/RELEASE with
> > anything other than smp_mb() (for now, Risc-V is in this same boat and
> > MIPS could be if they ever sort out their fancy barriers).
> >
> > TSO archs use a regular STORE for RELEASE, but all their atomics imply a
> > smp_mb() and there are enough around to make one happen (typically
> > mm_cpumask updates).
> >
> > Everybody else, aside from ARM64 and PPC must use smp_mb() for
> > ACQUIRE/RELEASE.
> >
> > ARM64 has a super duper barrier in switch_to().
> >
> > Which only leaves PPC stranded.. but the 'good' news is that mpe says
> > they'll probably need a barrier in switch_mm() in any case.
> 
> I may have been sleep deprived. We have a patch, probably soon to be
> merged, which will add a smp_mb() in switch_mm() but *only* when we add
> a CPU to mm_cpumask, ie. when we run on a CPU we haven't run on before.
> 
> I'm not across membarrier enough to know if that's sufficient, but it
> seems unlikely?

Correct, that would be insufficient. We'd need it every time switch_mm()
does indeed change the effective mm.

Now you also spoke of looking at clearing bits in mm_cpumask(), and I
suspect that if you do that, you end up having to do a barrier every
time.

[toc] | [prev] | [next] | [standalone]

#1700539

On Mon, 31 Jul 2017 23:20:59 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Peter Zijlstra <peterz@infradead.org> writes:
> 
> > On Fri, Jul 28, 2017 at 10:55:32AM +0200, Peter Zijlstra wrote:  
> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> >> index e9785f7aed75..33f34a201255 100644
> >> --- a/kernel/sched/core.c
> >> +++ b/kernel/sched/core.c
> >> @@ -2641,8 +2641,18 @@ static struct rq *finish_task_switch(struct task_struct *prev)
> >>  	finish_arch_post_lock_switch();
> >>  
> >>  	fire_sched_in_preempt_notifiers(current);
> >> +
> >> +	/*
> >> +	 * For CONFIG_MEMBARRIER we need a full memory barrier after the
> >> +	 * rq->curr assignment. Not all architectures have one in either
> >> +	 * switch_to() or switch_mm() so we use (and complement) the one
> >> +	 * implied by mmdrop()'s atomic_dec_and_test().
> >> +	 */
> >>  	if (mm)
> >>  		mmdrop(mm);
> >> +	else if (IS_ENABLED(CONFIG_MEMBARRIER))
> >> +		smp_mb();
> >> +
> >>  	if (unlikely(prev_state == TASK_DEAD)) {
> >>  		if (prev->sched_class->task_dead)
> >>  			prev->sched_class->task_dead(prev);
> >> 
> >>   
> >  
> >> a whole bunch of architectures don't in fact need this extra barrier at all.  
> >
> > In fact, I'm fairly sure its only PPC.
> >
> > Because only ARM64 and PPC actually implement ACQUIRE/RELEASE with
> > anything other than smp_mb() (for now, Risc-V is in this same boat and
> > MIPS could be if they ever sort out their fancy barriers).
> >
> > TSO archs use a regular STORE for RELEASE, but all their atomics imply a
> > smp_mb() and there are enough around to make one happen (typically
> > mm_cpumask updates).
> >
> > Everybody else, aside from ARM64 and PPC must use smp_mb() for
> > ACQUIRE/RELEASE.
> >
> > ARM64 has a super duper barrier in switch_to().
> >
> > Which only leaves PPC stranded.. but the 'good' news is that mpe says
> > they'll probably need a barrier in switch_mm() in any case.  
> 
> I may have been sleep deprived. We have a patch, probably soon to be
> merged, which will add a smp_mb() in switch_mm() but *only* when we add
> a CPU to mm_cpumask, ie. when we run on a CPU we haven't run on before.
> 
> I'm not across membarrier enough to know if that's sufficient, but it
> seems unlikely?

Won't be sufficient, they need a barrier after assigning rq->curr.
It can be avoided when switching between threads with the same mm.

I would like to see how bad membarrier performance is if we made
that side heavy enough to avoid the barrier in context switch (e.g.,
by taking the rq locks, or using synchronize_sched_expedited -- on
a per-arch basis of course).

Is there some (realistic-ish) benchmark using membarrier we can
experiment with?

Thanks,
Nick

[toc] | [prev] | [next] | [standalone]

#1700573

----- On Jul 31, 2017, at 8:35 PM, Nicholas Piggin npiggin@gmail.com wrote:

> On Mon, 31 Jul 2017 23:20:59 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
>> Peter Zijlstra <peterz@infradead.org> writes:
>> 
>> > On Fri, Jul 28, 2017 at 10:55:32AM +0200, Peter Zijlstra wrote:
>> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> >> index e9785f7aed75..33f34a201255 100644
>> >> --- a/kernel/sched/core.c
>> >> +++ b/kernel/sched/core.c
>> >> @@ -2641,8 +2641,18 @@ static struct rq *finish_task_switch(struct task_struct
>> >> *prev)
>> >>  	finish_arch_post_lock_switch();
>> >>  
>> >>  	fire_sched_in_preempt_notifiers(current);
>> >> +
>> >> +	/*
>> >> +	 * For CONFIG_MEMBARRIER we need a full memory barrier after the
>> >> +	 * rq->curr assignment. Not all architectures have one in either
>> >> +	 * switch_to() or switch_mm() so we use (and complement) the one
>> >> +	 * implied by mmdrop()'s atomic_dec_and_test().
>> >> +	 */
>> >>  	if (mm)
>> >>  		mmdrop(mm);
>> >> +	else if (IS_ENABLED(CONFIG_MEMBARRIER))
>> >> +		smp_mb();
>> >> +
>> >>  	if (unlikely(prev_state == TASK_DEAD)) {
>> >>  		if (prev->sched_class->task_dead)
>> >>  			prev->sched_class->task_dead(prev);
>> >> 
>> >>   
>> >  
>> >> a whole bunch of architectures don't in fact need this extra barrier at all.
>> >
>> > In fact, I'm fairly sure its only PPC.
>> >
>> > Because only ARM64 and PPC actually implement ACQUIRE/RELEASE with
>> > anything other than smp_mb() (for now, Risc-V is in this same boat and
>> > MIPS could be if they ever sort out their fancy barriers).
>> >
>> > TSO archs use a regular STORE for RELEASE, but all their atomics imply a
>> > smp_mb() and there are enough around to make one happen (typically
>> > mm_cpumask updates).
>> >
>> > Everybody else, aside from ARM64 and PPC must use smp_mb() for
>> > ACQUIRE/RELEASE.
>> >
>> > ARM64 has a super duper barrier in switch_to().
>> >
>> > Which only leaves PPC stranded.. but the 'good' news is that mpe says
>> > they'll probably need a barrier in switch_mm() in any case.
>> 
>> I may have been sleep deprived. We have a patch, probably soon to be
>> merged, which will add a smp_mb() in switch_mm() but *only* when we add
>> a CPU to mm_cpumask, ie. when we run on a CPU we haven't run on before.
>> 
>> I'm not across membarrier enough to know if that's sufficient, but it
>> seems unlikely?
> 
> Won't be sufficient, they need a barrier after assigning rq->curr.
> It can be avoided when switching between threads with the same mm.
> 
> I would like to see how bad membarrier performance is if we made
> that side heavy enough to avoid the barrier in context switch (e.g.,
> by taking the rq locks, or using synchronize_sched_expedited -- on
> a per-arch basis of course).
> 
> Is there some (realistic-ish) benchmark using membarrier we can
> experiment with?

Hi Nick,

I have benchmark programs using membarrier in liburcu [1]. They
represent very heavy usage of membarrier (perhaps more extreme than
reality).

The master branch is at:

git://git.liburcu.org/userspace-rcu.git

Below is a test branch I quickly hacked to make urcu use membarrier
MEMBARRIER_CMD_PRIVATE_EXPEDITED:

https://github.com/compudj/userspace-rcu-dev/tree/test-mb-private

An example benchmark is under tests/benchmark

Usage: test_urcu nr_readers nr_writers duration (s) <OPTIONS>

Here is an example on my 16-core Intel machine (with hyperthreading, so don't
expect this benchmark to be very reliable) with 4 reader threads, 1 writer
thread. The writer thread in this benchmark issues a synchronize_rcu() at
each iteration, so typically 2 sys_membarrier calls. I'm compiling with
the configure flag "--disable-sys-membarrier-fallback" to have the full
read-side speed.

Using MEMBARRIER_CMD_SHARED:

$ ./test_urcu 4 1 10
SUMMARY [...] test_urcu testdur   10 nr_readers   4 rdur      0 wdur      0 nr_writers   1 wdelay      0 nr_reads  17522991852 nr_writes          249 nr_ops  17522992101

438'074'796 reads / reader thread / s
         25 updates / s

Using MEMBARRIER_CMD_PRIVATE_EXPEDITED:

$ ./test_urcu 4 1 10
SUMMARY [...] testdur   10 nr_readers   4 rdur      0 wdur      0 nr_writers   1 wdelay      0 nr_reads  12709653290 nr_writes       799668 nr_ops  12710452958

317'741'332 reads / reader thread / s
     79'966 updates / s

As a comparison, refer to the test_urcu_mb benchmark, which uses
a full memory barrier in rcu_read_lock and rcu_read_unlock, and
can therefore do without any sys_membarrier. This is our fallback
when sys_membarrier returns -ENOSYS.

$ ./test_urcu_mb 4 1 10
SUMMARY [...] test_urcu_mb testdur   10 nr_readers   4 rdur      0 wdur      0 nr_writers   1 wdelay      0 nr_reads    137290634 nr_writes      8624184 nr_ops    145914818

  3'432'265 reads / reader thread / s
    862'418 updates / s

You could try adapting my membarrier private-expedited patch to grab the
rq lock for each CPU to see the impact on the benchmark for your specific
architecture. I suspect that the overhead that will matter in that case
is not only the speed of the membarrier operation, but also its disruption
of the scheduler rq lock cachelines, which will have effect on other unrelated
workloads. You may need to benchmark another scheduler-intensive workload
executed in parallel with heavy use of sys_membarrier adapted to grab rq locks
to figure this one out.

Let me know if you have questions,

Perhaps Dave Watson will have benchmarks on hazard pointers.

Thanks,

Mathieu


[1] http://liburcu.org


> 
> Thanks,
> Nick

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

[toc] | [prev] | [next] | [standalone]

#1700581

On Tue, 1 Aug 2017 01:33:09 +0000 (UTC)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> ----- On Jul 31, 2017, at 8:35 PM, Nicholas Piggin npiggin@gmail.com wrote:
> 
> > On Mon, 31 Jul 2017 23:20:59 +1000
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> >   
> >> Peter Zijlstra <peterz@infradead.org> writes:
> >>   
> >> > On Fri, Jul 28, 2017 at 10:55:32AM +0200, Peter Zijlstra wrote:  
> >> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> >> >> index e9785f7aed75..33f34a201255 100644
> >> >> --- a/kernel/sched/core.c
> >> >> +++ b/kernel/sched/core.c
> >> >> @@ -2641,8 +2641,18 @@ static struct rq *finish_task_switch(struct task_struct
> >> >> *prev)
> >> >>  	finish_arch_post_lock_switch();
> >> >>  
> >> >>  	fire_sched_in_preempt_notifiers(current);
> >> >> +
> >> >> +	/*
> >> >> +	 * For CONFIG_MEMBARRIER we need a full memory barrier after the
> >> >> +	 * rq->curr assignment. Not all architectures have one in either
> >> >> +	 * switch_to() or switch_mm() so we use (and complement) the one
> >> >> +	 * implied by mmdrop()'s atomic_dec_and_test().
> >> >> +	 */
> >> >>  	if (mm)
> >> >>  		mmdrop(mm);
> >> >> +	else if (IS_ENABLED(CONFIG_MEMBARRIER))
> >> >> +		smp_mb();
> >> >> +
> >> >>  	if (unlikely(prev_state == TASK_DEAD)) {
> >> >>  		if (prev->sched_class->task_dead)
> >> >>  			prev->sched_class->task_dead(prev);
> >> >> 
> >> >>     
> >> >    
> >> >> a whole bunch of architectures don't in fact need this extra barrier at all.  
> >> >
> >> > In fact, I'm fairly sure its only PPC.
> >> >
> >> > Because only ARM64 and PPC actually implement ACQUIRE/RELEASE with
> >> > anything other than smp_mb() (for now, Risc-V is in this same boat and
> >> > MIPS could be if they ever sort out their fancy barriers).
> >> >
> >> > TSO archs use a regular STORE for RELEASE, but all their atomics imply a
> >> > smp_mb() and there are enough around to make one happen (typically
> >> > mm_cpumask updates).
> >> >
> >> > Everybody else, aside from ARM64 and PPC must use smp_mb() for
> >> > ACQUIRE/RELEASE.
> >> >
> >> > ARM64 has a super duper barrier in switch_to().
> >> >
> >> > Which only leaves PPC stranded.. but the 'good' news is that mpe says
> >> > they'll probably need a barrier in switch_mm() in any case.  
> >> 
> >> I may have been sleep deprived. We have a patch, probably soon to be
> >> merged, which will add a smp_mb() in switch_mm() but *only* when we add
> >> a CPU to mm_cpumask, ie. when we run on a CPU we haven't run on before.
> >> 
> >> I'm not across membarrier enough to know if that's sufficient, but it
> >> seems unlikely?  
> > 
> > Won't be sufficient, they need a barrier after assigning rq->curr.
> > It can be avoided when switching between threads with the same mm.
> > 
> > I would like to see how bad membarrier performance is if we made
> > that side heavy enough to avoid the barrier in context switch (e.g.,
> > by taking the rq locks, or using synchronize_sched_expedited -- on
> > a per-arch basis of course).
> > 
> > Is there some (realistic-ish) benchmark using membarrier we can
> > experiment with?  
> 
> Hi Nick,
> 
> I have benchmark programs using membarrier in liburcu [1]. They
> represent very heavy usage of membarrier (perhaps more extreme than
> reality).
> 
> The master branch is at:
> 
> git://git.liburcu.org/userspace-rcu.git
> 
> Below is a test branch I quickly hacked to make urcu use membarrier
> MEMBARRIER_CMD_PRIVATE_EXPEDITED:
> 
> https://github.com/compudj/userspace-rcu-dev/tree/test-mb-private
> 
> An example benchmark is under tests/benchmark
> 
> Usage: test_urcu nr_readers nr_writers duration (s) <OPTIONS>
> 
> Here is an example on my 16-core Intel machine (with hyperthreading, so don't
> expect this benchmark to be very reliable) with 4 reader threads, 1 writer
> thread. The writer thread in this benchmark issues a synchronize_rcu() at
> each iteration, so typically 2 sys_membarrier calls. I'm compiling with
> the configure flag "--disable-sys-membarrier-fallback" to have the full
> read-side speed.
> 
> Using MEMBARRIER_CMD_SHARED:
> 
> $ ./test_urcu 4 1 10
> SUMMARY [...] test_urcu testdur   10 nr_readers   4 rdur      0 wdur      0 nr_writers   1 wdelay      0 nr_reads  17522991852 nr_writes          249 nr_ops  17522992101
> 
> 438'074'796 reads / reader thread / s
>          25 updates / s
> 
> Using MEMBARRIER_CMD_PRIVATE_EXPEDITED:
> 
> $ ./test_urcu 4 1 10
> SUMMARY [...] testdur   10 nr_readers   4 rdur      0 wdur      0 nr_writers   1 wdelay      0 nr_reads  12709653290 nr_writes       799668 nr_ops  12710452958
> 
> 317'741'332 reads / reader thread / s
>      79'966 updates / s
> 
> As a comparison, refer to the test_urcu_mb benchmark, which uses
> a full memory barrier in rcu_read_lock and rcu_read_unlock, and
> can therefore do without any sys_membarrier. This is our fallback
> when sys_membarrier returns -ENOSYS.
> 
> $ ./test_urcu_mb 4 1 10
> SUMMARY [...] test_urcu_mb testdur   10 nr_readers   4 rdur      0 wdur      0 nr_writers   1 wdelay      0 nr_reads    137290634 nr_writes      8624184 nr_ops    145914818
> 
>   3'432'265 reads / reader thread / s
>     862'418 updates / s
> 
> You could try adapting my membarrier private-expedited patch to grab the
> rq lock for each CPU to see the impact on the benchmark for your specific
> architecture. I suspect that the overhead that will matter in that case
> is not only the speed of the membarrier operation, but also its disruption
> of the scheduler rq lock cachelines, which will have effect on other unrelated
> workloads. You may need to benchmark another scheduler-intensive workload
> executed in parallel with heavy use of sys_membarrier adapted to grab rq locks
> to figure this one out.
> 
> Let me know if you have questions,

Thanks for this, I'll take a look. This should be a good start as a stress
test, but I'd also be interested in some application. The reason being that
for example using runqueue locks may give reasonable maximum throughput
numbers, but could cause some latency or slowdown when it's used in more
realistic scenario.

We don't have numbers for smp_mb in switch_mm. As you say that should be a
lower overhead, but probably still undesirable. But that's one of the
things I'm looking at.

Will report back after I've tried a few more things.

Thanks,
Nick

[toc] | [prev] | [next] | [standalone]

#1700734

On Tue, Aug 01, 2017 at 12:00:47PM +1000, Nicholas Piggin wrote:
> Thanks for this, I'll take a look. This should be a good start as a stress
> test, but I'd also be interested in some application. The reason being that
> for example using runqueue locks may give reasonable maximum throughput
> numbers, but could cause some latency or slowdown when it's used in more
> realistic scenario.

Given this is an unprivileged interface we have to consider DoS and
other such lovely things.  And since we cannot use mm_cpumask() we're
stuck with for_each_online_cpu().

Combined that means that using rq->lock is completely out of the
question, some numbnut doing 'for (;;) sys_membarrier();' can
completely wreck the system.

Yes, it might work for 'normal' workloads, but the interference
potential is just too big.

I have the same problem with Paul's synchronize_rcu_expedited() patch,
that is a machine wide IPI spray and will interfere with unrelated work.

[toc] | [prev] | [next] | [standalone]

#1700801

On Tue, 1 Aug 2017 10:12:30 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Aug 01, 2017 at 12:00:47PM +1000, Nicholas Piggin wrote:
> > Thanks for this, I'll take a look. This should be a good start as a stress
> > test, but I'd also be interested in some application. The reason being that
> > for example using runqueue locks may give reasonable maximum throughput
> > numbers, but could cause some latency or slowdown when it's used in more
> > realistic scenario.  
> 
> Given this is an unprivileged interface we have to consider DoS and
> other such lovely things.  And since we cannot use mm_cpumask() we're
> stuck with for_each_online_cpu().

I think we *can* make that part of it per-arch, as well as whether
or not to use runqueue locks. It's kind of crazy not to use it when
it's available. Avoiding CPUs you aren't allowed to run on is also
nice for compartmentalization.

> Combined that means that using rq->lock is completely out of the
> question, some numbnut doing 'for (;;) sys_membarrier();' can
> completely wreck the system.

In what way would it wreck the system? It's not holding the lock over
the IPI, only to inspect the rq->curr->mm briefly. 

> Yes, it might work for 'normal' workloads, but the interference
> potential is just too big.

Well it's good to be concerned about it. I do see your point. Although
I don't know if it's all that complicated to use unprivileged ops to
badly hurt QoS on most systems already :)

If mm cpumask is used, I think it's okay. You can cause quite similar
kind of iteration over CPUs and lots of IPIs, tlb flushes, etc using
munmap/mprotect/etc, or context switch IPIs, etc. Are we reaching the
stage where we're controlling those kinds of ops in terms of impact
to the rest of the system? 

> I have the same problem with Paul's synchronize_rcu_expedited() patch,
> that is a machine wide IPI spray and will interfere with unrelated work.

Possibly global IPI would be a more serious concern.

Thanks,
Nick

[toc] | [prev] | [next] | [standalone]

#1700815

On Tue, Aug 01, 2017 at 07:57:17PM +1000, Nicholas Piggin wrote:
> On Tue, 1 Aug 2017 10:12:30 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Tue, Aug 01, 2017 at 12:00:47PM +1000, Nicholas Piggin wrote:
> > > Thanks for this, I'll take a look. This should be a good start as a stress
> > > test, but I'd also be interested in some application. The reason being that
> > > for example using runqueue locks may give reasonable maximum throughput
> > > numbers, but could cause some latency or slowdown when it's used in more
> > > realistic scenario.  
> > 
> > Given this is an unprivileged interface we have to consider DoS and
> > other such lovely things.  And since we cannot use mm_cpumask() we're
> > stuck with for_each_online_cpu().
> 
> I think we *can* make that part of it per-arch, as well as whether
> or not to use runqueue locks. It's kind of crazy not to use it when
> it's available. Avoiding CPUs you aren't allowed to run on is also
> nice for compartmentalization.

Right, but a wee bit hard to do since most of the affinity stuff is per
task, not per process. And that would of course only help with work that
is hard partitioned.

> > Combined that means that using rq->lock is completely out of the
> > question, some numbnut doing 'for (;;) sys_membarrier();' can
> > completely wreck the system.
> 
> In what way would it wreck the system? It's not holding the lock over
> the IPI, only to inspect the rq->curr->mm briefly. 

But you're bouncing the rq->lock around the system at fairly high rates.
For big enough systems this is enough to severely hurt things.

> > Yes, it might work for 'normal' workloads, but the interference
> > potential is just too big.
> 
> Well it's good to be concerned about it. I do see your point. Although
> I don't know if it's all that complicated to use unprivileged ops to
> badly hurt QoS on most systems already :)

If so, we should look at fixing that, not make it worse.

> If mm cpumask is used, I think it's okay. You can cause quite similar
> kind of iteration over CPUs and lots of IPIs, tlb flushes, etc using
> munmap/mprotect/etc, or context switch IPIs, etc. Are we reaching the
> stage where we're controlling those kinds of ops in terms of impact
> to the rest of the system? 

So x86 has a tight mm_cpumask(), we only broadcast TLB invalidate IPIs
to those CPUs actually running threads of our process (or very
recently). So while there can be the sporadic stray IPI for a CPU that
recently ran a thread of the target process, it will not get another one
until it switches back into the process.

On machines that need manual TLB broadcasts and don't keep a tight mask,
yes you can interfere at will, but if they care they can fix by
tightening the mask.

In either case, the mm_cpumask() will be bounded by the set of CPUs the
threads are allowed to run on and will not interfere with the rest of
the system.

As to scheduler IPIs, those are limited to the CPUs the user is limited
to and are rate limited by the wakeup-latency of the tasks. After all,
all the time a task is runnable but not running, wakeups are no-ops.

Trouble is of course, that not everybody even sets a single bit in
mm_cpumask() and those that never clear bits will end up with a fairly
wide mask, still interfering with work that isn't hard partitioned.

[toc] | [prev] | [next] | [standalone]

#1700821

On 08/01/2017 01:22 PM, Peter Zijlstra wrote:
>
>> If mm cpumask is used, I think it's okay. You can cause quite similar
>> kind of iteration over CPUs and lots of IPIs, tlb flushes, etc using
>> munmap/mprotect/etc, or context switch IPIs, etc. Are we reaching the
>> stage where we're controlling those kinds of ops in terms of impact
>> to the rest of the system?
> So x86 has a tight mm_cpumask(), we only broadcast TLB invalidate IPIs
> to those CPUs actually running threads of our process (or very
> recently). So while there can be the sporadic stray IPI for a CPU that
> recently ran a thread of the target process, it will not get another one
> until it switches back into the process.
>
> On machines that need manual TLB broadcasts and don't keep a tight mask,
> yes you can interfere at will, but if they care they can fix by
> tightening the mask.
>
> In either case, the mm_cpumask() will be bounded by the set of CPUs the
> threads are allowed to run on and will not interfere with the rest of
> the system.
>
> As to scheduler IPIs, those are limited to the CPUs the user is limited
> to and are rate limited by the wakeup-latency of the tasks. After all,
> all the time a task is runnable but not running, wakeups are no-ops.
>
> Trouble is of course, that not everybody even sets a single bit in
> mm_cpumask() and those that never clear bits will end up with a fairly
> wide mask, still interfering with work that isn't hard partitioned.

I hate to propose a way to make this more complicated, but this could be 
fixed by a process first declaring its intent to use expedited 
process-wide membarrier; if it does, then every context switch updates a 
process-wide cpumask indicating which cpus are currently running threads 
of that process:

   if (prev->mm != next->mm)
       if (prev->mm->running_cpumask)
              cpumask_clear(...);
       else if (next->mm->running_cpumask)
              cpumask_set(...);

now only processes that want expedited process-wide membarrier pay for 
it (in other than some predictable branches). You can even have threads 
opt-in, so unrelated threads that don't participate in the party don't 
cause those bits to be set.

[toc] | [prev] | [next] | [standalone]

#1700852

On Tue, Aug 01, 2017 at 01:32:43PM +0300, Avi Kivity wrote:
> I hate to propose a way to make this more complicated, but this could be
> fixed by a process first declaring its intent to use expedited process-wide
> membarrier; if it does, then every context switch updates a process-wide
> cpumask indicating which cpus are currently running threads of that process:
> 
>   if (prev->mm != next->mm)
>       if (prev->mm->running_cpumask)
>              cpumask_clear(...);
>       else if (next->mm->running_cpumask)
>              cpumask_set(...);
> 
> now only processes that want expedited process-wide membarrier pay for it
> (in other than some predictable branches). You can even have threads opt-in,
> so unrelated threads that don't participate in the party don't cause those
> bits to be set.

Either that or conditionally put in a smp_mb in switch_mm() for that
process. But yes, once we advertise intent (either explicit or implicit
on first use), there's various things that can be done.

[toc] | [prev] | [next] | [standalone]

#1700834

On Tue, 1 Aug 2017 12:22:03 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Aug 01, 2017 at 07:57:17PM +1000, Nicholas Piggin wrote:
> > On Tue, 1 Aug 2017 10:12:30 +0200
> > Peter Zijlstra <peterz@infradead.org> wrote:
> >   
> > > On Tue, Aug 01, 2017 at 12:00:47PM +1000, Nicholas Piggin wrote:  
> > > > Thanks for this, I'll take a look. This should be a good start as a stress
> > > > test, but I'd also be interested in some application. The reason being that
> > > > for example using runqueue locks may give reasonable maximum throughput
> > > > numbers, but could cause some latency or slowdown when it's used in more
> > > > realistic scenario.    
> > > 
> > > Given this is an unprivileged interface we have to consider DoS and
> > > other such lovely things.  And since we cannot use mm_cpumask() we're
> > > stuck with for_each_online_cpu().  
> > 
> > I think we *can* make that part of it per-arch, as well as whether
> > or not to use runqueue locks. It's kind of crazy not to use it when
> > it's available. Avoiding CPUs you aren't allowed to run on is also
> > nice for compartmentalization.  
> 
> Right, but a wee bit hard to do since most of the affinity stuff is per
> task, not per process. And that would of course only help with work that
> is hard partitioned.
> 
> > > Combined that means that using rq->lock is completely out of the
> > > question, some numbnut doing 'for (;;) sys_membarrier();' can
> > > completely wreck the system.  
> > 
> > In what way would it wreck the system? It's not holding the lock over
> > the IPI, only to inspect the rq->curr->mm briefly.   
> 
> But you're bouncing the rq->lock around the system at fairly high rates.
> For big enough systems this is enough to severely hurt things.

If you already have scheduling access on those CPUs to set mm cpumask,
then you can do worse I guess.

> 
> > > Yes, it might work for 'normal' workloads, but the interference
> > > potential is just too big.  
> > 
> > Well it's good to be concerned about it. I do see your point. Although
> > I don't know if it's all that complicated to use unprivileged ops to
> > badly hurt QoS on most systems already :)  
> 
> If so, we should look at fixing that, not make it worse.

Point is I don't think this *is* worse. It's not too difficult to acquire
per-cpu locks and cachelines and IPIs to other CPUs if you are a malicious
DoS. Particularly not when you can scheudle tasks around those CPUs.

> > If mm cpumask is used, I think it's okay. You can cause quite similar
> > kind of iteration over CPUs and lots of IPIs, tlb flushes, etc using
> > munmap/mprotect/etc, or context switch IPIs, etc. Are we reaching the
> > stage where we're controlling those kinds of ops in terms of impact
> > to the rest of the system?   
> 
> So x86 has a tight mm_cpumask(), we only broadcast TLB invalidate IPIs
> to those CPUs actually running threads of our process (or very
> recently). So while there can be the sporadic stray IPI for a CPU that
> recently ran a thread of the target process, it will not get another one
> until it switches back into the process.
> 
> On machines that need manual TLB broadcasts and don't keep a tight mask,
> yes you can interfere at will, but if they care they can fix by
> tightening the mask.
> 
> In either case, the mm_cpumask() will be bounded by the set of CPUs the
> threads are allowed to run on and will not interfere with the rest of
> the system.

Yep. Powerpc at least will use its mm_cpumask for iterating here
(regardless of how exactly we get the ordering right). Even though
at the moment it is a lazy mask, it's better than a global walk.

> As to scheduler IPIs, those are limited to the CPUs the user is limited
> to and are rate limited by the wakeup-latency of the tasks. After all,
> all the time a task is runnable but not running, wakeups are no-ops.

That's on the order of millions per second per core though, isn't it?

> 
> Trouble is of course, that not everybody even sets a single bit in
> mm_cpumask() and those that never clear bits will end up with a fairly
> wide mask, still interfering with work that isn't hard partitioned.

Right, I just don't see what real problem this opens up that you don't
already have when you are not hard partitioned, therefore it doesn't
make sense to add a slowdown to the context switch fastpath to close
one hole in the sieve.

Completely recognizing that other architectures can do it without
taking rq lock at all and will not be forced to do so.

[toc] | [prev] | [next] | [standalone]

#1700869

On Tue, Aug 01, 2017 at 08:39:28PM +1000, Nicholas Piggin wrote:
> On Tue, 1 Aug 2017 12:22:03 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:

> > But you're bouncing the rq->lock around the system at fairly high rates.
> > For big enough systems this is enough to severely hurt things.
> 
> If you already have scheduling access on those CPUs to set mm cpumask,
> then you can do worse I guess.

Ah, so currently, because of ARM64, we have to use for_each_online_cpu()
and then we're not limited to any affinity mask and will wreck across
partitioning.

If ARM64 starts setting bits in mm_cpumask().. then yes.

> > > > Yes, it might work for 'normal' workloads, but the interference
> > > > potential is just too big.  
> > > 
> > > Well it's good to be concerned about it. I do see your point. Although
> > > I don't know if it's all that complicated to use unprivileged ops to
> > > badly hurt QoS on most systems already :)  
> > 
> > If so, we should look at fixing that, not make it worse.
> 
> Point is I don't think this *is* worse. It's not too difficult to acquire
> per-cpu locks and cachelines and IPIs to other CPUs if you are a malicious
> DoS. Particularly not when you can scheudle tasks around those CPUs.

So we try and not take remote rq->locks for wakeups when those locks are
outside of the cache domain.

> > In either case, the mm_cpumask() will be bounded by the set of CPUs the
> > threads are allowed to run on and will not interfere with the rest of
> > the system.
> 
> Yep. Powerpc at least will use its mm_cpumask for iterating here
> (regardless of how exactly we get the ordering right). Even though
> at the moment it is a lazy mask, it's better than a global walk.

Agreed.

> > As to scheduler IPIs, those are limited to the CPUs the user is limited
> > to and are rate limited by the wakeup-latency of the tasks. After all,
> > all the time a task is runnable but not running, wakeups are no-ops.
> 
> That's on the order of millions per second per core though, isn't it?

Thousands, assuming another process is actually running. We'll not
always win the wakeup-preempt race. And once we're queued, no more
wakeups until we get to run.

> > Trouble is of course, that not everybody even sets a single bit in
> > mm_cpumask() and those that never clear bits will end up with a fairly
> > wide mask, still interfering with work that isn't hard partitioned.
> 
> Right, I just don't see what real problem this opens up that you don't
> already have when you are not hard partitioned, therefore it doesn't
> make sense to add a slowdown to the context switch fastpath to close
> one hole in the sieve.
> 
> Completely recognizing that other architectures can do it without
> taking rq lock at all and will not be forced to do so.

If we can limit this to hard partitioned, that would be good indeed.

I'm just trying to avoid having two implementation of this thing. At the
same time I very much understand your reluctance to add this barrier.

In any case, supposing we can do that intent thing. How horrible would
something like:


  context_switch()
    if (unlikely(mm->needs_barrier))
      smp_mb__after_unlock_lock();


be? We only need the extra barrier when we switch _into_ mm's that care
about sys_membarrier() in the first place. At which point they pay the
price.

[toc] | [prev] | [next] | [standalone]

#1700934

On Tue, 1 Aug 2017 13:00:23 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Aug 01, 2017 at 08:39:28PM +1000, Nicholas Piggin wrote:

> > Right, I just don't see what real problem this opens up that you don't
> > already have when you are not hard partitioned, therefore it doesn't
> > make sense to add a slowdown to the context switch fastpath to close
> > one hole in the sieve.
> > 
> > Completely recognizing that other architectures can do it without
> > taking rq lock at all and will not be forced to do so.  
> 
> If we can limit this to hard partitioned, that would be good indeed.
> 
> I'm just trying to avoid having two implementation of this thing. At the
> same time I very much understand your reluctance to add this barrier.

Well I think we could have some kind of
for_each_cpu_where_this_process_is_running macro that is needed to
abstract the arch details.

Presumably we're already going to get two implementations of that one --
I can't imagine x86 would be happy with doing a for_all_cpus iteration
just because arm does not have the cpumask. powerpc will only make that
3 :)

> 
> In any case, supposing we can do that intent thing. How horrible would
> something like:
> 
> 
>   context_switch()
>     if (unlikely(mm->needs_barrier))
>       smp_mb__after_unlock_lock();
> 
> 
> be? We only need the extra barrier when we switch _into_ mm's that care
> about sys_membarrier() in the first place. At which point they pay the
> price.

Not beautiful :) and it would also have to have an arch speicific bit on
the other side. Although yes it gives a different way to reduce cost without
rq.

So Paul and googling filled me in on the importance of this syscall. Also
I do appreciate the concern about taking rq lock. I just think maybe we
(powerpc) pay a few more cycles in the new syscall rather than context
switch. It will take a little while to get a good idea of performance and
behaviour on bigger systems where this will matter most.

[toc] | [prev] | [next] | [standalone]

#1701031

On Tue, Aug 01, 2017 at 12:22:03PM +0200, Peter Zijlstra wrote:

[ . . . ]

> As to scheduler IPIs, those are limited to the CPUs the user is limited
> to and are rate limited by the wakeup-latency of the tasks. After all,
> all the time a task is runnable but not running, wakeups are no-ops.

Can't that wakeup-latency limitation be overcome by a normal user simply
by having lots of tasks to wake up, which then go back to sleep almost
immediately?  Coupled with very a low-priority CPU-bound task on each CPU?

							Thanx, Paul

[toc] | [prev] | [next] | [standalone]

#1701102

On Tue, Aug 01, 2017 at 06:23:09AM -0700, Paul E. McKenney wrote:
> On Tue, Aug 01, 2017 at 12:22:03PM +0200, Peter Zijlstra wrote:
> 
> [ . . . ]
> 
> > As to scheduler IPIs, those are limited to the CPUs the user is limited
> > to and are rate limited by the wakeup-latency of the tasks. After all,
> > all the time a task is runnable but not running, wakeups are no-ops.
> 
> Can't that wakeup-latency limitation be overcome by a normal user simply
> by having lots of tasks to wake up, which then go back to sleep almost
> immediately?  Coupled with very a low-priority CPU-bound task on each CPU?

Let me put it like this; there is no way to cause more interference
using IPIs then there is simply running while(1) loops ;-)

[toc] | [prev] | [next] | [standalone]

#1701587

On Tue, Aug 01, 2017 at 04:16:54PM +0200, Peter Zijlstra wrote:
> On Tue, Aug 01, 2017 at 06:23:09AM -0700, Paul E. McKenney wrote:
> > On Tue, Aug 01, 2017 at 12:22:03PM +0200, Peter Zijlstra wrote:
> > 
> > [ . . . ]
> > 
> > > As to scheduler IPIs, those are limited to the CPUs the user is limited
> > > to and are rate limited by the wakeup-latency of the tasks. After all,
> > > all the time a task is runnable but not running, wakeups are no-ops.
> > 
> > Can't that wakeup-latency limitation be overcome by a normal user simply
> > by having lots of tasks to wake up, which then go back to sleep almost
> > immediately?  Coupled with very a low-priority CPU-bound task on each CPU?
> 
> Let me put it like this; there is no way to cause more interference
> using IPIs then there is simply running while(1) loops ;-)

Very good, that does give us some guidance, give or take context switches
happening during the IPI latency window.  ;-)

							Thanx, Paul

[toc] | [prev] | [next] | [standalone]

#1701608

On Tue, 1 Aug 2017 16:32:03 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Tue, Aug 01, 2017 at 04:16:54PM +0200, Peter Zijlstra wrote:
> > On Tue, Aug 01, 2017 at 06:23:09AM -0700, Paul E. McKenney wrote:  
> > > On Tue, Aug 01, 2017 at 12:22:03PM +0200, Peter Zijlstra wrote:
> > > 
> > > [ . . . ]
> > >   
> > > > As to scheduler IPIs, those are limited to the CPUs the user is limited
> > > > to and are rate limited by the wakeup-latency of the tasks. After all,
> > > > all the time a task is runnable but not running, wakeups are no-ops.  
> > > 
> > > Can't that wakeup-latency limitation be overcome by a normal user simply
> > > by having lots of tasks to wake up, which then go back to sleep almost
> > > immediately?  Coupled with very a low-priority CPU-bound task on each CPU?  
> > 
> > Let me put it like this; there is no way to cause more interference
> > using IPIs then there is simply running while(1) loops ;-)  
> 
> Very good, that does give us some guidance, give or take context switches
> happening during the IPI latency window.  ;-)

I think we do have to be a bit careful. Peter's right when you're thinking
of just running arbitrary tasks on a single CPU, but for multiple CPUs, the
IPI sender will not necessarily get accounted the cost it incurs on the
target CPU.

So we do need to be careful about allowing a large amount of unprivileged
IPIs to arbitrary CPUs. Fortunately in this case the IPIs are restricted to
CPUs where our process is currently running. That's about the ideal case
where we're only disturbing our own job.

Thanks,
Nick

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web