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


Groups > linux.kernel > #1281207 > unrolled thread

Re: [PATCH 2/7] nohz: New tick dependency mask

Started byPeter Zijlstra <peterz@infradead.org>
First post2015-12-01 21:50 +0100
Last post2015-12-02 15:20 +0100
Articles 10 — 2 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 2/7] nohz: New tick dependency mask Peter Zijlstra <peterz@infradead.org> - 2015-12-01 21:50 +0100
    Re: [PATCH 2/7] nohz: New tick dependency mask Frederic Weisbecker <fweisbec@gmail.com> - 2015-12-01 23:30 +0100
      Re: [PATCH 2/7] nohz: New tick dependency mask Peter Zijlstra <peterz@infradead.org> - 2015-12-02 12:00 +0100
        Re: [PATCH 2/7] nohz: New tick dependency mask Frederic Weisbecker <fweisbec@gmail.com> - 2015-12-02 15:10 +0100
          Re: [PATCH 2/7] nohz: New tick dependency mask Peter Zijlstra <peterz@infradead.org> - 2015-12-02 16:10 +0100
            Re: [PATCH 2/7] nohz: New tick dependency mask Frederic Weisbecker <fweisbec@gmail.com> - 2015-12-08 17:00 +0100
      Re: [PATCH 2/7] nohz: New tick dependency mask Peter Zijlstra <peterz@infradead.org> - 2015-12-02 13:50 +0100
        Re: [PATCH 2/7] nohz: New tick dependency mask Frederic Weisbecker <fweisbec@gmail.com> - 2015-12-02 15:20 +0100
      Re: [PATCH 2/7] nohz: New tick dependency mask Peter Zijlstra <peterz@infradead.org> - 2015-12-02 13:50 +0100
        Re: [PATCH 2/7] nohz: New tick dependency mask Frederic Weisbecker <fweisbec@gmail.com> - 2015-12-02 15:20 +0100

#1281207 — Re: [PATCH 2/7] nohz: New tick dependency mask

FromPeter Zijlstra <peterz@infradead.org>
Date2015-12-01 21:50 +0100
SubjectRe: [PATCH 2/7] nohz: New tick dependency mask
Message-ID<qB027-65N-25@gated-at.bofh.it>
On Fri, Nov 13, 2015 at 03:22:04PM +0100, Frederic Weisbecker wrote:
> The tick dependency is evaluated on every IRQ. This is a batch of checks
> which determine whether it is safe to stop the tick or not. These checks
> are often split in many details: posix cpu timers, scheduler, sched clock,
> perf events. Each of which are made of smaller details: posix cpu
> timer involves checking process wide timers then thread wide timers. Perf
> involves checking freq events then more per cpu details.
> 
> Checking these details asynchronously every time we update the full
> dynticks state bring avoidable overhead and a messy layout.
> 
> Lets introduce instead tick dependency masks: one for system wide
> dependency (unstable sched clock), one for CPU wide dependency (sched,
> perf), and task/signal level dependencies. The subsystems are responsible
> of setting and clearing their dependency through a set of APIs that will
> take care of concurrent dependency mask modifications and kick targets
> to restart the relevant CPU tick whenever needed.

Maybe better explain why we need the per task and per signal thingy?

> +static void trace_tick_dependency(unsigned long dep)
> +{
> +	if (dep & TICK_POSIX_TIMER_MASK) {
> +		trace_tick_stop(0, "posix timers running\n");
> +		return;
> +	}
> +
> +	if (dep & TICK_PERF_EVENTS_MASK) {
> +		trace_tick_stop(0, "perf events running\n");
> +		return;
> +	}
> +
> +	if (dep & TICK_SCHED_MASK) {
> +		trace_tick_stop(0, "more than 1 task in runqueue\n");
> +		return;
> +	}
> +
> +	if (dep & TICK_CLOCK_UNSTABLE_MASK)
> +		trace_tick_stop(0, "unstable sched clock\n");
> +}

I would suggest ditching the strings and using the 

