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

Groups > linux.kernel > #1195180 > unrolled thread

Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick dependency mask model

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 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Frederic Weisbecker <fweisbec@gmail.com> - 2015-07-29 15:30 +0200
    Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Luiz Capitulino <lcapitulino@redhat.com> - 2015-07-30 16:40 +0200
      Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Frederic Weisbecker <fweisbec@gmail.com> - 2015-07-30 16:50 +0200
    Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Chris Metcalf <cmetcalf@ezchip.com> - 2015-07-30 21:40 +0200
      Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Frederic Weisbecker <fweisbec@gmail.com> - 2015-07-30 21:50 +0200
        Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Chris Metcalf <cmetcalf@ezchip.com> - 2015-07-30 22:00 +0200
          Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Chris Metcalf <cmetcalf@ezchip.com> - 2015-08-03 18:00 +0200
            Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-03 20:10 +0200
          Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Peter Zijlstra <peterz@infradead.org> - 2015-08-03 19:20 +0200
            Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-03 19:40 +0200
              Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick  dependency mask model Peter Zijlstra <peterz@infradead.org> - 2015-08-03 21:10 +0200

#1195180 — Re: [PATCH 08/10] posix-cpu-timers: Migrate to use new tick dependency mask model

On Fri, Jul 24, 2015 at 12:57:24PM -0400, Chris Metcalf wrote:
> On 07/23/2015 12:42 PM, Frederic Weisbecker wrote:
> >+static void cpu_timer_list_dequeue(struct cpu_timer_list *t)
> >+{
> >+	if (!list_empty(&t->entry))
> >+		cpu_timer_dec_tick_dependency();
> >+	list_del_init(&t->entry);
> >+}
> 
> Is the list_empty() test necessary? It wasn't in the original posix-timers
> code, and it feels like a pretty serious bug if you're doing a list_del on
> an empty list.

No multiple calls to list_del_init() is fine on a list_head as long as it
has been correctly initialized with INIT_LIST_HEAD() and list_del() hasn't
been called at some point before.

It's necessary because we do that dequeue also when we change the timer,
we disarm it in case it was added somewhere before.

> At a higher level, is the posix-cpu-timers code here really providing the
> right semantics? It seems like before, the code was checking a struct
> task-specific state, and now you are setting a global state such that if ANY
> task anywhere in the system (even on housekeeping cores) has a pending posix
> cpu timer, then nothing can go into nohz_full mode.
> 
> Perhaps what is needed is a task_struct->tick_dependency to go along with
> the system-wide and per-cpu flag words?

That's an excellent point! Indeed the tick dependency check on posix-cpu-timers
was made on task granularity before and now it's a global dependency.

Which means that if any task in the system has a posix-cpu-timer enqueued, it
prevents all CPUs from shutting down the tick. I need to mention that in the
changelog.

Now here is the rationale: I expect that nohz full users are not interested in
posix cpu timers at all. The only chance for one to run without breaking the
isolation is on housekeeping CPUs. So perhaps there is a corner case somewhere
but I assume there isn't until somebody reports an issue.

Keeping a task level dependency check means that we need to update it on context
switch. Plus it's not only about task but also process. So that means two
states to update on context switch and to check from interrupts. I don't think
it's worth the effort if there is no user at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1196078

On Thu, 30 Jul 2015 02:44:45 +0200
Frederic Weisbecker <fweisbec@gmail.com> wrote:

> 
> On Wed, Jul 29, 2015 at 01:24:16PM -0400, Chris Metcalf wrote:
> > On 07/29/2015 09:23 AM, Frederic Weisbecker wrote:
> > >>At a higher level, is the posix-cpu-timers code here really providing the
> > >>>right semantics? It seems like before, the code was checking a struct
> > >>>task-specific state, and now you are setting a global state such that if ANY
> > >>>task anywhere in the system (even on housekeeping cores) has a pending posix
> > >>>cpu timer, then nothing can go into nohz_full mode.
> > >>>
> > >>>Perhaps what is needed is a task_struct->tick_dependency to go along with
> > >>>the system-wide and per-cpu flag words?
> > >That's an excellent point! Indeed the tick dependency check on posix-cpu-timers
> > >was made on task granularity before and now it's a global dependency.
> > >
> > >Which means that if any task in the system has a posix-cpu-timer enqueued, it
> > >prevents all CPUs from shutting down the tick. I need to mention that in the
> > >changelog.
> > >
> > >Now here is the rationale: I expect that nohz full users are not interested in
> > >posix cpu timers at all. The only chance for one to run without breaking the
> > >isolation is on housekeeping CPUs. So perhaps there is a corner case somewhere
> > >but I assume there isn't until somebody reports an issue.
> > >
> > >Keeping a task level dependency check means that we need to update it on context
> > >switch. Plus it's not only about task but also process. So that means two
> > >states to update on context switch and to check from interrupts. I don't think
> > >it's worth the effort if there is no user at all.
> > 
> > I really worry about this!  The vision EZchip offers our customers is
> > that they can run whatever they want on the slow path housekeeping
> > cores, i.e. random control-plane code.  Then, on the fast-path cores,
> > they run their nohz_full stuff without interruption.  Often they don't
> > even know what the hell is running on their control plane cores - SNMP
> > or random third-party crap or god knows what.  And there is a decent
> > likelihood that some posix cpu timer code might sneak in.

