Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 6 Aug 2020 13:37:05 -0400 (EDT)
From:   Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To:     Peter Oskolkov <posk@posk.io>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Peter Oskolkov <posk@google.com>, paulmck <paulmck@kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Paul Turner <pjt@google.com>,
        Chris Kennelly <ckennelly@google.com>
Message-ID: <1668913120.1621.1596735425601.JavaMail.zimbra@efficios.com>
In-Reply-To: <CAFTs51Ufyds5mypiysL=2ZwtyMk9hjLm-9tJbYz=xciY1f2=5g@mail.gmail.com>
References: <20200806000859.160882-1-posk@google.com> <20200806134828.GA165568@hirez.programming.kicks-ass.net> <CAFTs51Ufyds5mypiysL=2ZwtyMk9hjLm-9tJbYz=xciY1f2=5g@mail.gmail.com>
Subject: Re: [PATCH 1/2] membarrier: add
 MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: membarrier: add MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU
Thread-Index: S/ODafaL6Qc5NbUukMlKTrc5xZy9ZQ==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

----- On Aug 6, 2020, at 1:07 PM, Peter Oskolkov posk@posk.io wrote:

> On Thu, Aug 6, 2020 at 6:48 AM <peterz@infradead.org> wrote:
>>
>> On Wed, Aug 05, 2020 at 05:08:58PM -0700, Peter Oskolkov wrote:
>>
>> Thanks for the Cc!
> 
> Always a pleasure!
> 
> (Sorry, included only membarrier maintainers in v1; in v2 included
> both membarrier and rseq maintainers).
> 
>>
>> > + * @MEMBARRIER_CMD_PRIVATE_RESTART_RSEQ_ON_CPU:
>> > + *                          If a thread belonging to the current process
>> > + *                          is currently in an RSEQ critical section on the
>> > + *                          CPU identified by flags parameter, restart it.
>> > + *                          @flags: if @flags >= 0, identifies the CPU to
>> > + *                                  restart RSEQ CS on; if == -1, restarts
>> > + *                                  RSEQ CSs on all CPUs.
>>
>> > +     } else if (cpu_id == -1) {
>> > +             on_each_cpu(membarrier_rseq_ipi,
>> > +                         current->group_leader, true);
>>
>> This is an unpriv IPI the world. That's a big no-no.
> 
> removed in v2.

I don't think the feature must be removed, but its implementation needs adjustment.

How about we simply piggy-back on the membarrier schemes we already have, and
implement:

membarrier_register_private_expedited(MEMBARRIER_FLAG_RSEQ)
membarrier_private_expedited(MEMBARRIER_FLAG_RSEQ)

All the logic is there to prevent sending IPIs to runqueues which are not running
threads associated with the same mm. Considering that preemption does an rseq abort,
running a thread belonging to a different mm should mean that this CPU is not
currently executing an rseq critical section, or if it was, it has already been
aborted, so it is quiescent.

Then you'll probably want to change membarrier_private_expedited so it takes an
extra "cpu" argument. If cpu=-1, iterate on all runqueues like we currently do.
If cpu >= 0, only IPI that CPU if the thread currently running has the same mm.

Also, should this belong to the membarrier or the rseq system call ? It just
looks like the membarrier happens to implement very similar things for barriers,
but arguably this is really about rseq. I wonder if we should expose this through
rseq instead, even if we end up using membarrier code.

Thanks,

Mathieu

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