> +static void kick_all_work_fn(struct work_struct *work)
> +{
> +       tick_nohz_full_kick_all();
> +}
> +static DECLARE_WORK(kick_all_work, kick_all_work_fn);
> +
> +void __tick_nohz_set_dep_delayed(enum tick_dependency_bit bit, unsigned long *dep)
> +{
> +	unsigned long prev;
> +
> +	prev = fetch_or(dep, BIT_MASK(bit));
> +	if (!prev) {
> +		/*
> +		* We need the IPIs to be sent from sane process context.

Why ?

> +		* The posix cpu timers are always set with irqs disabled.
> +		*/
> +		schedule_work(&kick_all_work);
> +	}
> +}
> +
> +/*
> + * Set a global tick dependency. Lets do the wide IPI kick asynchronously
> + * for callers with irqs disabled.

This seems to suggest you can call this with IRQs disabled

> + */
> +void tick_nohz_set_dep(enum tick_dependency_bit bit)
> +{
> +	unsigned long prev;
> +
> +	prev = fetch_or(&tick_dependency, BIT_MASK(bit));
> +	if (!prev)
> +		tick_nohz_full_kick_all();

But that function seems implemented using smp_call_function_many() which
cannot be called with IRQs disabled.

--
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]


#1281262

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2015-12-01 23:30 +0100
Message-ID<qB1AS-79P-5@gated-at.bofh.it>
In reply to#1281207
On Tue, Dec 01, 2015 at 09:41:09PM +0100, Peter Zijlstra wrote:
> On Fri, Nov 13, 2015 at 03:22:04PM +0100, Frederic Weisbecker wrote:
> > The tick dependency is evaluated on every IRQ. This is a batch of checks
> > which determine whether it is safe to stop the tick or not. These checks
> > are often split in many details: posix cpu timers, scheduler, sched clock,
> > perf events. Each of which are made of smaller details: posix cpu
> > timer involves checking process wide timers then thread wide timers. Perf
> > involves checking freq events then more per cpu details.
> > 
> > Checking these details asynchronously every time we update the full
> > dynticks state bring avoidable overhead and a messy layout.
> > 
> > Lets introduce instead tick dependency masks: one for system wide
> > dependency (unstable sched clock), one for CPU wide dependency (sched,
> > perf), and task/signal level dependencies. The subsystems are responsible
> > of setting and clearing their dependency through a set of APIs that will
> > take care of concurrent dependency mask modifications and kick targets
> > to restart the relevant CPU tick whenever needed.
> 
> Maybe better explain why we need the per task and per signal thingy?

I'll detail that some more in the changelog. The only user of the per task/per signal
tick dependency is posix cpu timer. I've been first proposing a global tick dependency
as soon as any posix cpu timer is armed. It simplified everything but some reviewers
complained (eg: some users might want to run posix timers on housekeepers without
bothering full dynticks CPUs). I could remove the per signal dependency with dispatching
it through all threads in the group each time there is an update but that's the best I can
think of.

> 
> > +static void trace_tick_dependency(unsigned long dep)
> > +{
> > +	if (dep & TICK_POSIX_TIMER_MASK) {
> > +		trace_tick_stop(0, "posix timers running\n");
> > +		return;
> > +	}
> > +
> > +	if (dep & TICK_PERF_EVENTS_MASK) {
> > +		trace_tick_stop(0, "perf events running\n");
> > +		return;
> > +	}
> > +
> > +	if (dep & TICK_SCHED_MASK) {
> > +		trace_tick_stop(0, "more than 1 task in runqueue\n");
> > +		return;
> > +	}
> > +
> > +	if (dep & TICK_CLOCK_UNSTABLE_MASK)
> > +		trace_tick_stop(0, "unstable sched clock\n");
> > +}
> 
> I would suggest ditching the strings and using the

Using a code value instead?

