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:   Tue, 9 Feb 2021 10:01:12 +0100
From:   Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
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 2021-02-08 23:26:57 [+0100], Miguel Ojeda wrote:
> 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.

If I post series with more than just one patch I have a cover letter
including:

|in the discussion about preempt count consistency across kernel
|configurations:
|
| https://lore.kernel.org/r/20200914204209.256266093@linutronix.de/
|
|it was concluded that the usage of in_interrupt() and related context
|checks should be removed from non-core code.
|
|In the long run, usage of 'preemptible, in_*irq etc.' should be banned from
|driver code completely.

since this patch was small, simple and removing not required code I kept
it out. Is this enough information for you?

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

The information is not wrong, it doesn't say you have to use it it your
driver. It also does not mention that you should not. I will look into
this.

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

You should keep track of the context you are in and not attempt to sleep
if it is not allowed. If you are doing something that may monopolize the
CPU then you should use cond_resched(). The difference compared to
schedule() is that you don't blindly invoke the scheduler and that it is
optimized away on a preemptible kernel. But as you noticed, you must not
not use it if the context does not allow it (like in interrupt handler,
disabled preemption and so on).

> Cheers,
> Miguel

Sebastian