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

Groups > linux.kernel > #1239773 > unrolled thread

Re: [PATCH v7 05/11] task_isolation: add debug boot flag

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 v7 05/11] task_isolation: add debug boot flag Luiz Capitulino <lcapitulino@redhat.com> - 2015-10-05 19:10 +0200
    Re: [PATCH v7 05/11] task_isolation: add debug boot flag Chris Metcalf <cmetcalf@ezchip.com> - 2015-10-08 02:40 +0200
      Re: [PATCH v7 05/11] task_isolation: add debug boot flag Luiz Capitulino <lcapitulino@redhat.com> - 2015-10-08 22:30 +0200

#1239773 — Re: [PATCH v7 05/11] task_isolation: add debug boot flag

On Mon, 28 Sep 2015 11:17:20 -0400
Chris Metcalf <cmetcalf@ezchip.com> wrote:

> The new "task_isolation_debug" flag simplifies debugging
> of TASK_ISOLATION kernels when processes are running in
> PR_TASK_ISOLATION_ENABLE mode.  Such processes should get no
> interrupts from the kernel, and if they do, when this boot flag is
> specified a kernel stack dump on the console is generated.
> 
> It's possible to use ftrace to simply detect whether a task_isolation
> core has unexpectedly entered the kernel.  But what this boot flag
> does is allow the kernel to provide better diagnostics, e.g. by
> reporting in the IPI-generating code what remote core and context
> is preparing to deliver an interrupt to a task_isolation core.
> 
> It may be worth considering other ways to generate useful debugging
> output rather than console spew, but for now that is simple and direct.

Honest question: does any of the task_isolation_debug() calls added
by this patch take care of the case where vmstat_shepherd() may
schedule vmstat_update() to run because a TASK_ISOLATION process is
changing memory stats?

If that's not taken care of yet, should we? I just don't know if we
should call task_isolation_exception() or task_isolation_debug().
In the case of the latter, wouldn't it be interesting to add it to
__queue_work() then?

