[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241008073800.GD14587@noisy.programming.kicks-ass.net>
Date: Tue, 8 Oct 2024 09:38:00 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Waiman Long <llong@...hat.com>
Cc: Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
Boqun Feng <boqun.feng@...il.com>, linux-kernel@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>,
Luis Goncalves <lgoncalv@...hat.com>, Chunyu Hu <chuhu@...hat.com>
Subject: Re: [PATCH] locking/rtmutex: Always use trylock in rt_mutex_trylock()
On Mon, Oct 07, 2024 at 11:54:54AM -0400, Waiman Long wrote:
>
> On 10/7/24 11:33 AM, Peter Zijlstra wrote:
> > On Mon, Oct 07, 2024 at 11:23:32AM -0400, Waiman Long wrote:
> >
> > > > Is the problem that:
> > > >
> > > > sched_tick()
> > raw_spin_lock(&rq->__lock);
> > > > task_tick_mm_cid()
> > > > task_work_add()
> > > > kasan_save_stack()
> > > > idiotic crap while holding rq->__lock ?
> > > >
> > > > Because afaict that is completely insane. And has nothing to do with
> > > > rtmutex.
> > > >
> > > > We are not going to change rtmutex because instrumentation shit is shit.
> > > Yes, it is because of KASAN that causes page allocation while holding the
> > > rq->__lock. Maybe we can blame KASAN for this. It is actually not a problem
> > > for non-PREEMPT_RT kernel because only trylock is being used. However, we
> > > don't use trylock all the way when rt_spin_trylock() is being used with
> > > PREEMPT_RT Kernel.
> > It has nothing to do with trylock, an everything to do with scheduler
> > locks being special.
> >
> > But even so, trying to squirrel a spinlock inside a raw_spinlock is
> > dodgy at the best of times, yes it mostly works, but should be avoided
> > whenever possible.
> >
> > And instrumentation just doesn't count.
> >
> > > This is certainly a problem that we need to fix as there
> > > may be other similar case not involving rq->__lock lurking somewhere.
> > There cannot be, lock order is:
> >
> > rtmutex->wait_lock
> > task->pi_lock
> > rq->__lock
> >
> > Trying to subvert that order gets you a splat, any other:
> >
> > raw_spin_lock(&foo);
> > spin_trylock(&bar);
> >
> > will 'work', despite probably not being a very good idea.
> >
> > Any case involving the scheduler locks needs to be eradicated, not
> > worked around.
>
> OK, I will see what I can do to work around this issue.
Something like the completely untested below might just work.
---
diff --git a/include/linux/task_work.h b/include/linux/task_work.h
index cf5e7e891a77..6d22414c5a83 100644
--- a/include/linux/task_work.h
+++ b/include/linux/task_work.h
@@ -14,11 +14,14 @@ init_task_work(struct callback_head *twork, task_work_func_t func)
}
enum task_work_notify_mode {
- TWA_NONE,
+ TWA_NONE = 0,
TWA_RESUME,
TWA_SIGNAL,
TWA_SIGNAL_NO_IPI,
TWA_NMI_CURRENT,
+
+ TWA_FLAGS = 0xff00,
+ TWAF_NO_KASAN = 0x0100,
};
static inline bool task_work_pending(struct task_struct *task)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 43e453ab7e20..e9b053b403c0 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -10458,7 +10458,7 @@ void task_tick_mm_cid(struct rq *rq, struct task_struct *curr)
return;
if (time_before(now, READ_ONCE(curr->mm->mm_cid_next_scan)))
return;
- task_work_add(curr, work, TWA_RESUME);
+ task_work_add(curr, work, TWA_RESUME | TWAF_NO_KASAN);
}
void sched_mm_cid_exit_signals(struct task_struct *t)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ab497fafa7be..a58d55bba7a3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3604,7 +3604,7 @@ static void task_tick_numa(struct rq *rq, struct task_struct *curr)
curr->node_stamp += period;
if (!time_before(jiffies, curr->mm->numa_next_scan))
- task_work_add(curr, work, TWA_RESUME);
+ task_work_add(curr, work, TWA_RESUME | TWAF_NO_KASAN);
}
}
diff --git a/kernel/task_work.c b/kernel/task_work.c
index 5d14d639ac71..ff07a77bd7be 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -55,13 +55,18 @@ int task_work_add(struct task_struct *task, struct callback_head *work,
enum task_work_notify_mode notify)
{
struct callback_head *head;
+ int flags = notify & TWA_FLAGS;
+ notify &= ~TWA_FLAGS;
if (notify == TWA_NMI_CURRENT) {
if (WARN_ON_ONCE(task != current))
return -EINVAL;
if (!IS_ENABLED(CONFIG_IRQ_WORK))
return -EINVAL;
- } else {
+ flags |= TWAF_NO_KASAN;
+ }
+
+ if (!(flags & TWAF_NO_KASAN)) {
/* record the work call stack in order to print it in KASAN reports */
kasan_record_aux_stack(work);
}
Powered by blists - more mailing lists