I share this thinking. We do the exactly same thing for KVM-RT and I
wouldn't be surprised at all if a posix timer pops up in the
housekeeping CPUs.

> I see. But note that installing a posix cpu timer ends up triggering an
> IPI to all nohz full CPUs. That's how nohz full has always behaved.
> So users running posix timers on nohz should already suffer issues anyway.

I haven't checked how this would affect us, but seems a lot less serious
then not having nohz at all.

> 
> > 
> > You mentioned needing two fields, for task and for process, but in
> > fact let's just add the one field to the one thing that needs it and
> > not worry about additional possible future needs.  And note that it's
> > the task_struct->signal where we need to add the field for posix cpu
> > timers (the signal_struct) since that's where the sharing occurs, and
> > given CLONE_SIGHAND I imagine it could be different from the general
> > "process" model anyway.
> 
> Well, posix cpu timers can be install per process (signal struct) or
> per thread (task struct).
> 
> But we can certainly simplify that with a per process flag and expand
> the thread dependency to the process scope.
> 
> Still there is the issue of telling the CPUs where a process runs when
> a posix timer is installed there. There is no process-like tsk->cpus_allowed.
> Either we send an IPI everywhere like we do now or we iterate through all
> threads in the process to OR all their cpumasks in order to send that IPI.
> 
> > 
> > In any case it seems like we don't need to do work at context switch.
> > Updates to the task's tick_dependency are just done as normal in the
> > task context via "current->signal->". When we are returning to user
> > space and we want to check the tick, again, we can just read via
> > "current->signal->".  Why would we need to copy the value around at
> > task switch time?  That's only necessary if you want to do something
> > like read/write the task tick_dependency via the cpu index, I would think.
> 
> Yeah you're right, at least the context switch should be fine.
> 
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1196095

On Thu, Jul 30, 2015 at 10:31:47AM -0400, Luiz Capitulino wrote:
> On Thu, 30 Jul 2015 02:44:45 +0200
> Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > On Wed, Jul 29, 2015 at 01:24:16PM -0400, Chris Metcalf wrote:
> > > I really worry about this!  The vision EZchip offers our customers is
> > > that they can run whatever they want on the slow path housekeeping
> > > cores, i.e. random control-plane code.  Then, on the fast-path cores,
> > > they run their nohz_full stuff without interruption.  Often they don't
> > > even know what the hell is running on their control plane cores - SNMP
> > > or random third-party crap or god knows what.  And there is a decent
> > > likelihood that some posix cpu timer code might sneak in.
> 
> I share this thinking. We do the exactly same thing for KVM-RT and I
> wouldn't be surprised at all if a posix timer pops up in the
> housekeeping CPUs.

Ok you guys convinced me, I'll reiterate with a per-process mask.

> 
> > I see. But note that installing a posix cpu timer ends up triggering an
> > IPI to all nohz full CPUs. That's how nohz full has always behaved.
> > So users running posix timers on nohz should already suffer issues anyway.
> 
> I haven't checked how this would affect us, but seems a lot less serious
> then not having nohz at all.

Indeed. It's the difference between just receiving one IPI on every CPU and
having one interrupt every 1/Hz.

I'll just keep that global IPI to tell all CPUs that they may run a task
with a posix cpu timer queued. It's necessary for the CPUs to restart the
tick if needed. It's the current behavior and it makes things very simple.
Besides, nobody complained about it yet.

I can fix it if people request it but this will be in another patchset because
it's a complicated issue on its own.

To summarize: this patchset won't change the current upstream behaviour.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1196395