> 
> > +static void kick_all_work_fn(struct work_struct *work)
> > +{
> > +       tick_nohz_full_kick_all();
> > +}
> > +static DECLARE_WORK(kick_all_work, kick_all_work_fn);
> > +
> > +void __tick_nohz_set_dep_delayed(enum tick_dependency_bit bit, unsigned long *dep)
> > +{
> > +	unsigned long prev;
> > +
> > +	prev = fetch_or(dep, BIT_MASK(bit));
> > +	if (!prev) {
> > +		/*
> > +		* We need the IPIs to be sent from sane process context.
> 
> Why ?

Because posix timers code is all called with interrupts disabled and we can't
send IPIs then.

> 
> > +		* The posix cpu timers are always set with irqs disabled.
> > +		*/
> > +		schedule_work(&kick_all_work);
> > +	}
> > +}
> > +
> > +/*
> > + * Set a global tick dependency. Lets do the wide IPI kick asynchronously
> > + * for callers with irqs disabled.
> 
> This seems to suggest you can call this with IRQs disabled

Ah right, that's a misleading comment. We need to use the _delayed() version
when interrupts are disabled.

Thanks.

> 
> > + */
> > +void tick_nohz_set_dep(enum tick_dependency_bit bit)
> > +{
> > +	unsigned long prev;
> > +
> > +	prev = fetch_or(&tick_dependency, BIT_MASK(bit));
> > +	if (!prev)
> > +		tick_nohz_full_kick_all();
> 
> But that function seems implemented using smp_call_function_many() which
> cannot be called with IRQs disabled.
--
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]


#1281625

FromPeter Zijlstra <peterz@infradead.org>
Date2015-12-02 12:00 +0100
Message-ID<qBdiG-64Q-11@gated-at.bofh.it>
In reply to#1281262
On Tue, Dec 01, 2015 at 11:20:28PM +0100, Frederic Weisbecker wrote:
> > > +	prev = fetch_or(dep, BIT_MASK(bit));
> > > +	if (!prev) {
> > > +		/*
> > > +		* We need the IPIs to be sent from sane process context.
> > 
> > Why ?
> 
> Because posix timers code is all called with interrupts disabled and we can't
> send IPIs then.
> 
> > 
> > > +		* The posix cpu timers are always set with irqs disabled.
> > > +		*/
> > > +		schedule_work(&kick_all_work);
> > > +	}
> > > +}
> > > +
> > > +/*
> > > + * Set a global tick dependency. Lets do the wide IPI kick asynchronously
> > > + * for callers with irqs disabled.
> > 
> > This seems to suggest you can call this with IRQs disabled
> 
> Ah right, that's a misleading comment. We need to use the _delayed() version
> when interrupts are disabled.

Why can't you use tick_nohz_full_kick_cpu() for all that, which is
usable from IRQ context and avoid all that delayed muck?
--
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]


#1281842

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2015-12-02 15:10 +0100
Message-ID<qBggB-8db-71@gated-at.bofh.it>
In reply to#1281625
On Wed, Dec 02, 2015 at 11:56:33AM +0100, Peter Zijlstra wrote:
> On Tue, Dec 01, 2015 at 11:20:28PM +0100, Frederic Weisbecker wrote:
> > > > +	prev = fetch_or(dep, BIT_MASK(bit));
> > > > +	if (!prev) {
> > > > +		/*
> > > > +		* We need the IPIs to be sent from sane process context.
> > > 
> > > Why ?
> > 
> > Because posix timers code is all called with interrupts disabled and we can't
> > send IPIs then.
> > 
> > > 
> > > > +		* The posix cpu timers are always set with irqs disabled.
> > > > +		*/
> > > > +		schedule_work(&kick_all_work);
> > > > +	}
> > > > +}
> > > > +
> > > > +/*
> > > > + * Set a global tick dependency. Lets do the wide IPI kick asynchronously
> > > > + * for callers with irqs disabled.
> > > 
> > > This seems to suggest you can call this with IRQs disabled
> > 
> > Ah right, that's a misleading comment. We need to use the _delayed() version
> > when interrupts are disabled.
> 
> Why can't you use tick_nohz_full_kick_cpu() for all that, which is
> usable from IRQ context and avoid all that delayed muck?

Because I need to kick all the CPUs where the task/signal is running on. That's
a bit difficult to do though. I think we had something to try to send an IPI to a task,
but I can't retrieve it. Looks easy to do anyway. But in the signal case I'd need to do
that for all tasks in the group. That sounds like a costly loop.

So I simplify that with a global IPI.

--
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]


#1281883

FromPeter Zijlstra <peterz@infradead.org>
Date2015-12-02 16:10 +0100
Message-ID<qBhcB-nj-11@gated-at.bofh.it>
In reply to#1281842
On Wed, Dec 02, 2015 at 03:08:16PM +0100, Frederic Weisbecker wrote:
> On Wed, Dec 02, 2015 at 11:56:33AM +0100, Peter Zijlstra wrote:
> > On Tue, Dec 01, 2015 at 11:20:28PM +0100, Frederic Weisbecker wrote:
> > > > > +	prev = fetch_or(dep, BIT_MASK(bit));
> > > > > +	if (!prev) {
> > > > > +		/*
> > > > > +		* We need the IPIs to be sent from sane process context.
> > > > 
> > > > Why ?
> > > 
> > > Because posix timers code is all called with interrupts disabled and we can't
> > > send IPIs then.
> > > 
> > > > 
> > > > > +		* The posix cpu timers are always set with irqs disabled.
> > > > > +		*/
> > > > > +		schedule_work(&kick_all_work);
> > > > > +	}
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * Set a global tick dependency. Lets do the wide IPI kick asynchronously
> > > > > + * for callers with irqs disabled.
> > > > 
> > > > This seems to suggest you can call this with IRQs disabled
> > > 
> > > Ah right, that's a misleading comment. We need to use the _delayed() version
> > > when interrupts are disabled.
> > 
> > Why can't you use tick_nohz_full_kick_cpu() for all that, which is
> > usable from IRQ context and avoid all that delayed muck?
> 
> Because I need to kick all the CPUs where the task/signal is running on. That's
> a bit difficult to do though. I think we had something to try to send an IPI to a task,
> but I can't retrieve it. Looks easy to do anyway. But in the signal case I'd need to do
> that for all tasks in the group. That sounds like a costly loop.
> 
> So I simplify that with a global IPI.

