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

Groups > linux.kernel > #1738975

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 Sun, Sep 24, 2017 at 02:23:04PM +0000, Mathieu Desnoyers wrote:
[...]
> >> 
> >> copy_mm() is performed without holding current->sighand->siglock, so
> >> it appears to be racing with concurrent membarrier register cmd.
> > 
> > Speak of racing, I think we currently have a problem if we do a
> > register_private_expedited in one thread and do a
> > membarrer_private_expedited in another thread(sharing the same mm), as
> > follow:
> > 
> >	{t1,t2,t3 sharing the same ->mm}
> >	CPU 0				CPU 1				CPU2
> >	====================		===================		============
> >	{in thread t1}
> >	membarrier_register_private_expedited():
> >	  ...
> >	  WRITE_ONCE(->mm->membarrier_private_expedited, 1);
> >	  membarrier_arch_register_private_expedited():
> >	    ...
> >	    <haven't set the TIF for t3 yet>
> > 
> >	    				{in thread t2}
> >					membarrier_private_expedited():
> >					  READ_ONCE(->mm->membarrier_private_expedited); // == 1
> >					  ...
> >					  for_each_online_cpu()
> >					    ...
> >					    <p is cpu_rq(CPU2)->curr>
> >					    if (p && p->mm == current->mm) // false
> >					    <so no ipi sent to CPU2>
> >					    				
> >									{about to switch to t3}
> >									rq->curr = t3;
> >									....
> >									context_switch():
> >									  ...
> >									  finish_task_swtich():
> >									    membarrier_sched_in():
> >									    <TIF is not set>
> >									    // no smp_mb() here.
> > 
> > , and we will miss the smp_mb() on CPU2, right? And this could even
> > happen if t2 has a membarrier_register_private_expedited() preceding the
> > membarrier_private_expedited().
> >					
> > Am I missing something subtle here?
> 
> I think the problem sits in this function:
> 
> static void membarrier_register_private_expedited(void)
> {
>         struct task_struct *p = current;
> 
>         if (READ_ONCE(p->mm->membarrier_private_expedited))
>                 return;
>         WRITE_ONCE(p->mm->membarrier_private_expedited, 1);
>         membarrier_arch_register_private_expedited(p);
> }
> 
> I need to change the order between WRITE_ONCE() and invoking
> membarrier_arch_register_private_expedited. If I issue the
> WRITE_ONCE after the arch code (which sets the TIF flags),
> then concurrent membarrier priv exped commands will simply
> return an -EPERM error:
> 
> static void membarrier_register_private_expedited(void)
> {
>         struct task_struct *p = current;
> 
>         if (READ_ONCE(p->mm->membarrier_private_expedited))
>                 return;
>         membarrier_arch_register_private_expedited(p);
>         WRITE_ONCE(p->mm->membarrier_private_expedited, 1);
> }
> 
> Do you agree that this would fix the race you identified ?
> 

Yep, that will do the trick ;-)

Regards,
Boqun

> Thanks,
> 
> Mathieu
> 

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