Re: [PATCH] auxdisplay: Remove in_interrupt() usage.

From: Miguel Ojeda
Date: Mon Feb 08 2021 - 17:30:38 EST


On Mon, Feb 8, 2021 at 9:41 PM Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
>
> It hurts to keep in_interrupt() because it is desired to have it removed
> from drivers. The problem is that pattern is often copied and people
> sometimes get it wrong. For instance, the code here invoked schedule()
> based on in_interrupt(). It did not check whether or not the interrupts
> are disabled which is also important. It may work now, it can break in
> future if an unrelated change is made. An example is commit
> c2d0f1a65ab9f ("scsi: libsas: Introduce a _gfp() variant of event notifiers")
> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git/commit/?id=c2d0f1a65ab9f
>
> in_interrupt() is often used in old code that was written before
> might_sleep() and lockdep was introduced.

Thanks -- can you please add a Link: tag to a lore URL or the docs or
similar where more information can be found regarding the
proposal/discussion for removing `in_interrurt()` etc.? It is useful
to track why these things are happening around the kernel.

Also, `hacking.rst` (and related documentation) should be updated
before this is done, so that we can link to it.

> What I meant was GFP_KERNEL for context which can sleep vs GFP_ATOMIC for
> context which must not sleep. The commit above also eliminates the
> in_interrupt() usage within the driver (in multiple steps).

I was thinking something along those lines, but `in_interrupt()` nor
`cond_resched()` take an explicit context either, so I am confused.
Does `cond_reched()` always do the right thing regardless of context?
The docs are not really clear:

"cond_resched() and cond_resched_lock(): latency reduction via
explicit rescheduling in places that are safe."

It could be read as "it will only resched whenever safe" or "only to
be called if it is safe".

Cheers,
Miguel