All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Wander Lairson Costa <wander@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Andy Lutomirski <luto@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Andrei Vagin <avagin@gmail.com>,
	open list <linux-kernel@vger.kernel.org>,
	Hu Chunyu <chuhu@redhat.com>, Oleg Nesterov <oleg@redhat.com>,
	Valentin Schneider <vschneid@redhat.com>,
	Paul McKenney <paulmck@kernel.org>
Subject: Re: [PATCH v4] kernel/fork: beware of __put_task_struct calling context
Date: Fri, 10 Feb 2023 18:08:47 +0100	[thread overview]
Message-ID: <Y+Z6H4eiWChxHF4a@linutronix.de> (raw)
In-Reply-To: <20230206170927.9d5afee653dfa0738983dbfa@linux-foundation.org>

On 2023-02-06 17:09:27 [-0800], Andrew Morton wrote:
> On Mon,  6 Feb 2023 10:04:47 -0300 Wander Lairson Costa <wander@redhat.com> wrote:
> 
> > Under PREEMPT_RT, __put_task_struct() indirectly acquires sleeping
> > locks. Therefore, it can't be called from an non-preemptible context.
> 
> Well that's regrettable.  Especially if non-preempt kernels don't do
> this.

Non-preemptible context on PREEMPT_RT. Interrupts handler and timers
don't count as non-preemptible because interrupt handler are threaded
and hrtimers are invoked in softirq context (which is preemptible on
PREEMPT_RT).

This here is different because the hrtimer in question was marked as
HRTIMER_MODE_REL_HARD. In this case it is invoked in hardirq context as
requested with all the problems that follow.

> Why does PREEMPT_RT do this and can it be fixed?

PREEMPT_RT tries to move as much as it can out of hardirq context into
preemptible context. A spinlock_t is preemptible on PREEMPT_RT while
it is not in other preemption models. The scheduler needs to use
raw_spinlock_t in order to be able to schedule a task from
hardirq-context without a deadlock.
For memory allocation only sleeping locks (spinlock_t) is used since
there are no memory allocation/ deallocation on PREEMPT_RT in hardirq
context. These two need to be separated.

> If it cannot be fixed then we should have a might_sleep() in
> __put_task_struct() for all kernel configurations, along with an
> apologetic comment explaining why.

__put_task_struct() should not be invoked in atomic context on
PREEMPT_RT. It is fine however in a regular timer hrtimer. Adding
might_sleep() will trigger a lot of false positives on a preemptible
kernel and RT.

A might_lock() on a spinlock_t should do the trick from LOCKDEP
perspective if CONFIG_PROVE_RAW_LOCK_NESTING is enabled.
In this case it should be visible due to rq-lock or due to hrtimer.

Sebastian

      parent reply	other threads:[~2023-02-10 17:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-06 13:04 [PATCH v4] kernel/fork: beware of __put_task_struct calling context Wander Lairson Costa
2023-02-06 14:56 ` Sebastian Andrzej Siewior
2023-02-06 15:27   ` Oleg Nesterov
2023-02-06 16:04     ` Sebastian Andrzej Siewior
2023-02-06 16:27       ` Oleg Nesterov
2023-02-10 16:48         ` Sebastian Andrzej Siewior
2023-02-06 18:36       ` Wander Lairson Costa
2023-02-06 18:34     ` Wander Lairson Costa
2023-02-06 18:32   ` Wander Lairson Costa
2023-02-07  1:09 ` Andrew Morton
2023-02-07 15:26   ` Wander Lairson Costa
2023-02-10 17:08   ` Sebastian Andrzej Siewior [this message]

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=Y+Z6H4eiWChxHF4a@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=avagin@gmail.com \
    --cc=chuhu@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=fenghua.yu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=vschneid@redhat.com \
    --cc=wander@redhat.com \
    /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.