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


Groups > linux.kernel > #1491703

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

From Dmitry Vyukov <dvyukov@google.com>
Newsgroups linux.kernel
Subject Re: [PATCH] kcov: properly check if we are in an interrupt
Date 2016-09-27 08:30 +0200
Message-ID <slU3U-64K-3@gated-at.bofh.it> (permalink)
References <skA7g-5af-33@gated-at.bofh.it> <slNF7-1NX-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, Sep 27, 2016 at 1:32 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Fri, 23 Sep 2016 16:51:13 +0200 Andrey Konovalov <andreyknvl@google.com> 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@google.com>
>> ---
>> 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?


I suspect there is a bunch of places that use in_interrupt(), but mean
the same as KCOV wants -- am I in interrupt? and not am I in interrupt
context or in normal task context but inside local_bh_disable(). For
example, why does fput handles closure asynchronously if the task
called local_bh_disable?

264 void fput(struct file *file)
265 {
266         if (atomic_long_dec_and_test(&file->f_count)) {
267                 struct task_struct *task = current;
268
269                 if (likely(!in_interrupt() && !(task->flags &
PF_KTHREAD))) {
270                         init_task_work(&file->f_u.fu_rcuhead, ____fput);
271                         if (!task_work_add(task,
&file->f_u.fu_rcuhead, true))
272                                 return;
273                         /*
274                          * After this task has run exit_task_work(),
275                          * task_work_add() will fail.  Fall
through to delayed
276                          * fput to avoid leaking *file.
277                          */
278                 }
279
280                 if (llist_add(&file->f_u.fu_llist, &delayed_fput_list))
281                         schedule_delayed_work(&delayed_fput_work, 1);
282         }
283 }




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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] kcov: properly check if we are in an interrupt Andrey Konovalov <andreyknvl@google.com> - 2016-09-23 17:00 +0200
  Re: [PATCH] kcov: properly check if we are in an interrupt Andrew Morton <akpm@linux-foundation.org> - 2016-09-27 01:40 +0200
    Re: [PATCH] kcov: properly check if we are in an interrupt Dmitry Vyukov <dvyukov@google.com> - 2016-09-27 08:30 +0200
      Re: [PATCH] kcov: properly check if we are in an interrupt Peter Zijlstra <peterz@infradead.org> - 2016-09-27 09:40 +0200
        Re: [PATCH] kcov: properly check if we are in an interrupt Dmitry Vyukov <dvyukov@google.com> - 2016-09-27 10:00 +0200
          Re: [PATCH] kcov: properly check if we are in an interrupt Peter Zijlstra <peterz@infradead.org> - 2016-09-27 13:00 +0200
            Re: [PATCH] kcov: properly check if we are in an interrupt Dmitry Vyukov <dvyukov@google.com> - 2016-09-27 14:40 +0200
          Re: [PATCH] kcov: properly check if we are in an interrupt Vegard Nossum <vegard.nossum@oracle.com> - 2016-09-27 13:30 +0200
            Re: [PATCH] kcov: properly check if we are in an interrupt Vegard Nossum <vegard.nossum@oracle.com> - 2016-09-27 13:30 +0200

csiph-web