On 07/29/2015 08:44 PM, Frederic Weisbecker wrote:
> On Wed, Jul 29, 2015 at 01:24:16PM -0400, Chris Metcalf wrote:
>> On 07/29/2015 09:23 AM, Frederic Weisbecker wrote:
>>>> At a higher level, is the posix-cpu-timers code here really providing the
>>>>> right semantics? It seems like before, the code was checking a struct
>>>>> task-specific state, and now you are setting a global state such that if ANY
>>>>> task anywhere in the system (even on housekeeping cores) has a pending posix
>>>>> cpu timer, then nothing can go into nohz_full mode.
>>>>>
>>>>> Perhaps what is needed is a task_struct->tick_dependency to go along with
>>>>> the system-wide and per-cpu flag words?
>>> That's an excellent point! Indeed the tick dependency check on posix-cpu-timers
>>> was made on task granularity before and now it's a global dependency.
>>>
>>> Which means that if any task in the system has a posix-cpu-timer enqueued, it
>>> prevents all CPUs from shutting down the tick. I need to mention that in the
>>> changelog.
>>>
>>> Now here is the rationale: I expect that nohz full users are not interested in
>>> posix cpu timers at all. The only chance for one to run without breaking the
>>> isolation is on housekeeping CPUs. So perhaps there is a corner case somewhere
>>> but I assume there isn't until somebody reports an issue.
>>>
>>> Keeping a task level dependency check means that we need to update it on context
>>> switch. Plus it's not only about task but also process. So that means two
>>> states to update on context switch and to check from interrupts. I don't think
>>> it's worth the effort if there is no user at all.
>> I really worry about this!  The vision EZchip offers our customers is
>> that they can run whatever they want on the slow path housekeeping
>> cores, i.e. random control-plane code.  Then, on the fast-path cores,
>> they run their nohz_full stuff without interruption.  Often they don't
>> even know what the hell is running on their control plane cores - SNMP
>> or random third-party crap or god knows what.  And there is a decent
>> likelihood that some posix cpu timer code might sneak in.
> I see. But note that installing a posix cpu timer ends up triggering an
> IPI to all nohz full CPUs. That's how nohz full has always behaved.
> So users running posix timers on nohz should already suffer issues anyway.

True now, yes, I'm just looking ahead to doing better when we have
a chance to improve things.

>> You mentioned needing two fields, for task and for process, but in
>> fact let's just add the one field to the one thing that needs it and
>> not worry about additional possible future needs.  And note that it's
>> the task_struct->signal where we need to add the field for posix cpu
>> timers (the signal_struct) since that's where the sharing occurs, and
>> given CLONE_SIGHAND I imagine it could be different from the general
>> "process" model anyway.
> Well, posix cpu timers can be install per process (signal struct) or
> per thread (task struct).
>
> But we can certainly simplify that with a per process flag and expand
> the thread dependency to the process scope.
>
> Still there is the issue of telling the CPUs where a process runs when
> a posix timer is installed there. There is no process-like tsk->cpus_allowed.
> Either we send an IPI everywhere like we do now or we iterate through all
> threads in the process to OR all their cpumasks in order to send that IPI.

Is there a reason the actual timer can't run on a housekeeping
core?  Then when it does wake_up_process() or whatever, the
specific target task will get an IPI to wake up at that point.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1196398

On Thu, Jul 30, 2015 at 03:35:06PM -0400, Chris Metcalf wrote:
> On 07/29/2015 08:44 PM, Frederic Weisbecker wrote:
> >I see. But note that installing a posix cpu timer ends up triggering an
> >IPI to all nohz full CPUs. That's how nohz full has always behaved.
> >So users running posix timers on nohz should already suffer issues anyway.
> 
> True now, yes, I'm just looking ahead to doing better when we have
> a chance to improve things.

Sure, but we can fix that global IPI as well anyway.

> 
> >>You mentioned needing two fields, for task and for process, but in
> >>fact let's just add the one field to the one thing that needs it and
> >>not worry about additional possible future needs.  And note that it's
> >>the task_struct->signal where we need to add the field for posix cpu
> >>timers (the signal_struct) since that's where the sharing occurs, and
> >>given CLONE_SIGHAND I imagine it could be different from the general
> >>"process" model anyway.
> >Well, posix cpu timers can be install per process (signal struct) or
> >per thread (task struct).
> >
> >But we can certainly simplify that with a per process flag and expand
> >the thread dependency to the process scope.
> >
> >Still there is the issue of telling the CPUs where a process runs when
> >a posix timer is installed there. There is no process-like tsk->cpus_allowed.
> >Either we send an IPI everywhere like we do now or we iterate through all
> >threads in the process to OR all their cpumasks in order to send that IPI.
> 
> Is there a reason the actual timer can't run on a housekeeping
> core?  Then when it does wake_up_process() or whatever, the
> specific target task will get an IPI to wake up at that point.

