linux-kernel - Re: [PATCH] auxdisplay: Remove in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 8 Feb 2021 23:26:57 +0100
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Willy Tarreau <w@....eu>
Subject: Re: [PATCH] auxdisplay: Remove in_interrupt() usage.

On Mon, Feb 8, 2021 at 9:41 PM Sebastian Andrzej Siewior
<bigeasy@...utronix.de> 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