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

Groups > linux.kernel > #1737152

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

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Fri, Sep 22, 2017 at 11:22:06AM +0800, Boqun Feng wrote:
> Hi Mathieu,
> 
> On Tue, Sep 19, 2017 at 06:13:41PM -0400, 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.
> > 
> 
> Sorry I'm late for the party, but I couldn't stop thinking whether we
> could avoid the register thing at all, because the registering makes
> sys_membarrier() more complex(both for the interface and the
> implementation). So how about we trade-off a little bit by taking
> some(not all) the rq->locks?
> 
> The idea is in membarrier_private_expedited(), we go through all ->curr
> on each CPU and 
> 
> 1)	If it's a userspace task and its ->mm is matched, we send an ipi
> 
> 2)	If it's a kernel task, we skip
> 
> 	(Because there will be a smp_mb() implied by mmdrop(), when it
> 	switchs to userspace task).
> 
> 3)	If it's a userspace task and its ->mm is not matched, we take
> 	the corresponding rq->lock and check rq->curr again, if its ->mm
> 	matched, we send an ipi, otherwise we do nothing.
> 
> 	(Because if we observe rq->curr is not matched with rq->lock
> 	held, when a task having matched ->mm schedules in, the rq->lock
> 	pairing along with the smp_mb__after_spinlock() will guarantee
> 	it observes all memory ops before sys_membarrir()).
> 
> membarrier_private_expedited() will look like this if we choose this
> way:
> 
> void membarrier_private_expedited()
> {
> 	int cpu;
> 	bool fallback = false;
> 	cpumask_var_t tmpmask;
> 	struct rq_flags rf;
> 
> 
> 	if (num_online_cpus() == 1)
> 		return;
> 
> 	smp_mb();
> 
> 	if (!zalloc_cpumask_var(&tmpmask, GFP_NOWAIT)) {
> 		/* Fallback for OOM. */
> 		fallback = true;
> 	}
> 
> 	cpus_read_lock();
> 	for_each_online_cpu(cpu) {
> 		struct task_struct *p;
> 
> 		if (cpu == raw_smp_processor_id())
> 			continue;
> 
> 		rcu_read_lock();
> 		p = task_rcu_dereference(&cpu_rq(cpu)->curr);
> 
> 		if (!p) {
> 			rcu_read_unlock();
> 			continue;
> 		}
> 
> 		if (p->mm == current->mm) {
> 			if (!fallback)
> 				__cpumask_set_cpu(cpu, tmpmask);
> 			else
> 				smp_call_function_single(cpu, ipi_mb, NULL, 1);
> 		}
> 
> 		if (p->mm == current->mm || !p->mm) {
> 			rcu_read_unlock();
> 			continue;
> 		}
> 
> 		rcu_read_unlock();
> 		
> 		/*
> 		 * This should be a arch-specific code, as we don't
> 		 * need it at else place other than some archs without
> 		 * a smp_mb() in switch_mm() (i.e. powerpc)
> 		 */
> 		rq_lock_irq(cpu_rq(cpu), &rf);
> 		if (p->mm == current->mm) {

Oops, this one should be

		if (cpu_curr(cpu)->mm == current->mm)

> 			if (!fallback)
> 				__cpumask_set_cpu(cpu, tmpmask);
> 			else
> 				smp_call_function_single(cpu, ipi_mb, NULL, 1);

, and this better be moved out of the lock rq->lock critical section.

Regards,
Boqun

> 		}
> 		rq_unlock_irq(cpu_rq(cpu), &rf);
> 	}
> 	if (!fallback) {
> 		smp_call_function_many(tmpmask, ipi_mb, NULL, 1);
> 		free_cpumask_var(tmpmask);
> 	}
> 	cpus_read_unlock();
> 
> 	smp_mb();
> }
> 
> Thoughts?
> 
> Regards,
> Boqun
> 
[...]

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC PATCH v3 1/2] membarrier: Provide register expedited private command Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-09-20 00:20 +0200
  [RFC PATCH 2/2] membarrier: selftest: Test private expedited cmd Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-09-20 00:20 +0200
  Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Boqun Feng <boqun.feng@gmail.com> - 2017-09-22 05:30 +0200
    Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Boqun Feng <boqun.feng@gmail.com> - 2017-09-22 05:40 +0200
      Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-09-22 07:30 +0200
    Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Peter Zijlstra <peterz@infradead.org> - 2017-09-22 10:30 +0200
      Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Boqun Feng <boqun.feng@gmail.com> - 2017-09-22 11:00 +0200
  Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Boqun Feng <boqun.feng@gmail.com> - 2017-09-22 11:10 +0200
    Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-09-22 17:10 +0200
      Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Boqun Feng <boqun.feng@gmail.com> - 2017-09-24 15:40 +0200
        Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-09-24 16:30 +0200
          Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Boqun Feng <boqun.feng@gmail.com> - 2017-09-25 14:20 +0200
            Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Peter Zijlstra <peterz@infradead.org> - 2017-09-25 14:30 +0200
              Re: [RFC PATCH v3 1/2] membarrier: Provide register expedited  private command Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2017-09-25 14:50 +0200

csiph-web