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


Groups > linux.kernel > #1524129 > unrolled thread

Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled

Started byLai Jiangshan <jiangshanlai@gmail.com>
First post2016-11-17 08:00 +0100
Last post2016-11-17 18:30 +0100
Articles 10 — 4 participants

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: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled Lai Jiangshan <jiangshanlai@gmail.com> - 2016-11-17 08:00 +0100
    Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-11-17 12:50 +0100
      Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-11-17 15:40 +0100
        Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled Steven Rostedt <rostedt@goodmis.org> - 2016-11-17 18:20 +0100
          Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2016-11-17 18:30 +0100
            Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled Steven Rostedt <rostedt@goodmis.org> - 2016-11-17 18:50 +0100
              Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-17 19:20 +0100
        Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-17 19:50 +0100
      Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-17 20:30 +0100
    Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2016-11-17 18:30 +0100

#1524129 — Re: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled

FromLai Jiangshan <jiangshanlai@gmail.com>
Date2016-11-17 08:00 +0100
SubjectRe: [PATCH] Fix: disable sys_membarrier when nohz_full is enabled
Message-ID<sEoPT-5Cb-5@gated-at.bofh.it>
On Fri, Nov 4, 2016 at 12:29 AM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> Userspace applications should be allowed to expect the membarrier system
> call with MEMBARRIER_CMD_SHARED command to issue memory barriers on
> nohz_full CPUs, but synchronize_sched() does not take those into
> account.
>
> Given that we do not want unrelated processes to be able to affect
> real-time sensitive nohz_full CPUs, simply return ENOSYS when membarrier
> is invoked on a kernel with enabled nohz_full CPUs.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> CC: Josh Triplett <josh@joshtriplett.org>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Lai Jiangshan <jiangshanlai@gmail.com>
> CC: <stable@vger.kernel.org>    [3.10+]
> ---
>  kernel/membarrier.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/kernel/membarrier.c b/kernel/membarrier.c
> index 536c727..9f9284f 100644
> --- a/kernel/membarrier.c
> +++ b/kernel/membarrier.c
> @@ -16,6 +16,7 @@
>
>  #include <linux/syscalls.h>
>  #include <linux/membarrier.h>
> +#include <linux/tick.h>
>
>  /*
>   * Bitmask made from a "or" of all commands within enum membarrier_cmd,
> @@ -51,6 +52,9 @@
>   */
>  SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
>  {
> +       /* MEMBARRIER_CMD_SHARED is not compatible with nohz_full. */
> +       if (tick_nohz_full_enabled())
> +               return -ENOSYS;

I guess this code needs to be moved down into the branch of
"case MEMBARRIER_CMD_SHARED" to match its comment.

Acked-by: Lai Jiangshan <jiangshanlai@gmail.com>

But I'm afraid, in the future, tick_nohz_full will become a default y
feature. thus it makes sys_membarrier() always disabled. we might
need a new MEMBARRIER_CMD_XXX to handle it?

thanks,
Lai

[toc] | [next] | [standalone]


#1524406

FromMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date2016-11-17 12:50 +0100
Message-ID<sEtmy-7P-23@gated-at.bofh.it>
In reply to#1524129
----- On Nov 17, 2016, at 1:51 AM, Lai Jiangshan jiangshanlai@gmail.com wrote:

> On Fri, Nov 4, 2016 at 12:29 AM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>> Userspace applications should be allowed to expect the membarrier system
>> call with MEMBARRIER_CMD_SHARED command to issue memory barriers on
>> nohz_full CPUs, but synchronize_sched() does not take those into
>> account.
>>
>> Given that we do not want unrelated processes to be able to affect
>> real-time sensitive nohz_full CPUs, simply return ENOSYS when membarrier
>> is invoked on a kernel with enabled nohz_full CPUs.
>>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>> CC: Josh Triplett <josh@joshtriplett.org>
>> CC: Steven Rostedt <rostedt@goodmis.org>
>> CC: Lai Jiangshan <jiangshanlai@gmail.com>
>> CC: <stable@vger.kernel.org>    [3.10+]
>> ---
>>  kernel/membarrier.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/membarrier.c b/kernel/membarrier.c
>> index 536c727..9f9284f 100644
>> --- a/kernel/membarrier.c
>> +++ b/kernel/membarrier.c
>> @@ -16,6 +16,7 @@
>>
>>  #include <linux/syscalls.h>
>>  #include <linux/membarrier.h>
>> +#include <linux/tick.h>
>>
>>  /*
>>   * Bitmask made from a "or" of all commands within enum membarrier_cmd,
>> @@ -51,6 +52,9 @@
>>   */
>>  SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
>>  {
>> +       /* MEMBARRIER_CMD_SHARED is not compatible with nohz_full. */
>> +       if (tick_nohz_full_enabled())
>> +               return -ENOSYS;
> 
> I guess this code needs to be moved down into the branch of
> "case MEMBARRIER_CMD_SHARED" to match its comment.

No, that would be unexpected from user-space. Either a system
call is implemented or not, not "implemented for some parameters".

We also want MEMBARRIER_CMD_QUERY to return -ENOSYS in this case,
and all other parameter values to also return -ENOSYS (rather than
-EINVAL).

If a system call that returns successfully on CMD_QUERY or EINVAL,
user-space may assume it will not have to handle ENOSYS in the
next calls.


> 
> Acked-by: Lai Jiangshan <jiangshanlai@gmail.com>
> 
> But I'm afraid, in the future, tick_nohz_full will become a default y
> feature. thus it makes sys_membarrier() always disabled. we might
> need a new MEMBARRIER_CMD_XXX to handle it?

This may require that we send an IPI to nohz_full CPUs, which will
disturb them real-time wise. Any better ideas ?

Thanks,

Mathieu

> 
> thanks,
> Lai

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

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


#1524461

FromMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date2016-11-17 15:40 +0100
Message-ID<sEw15-1T0-67@gated-at.bofh.it>
In reply to#1524406
----- On Nov 17, 2016, at 8:40 AM, Paul E. McKenney paulmck@linux.vnet.ibm.com wrote:

> On Thu, Nov 17, 2016 at 11:46:34AM +0000, Mathieu Desnoyers wrote:
>> ----- On Nov 17, 2016, at 1:51 AM, Lai Jiangshan jiangshanlai@gmail.com wrote:
>> 
>> > On Fri, Nov 4, 2016 at 12:29 AM, Mathieu Desnoyers
>> > <mathieu.desnoyers@efficios.com> wrote:
>> >> Userspace applications should be allowed to expect the membarrier system
>> >> call with MEMBARRIER_CMD_SHARED command to issue memory barriers on
>> >> nohz_full CPUs, but synchronize_sched() does not take those into
>> >> account.
>> >>
>> >> Given that we do not want unrelated processes to be able to affect
>> >> real-time sensitive nohz_full CPUs, simply return ENOSYS when membarrier
>> >> is invoked on a kernel with enabled nohz_full CPUs.
>> >>
>> >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> >> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>> >> CC: Josh Triplett <josh@joshtriplett.org>
>> >> CC: Steven Rostedt <rostedt@goodmis.org>
>> >> CC: Lai Jiangshan <jiangshanlai@gmail.com>
>> >> CC: <stable@vger.kernel.org>    [3.10+]
>> >> ---
>> >>  kernel/membarrier.c | 4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >>
>> >> diff --git a/kernel/membarrier.c b/kernel/membarrier.c
>> >> index 536c727..9f9284f 100644
>> >> --- a/kernel/membarrier.c
>> >> +++ b/kernel/membarrier.c
>> >> @@ -16,6 +16,7 @@
>> >>
>> >>  #include <linux/syscalls.h>
>> >>  #include <linux/membarrier.h>
>> >> +#include <linux/tick.h>
>> >>
>> >>  /*
>> >>   * Bitmask made from a "or" of all commands within enum membarrier_cmd,
>> >> @@ -51,6 +52,9 @@
>> >>   */
>> >>  SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
>> >>  {
>> >> +       /* MEMBARRIER_CMD_SHARED is not compatible with nohz_full. */
>> >> +       if (tick_nohz_full_enabled())
>> >> +               return -ENOSYS;
>> > 
>> > I guess this code needs to be moved down into the branch of
>> > "case MEMBARRIER_CMD_SHARED" to match its comment.
>> 
>> No, that would be unexpected from user-space. Either a system
>> call is implemented or not, not "implemented for some parameters".
>> 
>> We also want MEMBARRIER_CMD_QUERY to return -ENOSYS in this case,
>> and all other parameter values to also return -ENOSYS (rather than
>> -EINVAL).
>> 
>> If a system call that returns successfully on CMD_QUERY or EINVAL,
>> user-space may assume it will not have to handle ENOSYS in the
>> next calls.
>> 
>> 
>> > 
>> > Acked-by: Lai Jiangshan <jiangshanlai@gmail.com>
>> > 
>> > But I'm afraid, in the future, tick_nohz_full will become a default y
>> > feature. thus it makes sys_membarrier() always disabled. we might
>> > need a new MEMBARRIER_CMD_XXX to handle it?
>> 
>> This may require that we send an IPI to nohz_full CPUs, which will
>> disturb them real-time wise. Any better ideas ?
> 
> Restrict the IPIs to CPUs running the process executing the
> sys_membarrier() system call.  This would mean that CPUs only
> are interrupted by their own application's request.

This would break use-cases of cross-process shared memory. :-(

Mathieu


> 
> 							Thanx, Paul

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

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


#1524511

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-11-17 18:20 +0100
Message-ID<sEyvT-3yz-1@gated-at.bofh.it>
In reply to#1524461
On Thu, 17 Nov 2016 13:54:27 +0000 (UTC)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

 
> >> > 
> >> > Acked-by: Lai Jiangshan <jiangshanlai@gmail.com>
> >> > 
> >> > But I'm afraid, in the future, tick_nohz_full will become a default y
> >> > feature. thus it makes sys_membarrier() always disabled. we might
> >> > need a new MEMBARRIER_CMD_XXX to handle it?  
> >> 
> >> This may require that we send an IPI to nohz_full CPUs, which will
> >> disturb them real-time wise. Any better ideas ?  
> > 
> > Restrict the IPIs to CPUs running the process executing the
> > sys_membarrier() system call.  This would mean that CPUs only
> > are interrupted by their own application's request.  
> 
> This would break use-cases of cross-process shared memory. :-(

Perhaps make this an opt in. That is, all processes that want to be
affected by this can call this function with some flag that sets a flag
in tasks struct. And have that process get an IPI even in no-hz-full
mode if it asked to do it.

-- Steve

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


#1524551

FromMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date2016-11-17 18:30 +0100
Message-ID<sEyFz-3CT-43@gated-at.bofh.it>
In reply to#1524511
----- On Nov 17, 2016, at 9:50 AM, rostedt rostedt@goodmis.org wrote:

> On Thu, 17 Nov 2016 13:54:27 +0000 (UTC)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
> 
>> >> > 
>> >> > Acked-by: Lai Jiangshan <jiangshanlai@gmail.com>
>> >> > 
>> >> > But I'm afraid, in the future, tick_nohz_full will become a default y
>> >> > feature. thus it makes sys_membarrier() always disabled. we might
>> >> > need a new MEMBARRIER_CMD_XXX to handle it?
>> >> 
>> >> This may require that we send an IPI to nohz_full CPUs, which will
>> >> disturb them real-time wise. Any better ideas ?
>> > 
>> > Restrict the IPIs to CPUs running the process executing the
>> > sys_membarrier() system call.  This would mean that CPUs only
>> > are interrupted by their own application's request.
>> 
>> This would break use-cases of cross-process shared memory. :-(
> 
> Perhaps make this an opt in. That is, all processes that want to be
> affected by this can call this function with some flag that sets a flag
> in tasks struct. And have that process get an IPI even in no-hz-full
> mode if it asked to do it.

That's an interesting approach. I would be tempted to give it a
per-thread (rather than per-process) scope.

E.g., a thread could do the following to ask to be
interrupted by IPIs:

membarrier(MEMBARRIER_CMD_REGISTER_EXPEDITED, 0)

and could unregister with:

membarrier(MEMBARRIER_CMD_UNREGISTER_EXPEDITED, 0)

We can then keep a per-thread refcount internally.
(not sure the "EXPEDITED" is the right word there...
do we want it to be "NOHZ_FULL" instead ?)

Then in membarrier(MEMBARRIER_CMD_SHARED, 0), for each
nohz_full cpu, we grab the rq lock, and only send an IPI
if the running thread is registered as "expedited".

Thoughts ?

Thanks,

Mathieu

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

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


#1524597

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-11-17 18:50 +0100
Message-ID<sEyYV-3Mb-1@gated-at.bofh.it>
In reply to#1524551
On Thu, 17 Nov 2016 15:02:18 +0000 (UTC)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:


> That's an interesting approach. I would be tempted to give it a
> per-thread (rather than per-process) scope.

Sure, per thread, but have it inherit to child processes.

> 
> E.g., a thread could do the following to ask to be
> interrupted by IPIs:
> 
> membarrier(MEMBARRIER_CMD_REGISTER_EXPEDITED, 0)
> 
> and could unregister with:
> 
> membarrier(MEMBARRIER_CMD_UNREGISTER_EXPEDITED, 0)

Sure why not ;-)

> 
> We can then keep a per-thread refcount internally.
> (not sure the "EXPEDITED" is the right word there...
> do we want it to be "NOHZ_FULL" instead ?)

No, it shouldn't mention NOHZ_FULL. Perhaps have all tasks do this
regardless, even though it will only affect nohz full ones. But in the
future it may be other tasks as well.

> 
> Then in membarrier(MEMBARRIER_CMD_SHARED, 0), for each
> nohz_full cpu, we grab the rq lock, and only send an IPI
> if the running thread is registered as "expedited".

Yeah, something like that. That way it wont interrupt tasks that are
running in no-hz-full and don't care about this syscall.

-- Steve

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


#1524654

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-11-17 19:20 +0100
Message-ID<sEzrX-4eD-9@gated-at.bofh.it>
In reply to#1524597
On Thu, Nov 17, 2016 at 10:17:25AM -0500, Steven Rostedt wrote:
> On Thu, 17 Nov 2016 15:02:18 +0000 (UTC)
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> 
> 
> > That's an interesting approach. I would be tempted to give it a
> > per-thread (rather than per-process) scope.
> 
> Sure, per thread, but have it inherit to child processes.
> 
> > 
> > E.g., a thread could do the following to ask to be
> > interrupted by IPIs:
> > 
> > membarrier(MEMBARRIER_CMD_REGISTER_EXPEDITED, 0)
> > 
> > and could unregister with:
> > 
> > membarrier(MEMBARRIER_CMD_UNREGISTER_EXPEDITED, 0)
> 
> Sure why not ;-)

Makes a lot of sense to me!

> > We can then keep a per-thread refcount internally.
> > (not sure the "EXPEDITED" is the right word there...
> > do we want it to be "NOHZ_FULL" instead ?)
> 
> No, it shouldn't mention NOHZ_FULL. Perhaps have all tasks do this
> regardless, even though it will only affect nohz full ones. But in the
> future it may be other tasks as well.
> 
> > 
> > Then in membarrier(MEMBARRIER_CMD_SHARED, 0), for each
> > nohz_full cpu, we grab the rq lock, and only send an IPI
> > if the running thread is registered as "expedited".
> 
> Yeah, something like that. That way it wont interrupt tasks that are
> running in no-hz-full and don't care about this syscall.

And this as well!

							Thanx, Paul

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


#1524719

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-11-17 19:50 +0100
Message-ID<sEzV0-4p4-29@gated-at.bofh.it>
In reply to#1524461
On Thu, Nov 17, 2016 at 01:54:27PM +0000, Mathieu Desnoyers wrote:
> ----- On Nov 17, 2016, at 8:40 AM, Paul E. McKenney paulmck@linux.vnet.ibm.com wrote:
> 
> > On Thu, Nov 17, 2016 at 11:46:34AM +0000, Mathieu Desnoyers wrote:
> >> ----- On Nov 17, 2016, at 1:51 AM, Lai Jiangshan jiangshanlai@gmail.com wrote:
> >> 
> >> > On Fri, Nov 4, 2016 at 12:29 AM, Mathieu Desnoyers
> >> > <mathieu.desnoyers@efficios.com> wrote:
> >> >> Userspace applications should be allowed to expect the membarrier system
> >> >> call with MEMBARRIER_CMD_SHARED command to issue memory barriers on
> >> >> nohz_full CPUs, but synchronize_sched() does not take those into
> >> >> account.
> >> >>
> >> >> Given that we do not want unrelated processes to be able to affect
> >> >> real-time sensitive nohz_full CPUs, simply return ENOSYS when membarrier
> >> >> is invoked on a kernel with enabled nohz_full CPUs.
> >> >>
> >> >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> >> >> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >> >> CC: Josh Triplett <josh@joshtriplett.org>
> >> >> CC: Steven Rostedt <rostedt@goodmis.org>
> >> >> CC: Lai Jiangshan <jiangshanlai@gmail.com>
> >> >> CC: <stable@vger.kernel.org>    [3.10+]
> >> >> ---
> >> >>  kernel/membarrier.c | 4 ++++
> >> >>  1 file changed, 4 insertions(+)
> >> >>
> >> >> diff --git a/kernel/membarrier.c b/kernel/membarrier.c
> >> >> index 536c727..9f9284f 100644
> >> >> --- a/kernel/membarrier.c
> >> >> +++ b/kernel/membarrier.c
> >> >> @@ -16,6 +16,7 @@
> >> >>
> >> >>  #include <linux/syscalls.h>
> >> >>  #include <linux/membarrier.h>
> >> >> +#include <linux/tick.h>
> >> >>
> >> >>  /*
> >> >>   * Bitmask made from a "or" of all commands within enum membarrier_cmd,
> >> >> @@ -51,6 +52,9 @@
> >> >>   */
> >> >>  SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
> >> >>  {
> >> >> +       /* MEMBARRIER_CMD_SHARED is not compatible with nohz_full. */
> >> >> +       if (tick_nohz_full_enabled())
> >> >> +               return -ENOSYS;
> >> > 
> >> > I guess this code needs to be moved down into the branch of
> >> > "case MEMBARRIER_CMD_SHARED" to match its comment.
> >> 
> >> No, that would be unexpected from user-space. Either a system
> >> call is implemented or not, not "implemented for some parameters".
> >> 
> >> We also want MEMBARRIER_CMD_QUERY to return -ENOSYS in this case,
> >> and all other parameter values to also return -ENOSYS (rather than
> >> -EINVAL).
> >> 
> >> If a system call that returns successfully on CMD_QUERY or EINVAL,
> >> user-space may assume it will not have to handle ENOSYS in the
> >> next calls.
> >> 
> >> 
> >> > 
> >> > Acked-by: Lai Jiangshan <jiangshanlai@gmail.com>
> >> > 
> >> > But I'm afraid, in the future, tick_nohz_full will become a default y
> >> > feature. thus it makes sys_membarrier() always disabled. we might
> >> > need a new MEMBARRIER_CMD_XXX to handle it?
> >> 
> >> This may require that we send an IPI to nohz_full CPUs, which will
> >> disturb them real-time wise. Any better ideas ?
> > 
> > Restrict the IPIs to CPUs running the process executing the
> > sys_membarrier() system call.  This would mean that CPUs only
> > are interrupted by their own application's request.
> 
> This would break use-cases of cross-process shared memory. :-(

Good point -- getting this working does look to be good clean fun...

							Thanx, Paul

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


#1524746

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-11-17 20:30 +0100
Message-ID<sEw15-1T0-69@gated-at.bofh.it>
In reply to#1524406
On Thu, Nov 17, 2016 at 11:46:34AM +0000, Mathieu Desnoyers wrote:
> ----- On Nov 17, 2016, at 1:51 AM, Lai Jiangshan jiangshanlai@gmail.com wrote:
> 
> > On Fri, Nov 4, 2016 at 12:29 AM, Mathieu Desnoyers
> > <mathieu.desnoyers@efficios.com> wrote:
> >> Userspace applications should be allowed to expect the membarrier system
> >> call with MEMBARRIER_CMD_SHARED command to issue memory barriers on
> >> nohz_full CPUs, but synchronize_sched() does not take those into
> >> account.
> >>
> >> Given that we do not want unrelated processes to be able to affect
> >> real-time sensitive nohz_full CPUs, simply return ENOSYS when membarrier
> >> is invoked on a kernel with enabled nohz_full CPUs.
> >>
> >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> >> CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >> CC: Josh Triplett <josh@joshtriplett.org>
> >> CC: Steven Rostedt <rostedt@goodmis.org>
> >> CC: Lai Jiangshan <jiangshanlai@gmail.com>
> >> CC: <stable@vger.kernel.org>    [3.10+]
> >> ---
> >>  kernel/membarrier.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/kernel/membarrier.c b/kernel/membarrier.c
> >> index 536c727..9f9284f 100644
> >> --- a/kernel/membarrier.c
> >> +++ b/kernel/membarrier.c
> >> @@ -16,6 +16,7 @@
> >>
> >>  #include <linux/syscalls.h>
> >>  #include <linux/membarrier.h>
> >> +#include <linux/tick.h>
> >>
> >>  /*
> >>   * Bitmask made from a "or" of all commands within enum membarrier_cmd,
> >> @@ -51,6 +52,9 @@
> >>   */
> >>  SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
> >>  {
> >> +       /* MEMBARRIER_CMD_SHARED is not compatible with nohz_full. */
> >> +       if (tick_nohz_full_enabled())
> >> +               return -ENOSYS;
> > 
> > I guess this code needs to be moved down into the branch of
> > "case MEMBARRIER_CMD_SHARED" to match its comment.
> 
> No, that would be unexpected from user-space. Either a system
> call is implemented or not, not "implemented for some parameters".
> 
> We also want MEMBARRIER_CMD_QUERY to return -ENOSYS in this case,
> and all other parameter values to also return -ENOSYS (rather than
> -EINVAL).
> 
> If a system call that returns successfully on CMD_QUERY or EINVAL,
> user-space may assume it will not have to handle ENOSYS in the
> next calls.
> 
> 
> > 
> > Acked-by: Lai Jiangshan <jiangshanlai@gmail.com>
> > 
> > But I'm afraid, in the future, tick_nohz_full will become a default y
> > feature. thus it makes sys_membarrier() always disabled. we might
> > need a new MEMBARRIER_CMD_XXX to handle it?
> 
> This may require that we send an IPI to nohz_full CPUs, which will
> disturb them real-time wise. Any better ideas ?

Restrict the IPIs to CPUs running the process executing the
sys_membarrier() system call.  This would mean that CPUs only
are interrupted by their own application's request.

							Thanx, Paul

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


#1524557

From"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Date2016-11-17 18:30 +0100
Message-ID<sEyFA-3CT-67@gated-at.bofh.it>
In reply to#1524129
On Thu, Nov 17, 2016 at 02:51:26PM +0800, Lai Jiangshan wrote:
> On Fri, Nov 4, 2016 at 12:29 AM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
> > Userspace applications should be allowed to expect the membarrier system
> > call with MEMBARRIER_CMD_SHARED command to issue memory barriers on
> > nohz_full CPUs, but synchronize_sched() does not take those into
> > account.
> >
> > Given that we do not want unrelated processes to be able to affect
> > real-time sensitive nohz_full CPUs, simply return ENOSYS when membarrier
> > is invoked on a kernel with enabled nohz_full CPUs.
> >
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > CC: Josh Triplett <josh@joshtriplett.org>
> > CC: Steven Rostedt <rostedt@goodmis.org>
> > CC: Lai Jiangshan <jiangshanlai@gmail.com>
> > CC: <stable@vger.kernel.org>    [3.10+]
> > ---
> >  kernel/membarrier.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/kernel/membarrier.c b/kernel/membarrier.c
> > index 536c727..9f9284f 100644
> > --- a/kernel/membarrier.c
> > +++ b/kernel/membarrier.c
> > @@ -16,6 +16,7 @@
> >
> >  #include <linux/syscalls.h>
> >  #include <linux/membarrier.h>
> > +#include <linux/tick.h>
> >
> >  /*
> >   * Bitmask made from a "or" of all commands within enum membarrier_cmd,
> > @@ -51,6 +52,9 @@
> >   */
> >  SYSCALL_DEFINE2(membarrier, int, cmd, int, flags)
> >  {
> > +       /* MEMBARRIER_CMD_SHARED is not compatible with nohz_full. */
> > +       if (tick_nohz_full_enabled())
> > +               return -ENOSYS;
> 
> I guess this code needs to be moved down into the branch of
> "case MEMBARRIER_CMD_SHARED" to match its comment.
> 
> Acked-by: Lai Jiangshan <jiangshanlai@gmail.com>

Added, thank you!

> But I'm afraid, in the future, tick_nohz_full will become a default y
> feature. thus it makes sys_membarrier() always disabled. we might
> need a new MEMBARRIER_CMD_XXX to handle it?

Makes a lot of sense to me!

							Thanx, Paul

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web