It makes sense if people run posix cpu timers on nohz full CPUs. But nobody
reported such usecase yet.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1196403

On 07/30/2015 03:45 PM, Frederic Weisbecker wrote:
>
>>>> You mentioned needing two fields, for task and for process, but in
>>>> fact let's just add the one field to the one thing that needs it and
>>>> not worry about additional possible future needs.  And note that it's
>>>> the task_struct->signal where we need to add the field for posix cpu
>>>> timers (the signal_struct) since that's where the sharing occurs, and
>>>> given CLONE_SIGHAND I imagine it could be different from the general
>>>> "process" model anyway.
>>> Well, posix cpu timers can be install per process (signal struct) or
>>> per thread (task struct).
>>>
>>> But we can certainly simplify that with a per process flag and expand
>>> the thread dependency to the process scope.
>>>
>>> Still there is the issue of telling the CPUs where a process runs when
>>> a posix timer is installed there. There is no process-like tsk->cpus_allowed.
>>> Either we send an IPI everywhere like we do now or we iterate through all
>>> threads in the process to OR all their cpumasks in order to send that IPI.
>> Is there a reason the actual timer can't run on a housekeeping
>> core?  Then when it does wake_up_process() or whatever, the
>> specific target task will get an IPI to wake up at that point.
> It makes sense if people run posix cpu timers on nohz full CPUs. But nobody
> reported such usecase yet.

The corner case I was trying to address with my comment above
is when a process includes both housekeeping and nohz_full threads.
This is generally a bad idea in my experience, but our customers
do this sometimes (usually because they're porting a big pile of
code from somewhere else), and if so it would be good if we didn't
have to keep every thread in that task ticking; presumably it is
enough to ensure the timer lands on a housekeeping core instead,
possibly the one for the non-fast-path thread in question, and then
the regular IPIs from wake_up_process() will be sufficient if for
some lame reason the signal ends up handled on a nohz_full core.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1198993

On 07/31/2015 10:49 AM, Frederic Weisbecker wrote:
> Instead of doing a per signal dependency, I'm going to use a per task
> one. Which means that if a per-process timer is enqueued, every thread
> of that process will have the tick dependency. But if the timer is
> enqueued to a single thread, only the thread is concerned.
>
> We'll see if offloading becomes really needed. It's not quite free because
> the housekeepers will have to poll on all nohz CPUs at a Hz frequency.

Seems reasonable for now!

Why would we need the Hz frequency polling, though?  I would
think it should be possible to just arrange it such that the timer
for posix cpu timers would just always be placed either on the core
that requested it, or if that core is nohz_full, on a housekeeping
core.  Then it would eventually fire from the housekeeping core,
and the logic could be such that (for a process-wide timer) it
would preferentially interrupt threads from that process that
were running on the housekeeping cores.  No polling.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1199132

On Mon, Aug 03, 2015 at 11:59:07AM -0400, Chris Metcalf wrote:
> On 07/31/2015 10:49 AM, Frederic Weisbecker wrote:
> >Instead of doing a per signal dependency, I'm going to use a per task
> >one. Which means that if a per-process timer is enqueued, every thread
> >of that process will have the tick dependency. But if the timer is
> >enqueued to a single thread, only the thread is concerned.
> >
> >We'll see if offloading becomes really needed. It's not quite free because
> >the housekeepers will have to poll on all nohz CPUs at a Hz frequency.
> 
> Seems reasonable for now!
> 
> Why would we need the Hz frequency polling, though?  I would
> think it should be possible to just arrange it such that the timer
> for posix cpu timers would just always be placed either on the core
> that requested it, or if that core is nohz_full, on a housekeeping
> core.  Then it would eventually fire from the housekeeping core,
> and the logic could be such that (for a process-wide timer) it
> would preferentially interrupt threads from that process that
> were running on the housekeeping cores.  No polling.

But you need to periodically poll on timer expiration from a housekeeper.
It's not only about firing the timer, it's about elapsing it against the
target cputime.

Since there is no tick on a nohz full CPU to account the time spent by
the task, you must do that elsewhere. And if you don't poll in a sufficient
frequency, the time accounted is less precise (a quick round-trip to kernel space
can be missed if the polling frequency is too low). Or you can combine it
with the VIRT_CPU_ACCOUNTING_GEN that we are using currently which records the
time spent in user and kernel space using hooks. Still you must check periodically
that the timer hasn't expired at a frequency that doesn't go further the
expiration time. Easy in the case of a timer attached to a single task but what
about a timer attached to a process? You must poll at least at expiration/nr_threads,
so you must handle thread creation as well.

