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

[Dmitry Vyukov]

On Tue, Sep 27, 2016 at 12:59 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Tue, Sep 27, 2016 at 09:50:41AM +0200, Dmitry Vyukov wrote:
>> On Tue, Sep 27, 2016 at 9:34 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>> > On Tue, Sep 27, 2016 at 08:21:32AM +0200, Dmitry Vyukov wrote:
>> >>
>> >> 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?
>> >
>> > Agreed, but it would mean auditing all in_interrupt()/irq_count() users.
>>
>>
>> I don't think this means auditing all users. We are not making things
>> worse by introduction of a new predicate.
>> It would be nice to look at some uses in core code, but the only place
>> with observed harm is KCOV.
>>
>> Any naming suggestions? Other than really_in_interrupt or
>> in_interrupt_and_not_in_bh_disabled?
>
> Hence the suggestion to audit and fix instead of making a bigger mess :/


Ah, I see what you mean.
I am not sure Andrey and me are well equipped to tackle it. We have
very narrow kernel experience. Lots of uses are in drivers, which we
don't even know how to invoke. Guessing re current state of the
things, calling potentially expensive functions with bh disabled is
probably bad, but we have no idea as to the threshold.

Any ideas regarding a shorter term fix? We have a very real problem in
KCOV. We could open code and comment the new check in KCOV. Or maybe
do a global s/in_interrupt/in_interrupt_or_bh_disabled/ and then
introduce new in_interrupt. I don't know what's the practice with such
global changes.