Re: [PATCH] kcov: properly check if we are in an interrupt

From: Andrew Morton
Date: Mon Sep 26 2016 - 19:32:53 EST


On Fri, 23 Sep 2016 16:51:13 +0200 Andrey Konovalov <andreyknvl@xxxxxxxxxx> wrote:

> in_interrupt() returns a nonzero value when we are either in an
> interrupt or have bh disabled via local_bh_disable(). Since we are
> interested in only ignoring coverage from actual interrupts, do a
> proper check of whether we are really in an interrupt.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> ---
> It would look totally better to reuse in_irq(), in_serving_softirq() and
> in_nmi() instead of checking flags manually, but that leads to slower
> generated code (three separate tests for each of the flags). Would it be
> better to add another macro to preempt.h that would check if we're actually
> in interrupt and use it?

Yes please. Is there anywhere else where such a macro can be used?

> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -54,7 +54,8 @@ void notrace __sanitizer_cov_trace_pc(void)
> * We are interested in code coverage as a function of a syscall inputs,
> * so we ignore code executed in interrupts.
> */
> - if (!t || in_interrupt())
> + if (!t || (preempt_count() & (HARDIRQ_MASK | SOFTIRQ_OFFSET
> + | NMI_MASK)))

Or include a prominent and very apologetic comment here explaining why
it is open-coded. But I do agree that not open-coding it is better.