Sure, but what I meant was that:

	for_each_cpu(cpu, mask)
		tick_nohz_full_kick_cpu(cpu);

is IRQ safe, whereas the current thingy is not. Sure, its a wee bit less
efficient, but do we really care?
--
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]


#1286625

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2015-12-08 17:00 +0100
Message-ID<qDsQh-4f7-7@gated-at.bofh.it>
In reply to#1281883
On Wed, Dec 02, 2015 at 04:09:08PM +0100, Peter Zijlstra wrote:
> On Wed, Dec 02, 2015 at 03:08:16PM +0100, Frederic Weisbecker wrote:
> > On Wed, Dec 02, 2015 at 11:56:33AM +0100, Peter Zijlstra wrote:
> > > On Tue, Dec 01, 2015 at 11:20:28PM +0100, Frederic Weisbecker wrote:
> > > > > > +	prev = fetch_or(dep, BIT_MASK(bit));
> > > > > > +	if (!prev) {
> > > > > > +		/*
> > > > > > +		* We need the IPIs to be sent from sane process context.
> > > > > 
> > > > > Why ?
> > > > 
> > > > Because posix timers code is all called with interrupts disabled and we can't
> > > > send IPIs then.
> > > > 
> > > > > 
> > > > > > +		* The posix cpu timers are always set with irqs disabled.
> > > > > > +		*/
> > > > > > +		schedule_work(&kick_all_work);
> > > > > > +	}
> > > > > > +}
> > > > > > +
> > > > > > +/*
> > > > > > + * Set a global tick dependency. Lets do the wide IPI kick asynchronously
> > > > > > + * for callers with irqs disabled.
> > > > > 
> > > > > This seems to suggest you can call this with IRQs disabled
> > > > 
> > > > Ah right, that's a misleading comment. We need to use the _delayed() version
> > > > when interrupts are disabled.
> > > 
> > > Why can't you use tick_nohz_full_kick_cpu() for all that, which is
> > > usable from IRQ context and avoid all that delayed muck?
> > 
> > Because I need to kick all the CPUs where the task/signal is running on. That's
> > a bit difficult to do though. I think we had something to try to send an IPI to a task,
> > but I can't retrieve it. Looks easy to do anyway. But in the signal case I'd need to do
> > that for all tasks in the group. That sounds like a costly loop.
> > 
> > So I simplify that with a global IPI.
> 
> Sure, but what I meant was that:
> 
> 	for_each_cpu(cpu, mask)
> 		tick_nohz_full_kick_cpu(cpu);
> 
> is IRQ safe, whereas the current thingy is not. Sure, its a wee bit less
> efficient, but do we really care?

Right, this overhead probably doesn't matter much. I'll do that and we'll see if
people complain.
--
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]


#1281749

FromPeter Zijlstra <peterz@infradead.org>
Date2015-12-02 13:50 +0100
Message-ID<qBf17-7eb-3@gated-at.bofh.it>
In reply to#1281262
On Tue, Dec 01, 2015 at 11:20:28PM +0100, Frederic Weisbecker wrote:

> > > +static void trace_tick_dependency(unsigned long dep)
> > > +{
> > > +	if (dep & TICK_POSIX_TIMER_MASK) {
> > > +		trace_tick_stop(0, "posix timers running\n");
> > > +		return;
> > > +	}
> > > +
> > > +	if (dep & TICK_PERF_EVENTS_MASK) {
> > > +		trace_tick_stop(0, "perf events running\n");
> > > +		return;
> > > +	}
> > > +
> > > +	if (dep & TICK_SCHED_MASK) {
> > > +		trace_tick_stop(0, "more than 1 task in runqueue\n");
> > > +		return;
> > > +	}
> > > +
> > > +	if (dep & TICK_CLOCK_UNSTABLE_MASK)
> > > +		trace_tick_stop(0, "unstable sched clock\n");
> > > +}
> > 
> > I would suggest ditching the strings and using the
> 
> Using a code value instead?

Duh, it seems I forgot to finish that sentence :/

I meant to say use the ftrace enum stuff. So yes, you encode a value and
the ftrace printing muck will generate a string if and when required.
--
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]


#1281847

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2015-12-02 15:20 +0100
Message-ID<qBgqe-8gM-15@gated-at.bofh.it>
In reply to#1281749
On Wed, Dec 02, 2015 at 01:45:31PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 01, 2015 at 11:20:28PM +0100, Frederic Weisbecker wrote:
> 
> > > > +static void trace_tick_dependency(unsigned long dep)
> > > > +{
> > > > +	if (dep & TICK_POSIX_TIMER_MASK) {
> > > > +		trace_tick_stop(0, "posix timers running\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	if (dep & TICK_PERF_EVENTS_MASK) {
> > > > +		trace_tick_stop(0, "perf events running\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	if (dep & TICK_SCHED_MASK) {
> > > > +		trace_tick_stop(0, "more than 1 task in runqueue\n");
> > > > +		return;
> > > > +	}
> > > > +
> > > > +	if (dep & TICK_CLOCK_UNSTABLE_MASK)
> > > > +		trace_tick_stop(0, "unstable sched clock\n");
> > > > +}
> > > 
> > > I would suggest ditching the strings and using the
> > 
> > Using a code value instead?
> 
> Duh, it seems I forgot to finish that sentence :/
> 
> I meant to say use the ftrace enum stuff. So yes, you encode a value and
> the ftrace printing muck will generate a string if and when required.

Right I first wanted to avoid that because it makes parsing more complicated for
tools but probably it works fine now with libtraceevents.
--
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]


#1281752

FromPeter Zijlstra <peterz@infradead.org>
Date2015-12-02 13:50 +0100
Message-ID<qBf17-7eb-13@gated-at.bofh.it>
In reply to#1281262
On Tue, Dec 01, 2015 at 11:20:28PM +0100, Frederic Weisbecker wrote:
> On Tue, Dec 01, 2015 at 09:41:09PM +0100, Peter Zijlstra wrote:
> > On Fri, Nov 13, 2015 at 03:22:04PM +0100, Frederic Weisbecker wrote:
> > > The tick dependency is evaluated on every IRQ. This is a batch of checks
> > > which determine whether it is safe to stop the tick or not. These checks
> > > are often split in many details: posix cpu timers, scheduler, sched clock,
> > > perf events. Each of which are made of smaller details: posix cpu
> > > timer involves checking process wide timers then thread wide timers. Perf
> > > involves checking freq events then more per cpu details.
> > > 
> > > Checking these details asynchronously every time we update the full
> > > dynticks state bring avoidable overhead and a messy layout.
> > > 
> > > Lets introduce instead tick dependency masks: one for system wide
> > > dependency (unstable sched clock), one for CPU wide dependency (sched,
> > > perf), and task/signal level dependencies. The subsystems are responsible
> > > of setting and clearing their dependency through a set of APIs that will
> > > take care of concurrent dependency mask modifications and kick targets
> > > to restart the relevant CPU tick whenever needed.
> > 
> > Maybe better explain why we need the per task and per signal thingy?
> 
> I'll detail that some more in the changelog. The only user of the per
> task/per signal tick dependency is posix cpu timer. I've been first
> proposing a global tick dependency as soon as any posix cpu timer is
> armed. 

> It simplified everything but some reviewers complained (eg:
> some users might want to run posix timers on housekeepers without
> bothering full dynticks CPUs). I could remove the per signal
> dependency with dispatching it through all threads in the group each
> time there is an update but that's the best I can think of.

Right, I remember some of that. Seems worth preserving these reasons.
Maybe even in code comments near the definition of these things.
--
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]


#1281856

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2015-12-02 15:20 +0100
Message-ID<qBgqf-8gM-31@gated-at.bofh.it>
In reply to#1281752
On Wed, Dec 02, 2015 at 01:48:06PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 01, 2015 at 11:20:28PM +0100, Frederic Weisbecker wrote:
> > On Tue, Dec 01, 2015 at 09:41:09PM +0100, Peter Zijlstra wrote:
> > > On Fri, Nov 13, 2015 at 03:22:04PM +0100, Frederic Weisbecker wrote:
> > > > The tick dependency is evaluated on every IRQ. This is a batch of checks
> > > > which determine whether it is safe to stop the tick or not. These checks
> > > > are often split in many details: posix cpu timers, scheduler, sched clock,
> > > > perf events. Each of which are made of smaller details: posix cpu
> > > > timer involves checking process wide timers then thread wide timers. Perf
> > > > involves checking freq events then more per cpu details.
> > > > 
> > > > Checking these details asynchronously every time we update the full
> > > > dynticks state bring avoidable overhead and a messy layout.
> > > > 
> > > > Lets introduce instead tick dependency masks: one for system wide
> > > > dependency (unstable sched clock), one for CPU wide dependency (sched,
> > > > perf), and task/signal level dependencies. The subsystems are responsible
> > > > of setting and clearing their dependency through a set of APIs that will
> > > > take care of concurrent dependency mask modifications and kick targets
> > > > to restart the relevant CPU tick whenever needed.
> > > 
> > > Maybe better explain why we need the per task and per signal thingy?
> > 
> > I'll detail that some more in the changelog. The only user of the per
> > task/per signal tick dependency is posix cpu timer. I've been first
> > proposing a global tick dependency as soon as any posix cpu timer is
> > armed. 
> 
> > It simplified everything but some reviewers complained (eg:
> > some users might want to run posix timers on housekeepers without
> > bothering full dynticks CPUs). I could remove the per signal
> > dependency with dispatching it through all threads in the group each
> > time there is an update but that's the best I can think of.
> 
> Right, I remember some of that. Seems worth preserving these reasons.
> Maybe even in code comments near the definition of these things.

Agreed! I'll comment that some more.

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/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web