> 
> Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
> ---
>  Documentation/kernel-parameters.txt |  7 +++++++
>  include/linux/isolation.h           |  2 ++
>  kernel/irq_work.c                   |  5 ++++-
>  kernel/sched/core.c                 | 21 +++++++++++++++++++++
>  kernel/signal.c                     |  5 +++++
>  kernel/smp.c                        |  4 ++++
>  kernel/softirq.c                    |  7 +++++++
>  7 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 22a4b687ea5b..48ff15f3166f 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3623,6 +3623,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			neutralize any effect of /proc/sys/kernel/sysrq.
>  			Useful for debugging.
>  
> +	task_isolation_debug	[KNL]
> +			In kernels built with CONFIG_TASK_ISOLATION and booted
> +			in nohz_full= mode, this setting will generate console
> +			backtraces when the kernel is about to interrupt a
> +			task that has requested PR_TASK_ISOLATION_ENABLE
> +			and is running on a nohz_full core.
> +
>  	tcpmhash_entries= [KNL,NET]
>  			Set the number of tcp_metrics_hash slots.
>  			Default value is 8192 or 16384 depending on total
> diff --git a/include/linux/isolation.h b/include/linux/isolation.h
> index 27a4469831c1..9f1747331a36 100644
> --- a/include/linux/isolation.h
> +++ b/include/linux/isolation.h
> @@ -18,11 +18,13 @@ extern void task_isolation_enter(void);
>  extern void task_isolation_syscall(int nr);
>  extern void task_isolation_exception(void);
>  extern void task_isolation_wait(void);
> +extern void task_isolation_debug(int cpu);
>  #else
>  static inline bool task_isolation_enabled(void) { return false; }
>  static inline void task_isolation_enter(void) { }
>  static inline void task_isolation_syscall(int nr) { }
>  static inline void task_isolation_exception(void) { }
> +static inline void task_isolation_debug(int cpu) { }
>  #endif
>  
>  static inline bool task_isolation_strict(void)
> diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> index cbf9fb899d92..745c2ea6a4e4 100644
> --- a/kernel/irq_work.c
> +++ b/kernel/irq_work.c
> @@ -17,6 +17,7 @@
>  #include <linux/cpu.h>
>  #include <linux/notifier.h>
>  #include <linux/smp.h>
> +#include <linux/isolation.h>
>  #include <asm/processor.h>
>  
>  
> @@ -75,8 +76,10 @@ bool irq_work_queue_on(struct irq_work *work, int cpu)
>  	if (!irq_work_claim(work))
>  		return false;
>  
> -	if (llist_add(&work->llnode, &per_cpu(raised_list, cpu)))
> +	if (llist_add(&work->llnode, &per_cpu(raised_list, cpu))) {
> +		task_isolation_debug(cpu);
>  		arch_send_call_function_single_ipi(cpu);
> +	}
>  
>  	return true;
>  }
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 3595403921bd..8ddabb0d7510 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -74,6 +74,7 @@
>  #include <linux/binfmts.h>
>  #include <linux/context_tracking.h>
>  #include <linux/compiler.h>
> +#include <linux/isolation.h>
>  
>  #include <asm/switch_to.h>
>  #include <asm/tlb.h>
> @@ -743,6 +744,26 @@ bool sched_can_stop_tick(void)
>  }
>  #endif /* CONFIG_NO_HZ_FULL */
>  
> +#ifdef CONFIG_TASK_ISOLATION
> +/* Enable debugging of any interrupts of task_isolation cores. */
> +static int task_isolation_debug_flag;
> +static int __init task_isolation_debug_func(char *str)
> +{
> +	task_isolation_debug_flag = true;
> +	return 1;
> +}
> +__setup("task_isolation_debug", task_isolation_debug_func);
> +
> +void task_isolation_debug(int cpu)
> +{
> +	if (task_isolation_debug_flag && tick_nohz_full_cpu(cpu) &&
> +	    (cpu_curr(cpu)->task_isolation_flags & PR_TASK_ISOLATION_ENABLE)) {
> +		pr_err("Interrupt detected for task_isolation cpu %d\n", cpu);
> +		dump_stack();
> +	}
> +}
> +#endif
> +
>  void sched_avg_update(struct rq *rq)
>  {
>  	s64 period = sched_avg_period();
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 0f6bbbe77b46..c6e09f0f7e24 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -684,6 +684,11 @@ int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
>   */
>  void signal_wake_up_state(struct task_struct *t, unsigned int state)
>  {
> +#ifdef CONFIG_TASK_ISOLATION
> +	/* If the task is being killed, don't complain about task_isolation. */
> +	if (state & TASK_WAKEKILL)
> +		t->task_isolation_flags = 0;
> +#endif
>  	set_tsk_thread_flag(t, TIF_SIGPENDING);
>  	/*
>  	 * TASK_WAKEKILL also means wake it up in the stopped/traced/killable
> diff --git a/kernel/smp.c b/kernel/smp.c
> index 07854477c164..b0bddff2693d 100644
> --- a/kernel/smp.c
> +++ b/kernel/smp.c
> @@ -14,6 +14,7 @@
>  #include <linux/smp.h>
>  #include <linux/cpu.h>
>  #include <linux/sched.h>
> +#include <linux/isolation.h>
>  
>  #include "smpboot.h"
>  
> @@ -178,6 +179,7 @@ static int generic_exec_single(int cpu, struct call_single_data *csd,
>  	 * locking and barrier primitives. Generic code isn't really
>  	 * equipped to do the right thing...
>  	 */
> +	task_isolation_debug(cpu);
>  	if (llist_add(&csd->llist, &per_cpu(call_single_queue, cpu)))
>  		arch_send_call_function_single_ipi(cpu);
>  
> @@ -457,6 +459,8 @@ void smp_call_function_many(const struct cpumask *mask,
>  	}
>  
>  	/* Send a message to all CPUs in the map */
> +	for_each_cpu(cpu, cfd->cpumask)
> +		task_isolation_debug(cpu);
>  	arch_send_call_function_ipi_mask(cfd->cpumask);
>  
>  	if (wait) {
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 479e4436f787..ed762fec7265 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -24,8 +24,10 @@
>  #include <linux/ftrace.h>
>  #include <linux/smp.h>
>  #include <linux/smpboot.h>
> +#include <linux/context_tracking.h>
>  #include <linux/tick.h>
>  #include <linux/irq.h>
> +#include <linux/isolation.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/irq.h>
> @@ -335,6 +337,11 @@ void irq_enter(void)
>  		_local_bh_enable();
>  	}
>  
> +	if (context_tracking_cpu_is_enabled() &&
> +	    context_tracking_in_user() &&
> +	    !in_interrupt())
> +		task_isolation_debug(smp_processor_id());
> +
>  	__irq_enter();
>  }
>  

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

#1241892

On 10/5/2015 1:07 PM, Luiz Capitulino wrote:
> On Mon, 28 Sep 2015 11:17:20 -0400
> Chris Metcalf <cmetcalf@ezchip.com> wrote:
>
>> The new "task_isolation_debug" flag simplifies debugging
>> of TASK_ISOLATION kernels when processes are running in
>> PR_TASK_ISOLATION_ENABLE mode.  Such processes should get no
>> interrupts from the kernel, and if they do, when this boot flag is
>> specified a kernel stack dump on the console is generated.
>>
>> It's possible to use ftrace to simply detect whether a task_isolation
>> core has unexpectedly entered the kernel.  But what this boot flag
>> does is allow the kernel to provide better diagnostics, e.g. by
>> reporting in the IPI-generating code what remote core and context
>> is preparing to deliver an interrupt to a task_isolation core.
>>
>> It may be worth considering other ways to generate useful debugging
>> output rather than console spew, but for now that is simple and direct.
> Honest question: does any of the task_isolation_debug() calls added
> by this patch take care of the case where vmstat_shepherd() may
> schedule vmstat_update() to run because a TASK_ISOLATION process is
> changing memory stats?

The task_isolation_debug() calls don't "take care of" any cases - they are
really just there to generate console dumps when the kernel unexpectedly
interrupts a task_isolated task.

The idea with vmstat is that before a task_isolated task returns to
userspace, it quiesces the vmstat thread (does a final sweep to collect
the stats and turns off the scheduled work item).  As a result, the vmstat
shepherd won't run while the task is in userspace.  When and if it returns
to the kernel, it will again sweep up the stats before returning to userspace.

The usual shepherd mechanism on a housekeeping core might notice
that the task had entered the kernel and started changing stats, and
might then asynchronously restart the scheduled work, but it should be
quiesced again regardless on the way back out to userspace.

> If that's not taken care of yet, should we? I just don't know if we
> should call task_isolation_exception() or task_isolation_debug().

task_isolation_exception() is called when an exception (page fault or
similar) is generated synchronously by the running task and we want
to make sure to notify the task with a signal if it has set up STRICT mode
to indicate that it is not planning to enter the kernel.

> In the case of the latter, wouldn't it be interesting to add it to
> __queue_work() then?

Well, queuing remote work involves sending an IPI, and we already tag
both the SMP send side AND the client side IRQ side with a task_isolation_debug(),
so I expect in practice it would be detected.

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

#1242719

On Wed, 7 Oct 2015 20:33:56 -0400
Chris Metcalf <cmetcalf@ezchip.com> wrote:

> On 10/5/2015 1:07 PM, Luiz Capitulino wrote:
> > On Mon, 28 Sep 2015 11:17:20 -0400
> > Chris Metcalf <cmetcalf@ezchip.com> wrote:
> >
> >> The new "task_isolation_debug" flag simplifies debugging
> >> of TASK_ISOLATION kernels when processes are running in
> >> PR_TASK_ISOLATION_ENABLE mode.  Such processes should get no
> >> interrupts from the kernel, and if they do, when this boot flag is
> >> specified a kernel stack dump on the console is generated.
> >>
> >> It's possible to use ftrace to simply detect whether a task_isolation
> >> core has unexpectedly entered the kernel.  But what this boot flag
> >> does is allow the kernel to provide better diagnostics, e.g. by
> >> reporting in the IPI-generating code what remote core and context
> >> is preparing to deliver an interrupt to a task_isolation core.
> >>
> >> It may be worth considering other ways to generate useful debugging
> >> output rather than console spew, but for now that is simple and direct.
> > Honest question: does any of the task_isolation_debug() calls added
> > by this patch take care of the case where vmstat_shepherd() may
> > schedule vmstat_update() to run because a TASK_ISOLATION process is
> > changing memory stats?
> 
> The task_isolation_debug() calls don't "take care of" any cases - they are
> really just there to generate console dumps when the kernel unexpectedly
> interrupts a task_isolated task.
> 
> The idea with vmstat is that before a task_isolated task returns to
> userspace, it quiesces the vmstat thread (does a final sweep to collect
> the stats and turns off the scheduled work item).  As a result, the vmstat
> shepherd won't run while the task is in userspace.  When and if it returns
> to the kernel, it will again sweep up the stats before returning to userspace.
> 
> The usual shepherd mechanism on a housekeeping core might notice
> that the task had entered the kernel and started changing stats, and
> might then asynchronously restart the scheduled work, but it should be
> quiesced again regardless on the way back out to userspace.

OK, I've missed the (obvious) fact that the process has to enter the
kernel to change stats. Thanks a lot for your explanation.

> > If that's not taken care of yet, should we? I just don't know if we
> > should call task_isolation_exception() or task_isolation_debug().
> 
> task_isolation_exception() is called when an exception (page fault or
> similar) is generated synchronously by the running task and we want
> to make sure to notify the task with a signal if it has set up STRICT mode
> to indicate that it is not planning to enter the kernel.
> 
> > In the case of the latter, wouldn't it be interesting to add it to
> > __queue_work() then?
> 
> Well, queuing remote work involves sending an IPI, and we already tag
> both the SMP send side AND the client side IRQ side with a task_isolation_debug(),
> so I expect in practice it would be detected.
> 

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