All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Willy Tarreau <w@1wt.eu>
Subject: Re: [PATCH] auxdisplay: Remove in_interrupt() usage.
Date: Tue, 9 Feb 2021 10:01:12 +0100	[thread overview]
Message-ID: <20210209090112.lewvvhnc2y7oyr27@linutronix.de> (raw)
In-Reply-To: <CANiq72mw47Qa9M6i23Dp+_3M8juBnv33PJ-6zFk++SV57G2-cQ@mail.gmail.com>

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

  reply	other threads:[~2021-02-09  9:09 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-08 17:58 [PATCH] auxdisplay: Remove in_interrupt() usage Sebastian Andrzej Siewior
2021-02-08 18:38 ` Miguel Ojeda
2021-02-08 19:07   ` Sebastian Andrzej Siewior
2021-02-08 20:14     ` Miguel Ojeda
2021-02-08 20:41       ` Sebastian Andrzej Siewior
2021-02-08 22:26         ` Miguel Ojeda
2021-02-09  9:01           ` Sebastian Andrzej Siewior [this message]
2021-02-10 21:46             ` Miguel Ojeda
2021-02-13 16:50               ` [PATCH v3] " Sebastian Andrzej Siewior
2021-02-16  9:32                 ` Miguel Ojeda
2021-02-16 10:28                   ` Sebastian Andrzej Siewior
2021-02-16 12:42                     ` Miguel Ojeda
2021-02-16 18:26                       ` Sebastian Andrzej Siewior
2021-02-16 18:27                         ` [PATCH v4] " Sebastian Andrzej Siewior
2021-02-16 20:21                         ` [PATCH v3] " Miguel Ojeda
2021-03-10 17:51                           ` Sebastian Andrzej Siewior
2021-03-10 18:04                             ` Miguel Ojeda
2021-02-08 19:37   ` [PATCH v2] " Sebastian Andrzej Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210209090112.lewvvhnc2y7oyr27@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=geert@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.