Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1491703
| 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 |
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 | Next — Previous in thread | Next in thread | Find similar | Unroll 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