Offlining posix timers sounds like a big headache if we don't poll at Hz time.

That said Rick has posted patches that offline cputime accounting. I'm not yet sure
this patchset is a good idea but offlining posix timers can be done on top of that.

Another thing: now I recall why I turned posix timers to a global tick dependency.
In case of a per task/process dependency we still need the context switch hook because
if we enqueue a timer to a sleeping task, the tick must be restarted when the task wakes
up. And that requires a check on context switch.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1199087

On Fri, Jul 31, 2015 at 04:49:55PM +0200, Frederic Weisbecker wrote:
> Instead of doing a per signal dependency, I'm going to use a per task
> one.

Urgh, does this mean you'll keep the horrid tick_nohz_task_switch()
thing?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1199123

On Mon, Aug 03, 2015 at 07:12:43PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 31, 2015 at 04:49:55PM +0200, Frederic Weisbecker wrote:
> > Instead of doing a per signal dependency, I'm going to use a per task
> > one.
> 
> Urgh, does this mean you'll keep the horrid tick_nohz_task_switch()
> thing?

I thought I would drop it, but now that I think about it more, I think I
need to keep it because if we enqueue a posix timer to a sleeping task
and that task later wakes up, we must restart the tick, and that is only
possible with a check on context switch :-(

This current patchset removed the need for that with a global dependency
for posix timers: as long as there is one enqueued we keep the tick. But
Chris and Luiz fear that Tilera users have posix timers on housekeepers.
They also suggested we offline the posix timers. I fear it's going to be
a high overhead as it means polling on the target task context of execution.
Unless we move the task itself to housekeepers...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1199195

On Mon, Aug 03, 2015 at 07:39:37PM +0200, Frederic Weisbecker wrote:
> On Mon, Aug 03, 2015 at 07:12:43PM +0200, Peter Zijlstra wrote:
> > On Fri, Jul 31, 2015 at 04:49:55PM +0200, Frederic Weisbecker wrote:
> > > Instead of doing a per signal dependency, I'm going to use a per task
> > > one.
> > 
> > Urgh, does this mean you'll keep the horrid tick_nohz_task_switch()
> > thing?
> 
> I thought I would drop it, but now that I think about it more, I think I
> need to keep it because if we enqueue a posix timer to a sleeping task
> and that task later wakes up, we must restart the tick, and that is only
> possible with a check on context switch :-(
> 
> This current patchset removed the need for that with a global dependency
> for posix timers: as long as there is one enqueued we keep the tick. But
> Chris and Luiz fear that Tilera users have posix timers on housekeepers.
> They also suggested we offline the posix timers. I fear it's going to be
> a high overhead as it means polling on the target task context of execution.
> Unless we move the task itself to housekeepers...

At least do something like the below, that irq save/restore is expensive
and can be trivially avoided.

Also, tick_nohz_full_kick() checks that tick_nohz_full_cpu() thing
again.

---
 kernel/sched/core.c      | 2 +-
 kernel/time/tick-sched.c | 9 +--------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 4d34035bb3ee..57d1af7c0660 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2471,6 +2471,7 @@ static struct rq *finish_task_switch(struct task_struct *prev)
 	vtime_task_switch(prev);
 	finish_arch_switch(prev);
 	perf_event_task_sched_in(prev, current);
+	tick_nohz_task_switch();
 	finish_lock_switch(rq, prev);
 	finish_arch_post_lock_switch();
 
@@ -2489,7 +2490,6 @@ static struct rq *finish_task_switch(struct task_struct *prev)
 		put_task_struct(prev);
 	}
 
-	tick_nohz_task_switch();
 	return rq;
 }
 
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 3319e16f31e5..9f2225ef230c 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -260,18 +260,11 @@ void tick_nohz_full_kick_all(void)
  */
 void __tick_nohz_task_switch(void)
 {
-	unsigned long flags;
-
-	local_irq_save(flags);
-
 	if (!tick_nohz_full_cpu(smp_processor_id()))
-		goto out;
+		return;
 
 	if (tick_nohz_tick_stopped() && !can_stop_full_tick())
 		tick_nohz_full_kick();
-
-out:
-	local_irq_restore(flags);
 }
 
 /* Parse the boot-time nohz CPU list from the kernel parameters. */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web