linux-kernel - Re: [RFC PATCH v3 03/10] task_work, sched: Add a _locked variant to task_work

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]

Message-ID: <xhsmhle221qs8.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
Date: Mon, 15 Jul 2024 19:57:59 +0200
From: Valentin Schneider <vschneid@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, rcu@...r.kernel.org, Ingo Molnar
 <mingo@...hat.com>, Juri Lelli <juri.lelli@...hat.com>, Vincent Guittot
 <vincent.guittot@...aro.org>, Dietmar Eggemann <dietmar.eggemann@....com>,
 Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Mel
 Gorman <mgorman@...e.de>, Phil Auld <pauld@...hat.com>, Clark Williams
 <williams@...hat.com>, Tomas Glozar <tglozar@...hat.com>, "Paul E.
 McKenney" <paulmck@...nel.org>, Frederic Weisbecker <frederic@...nel.org>,
 Neeraj Upadhyay <neeraj.upadhyay@...nel.org>, Joel Fernandes
 <joel@...lfernandes.org>, Josh Triplett <josh@...htriplett.org>, Boqun
 Feng <boqun.feng@...il.com>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Lai Jiangshan <jiangshanlai@...il.com>,
 Zqiang <qiang.zhang1211@...il.com>, Alexander Gordeev
 <agordeev@...ux.ibm.com>, Catalin Marinas <catalin.marinas@....com>, Arnd
 Bergmann <arnd@...db.de>, Guo Ren <guoren@...nel.org>, Palmer Dabbelt
 <palmer@...osinc.com>, Andrew Morton <akpm@...ux-foundation.org>, Oleg
 Nesterov <oleg@...hat.com>, Jens Axboe <axboe@...nel.dk>
Subject: Re: [RFC PATCH v3 03/10] task_work, sched: Add a _locked variant to
 task_work_cancel()

On 12/07/24 17:20, Peter Zijlstra wrote:
> On Thu, Jul 11, 2024 at 02:59:57PM +0200, Valentin Schneider wrote:
>> Later commits will need to issue a task_work_cancel() from within the
>> scheduler with the task's ->pi_lock held.
>> 
>> Add a _locked variant that expects p->pi_lock to be held. Expose it in a
>> separate scheduler header file, as this really is a scheduler-only
>> interface.
>> 
>> Signed-off-by: Valentin Schneider <vschneid@...hat.com>
>> ---
>>  kernel/sched/task_work_sched.h | 14 +++++++
>>  kernel/task_work.c             | 67 ++++++++++++++++++++++++++--------
>>  2 files changed, 66 insertions(+), 15 deletions(-)
>>  create mode 100644 kernel/sched/task_work_sched.h
>> 
>> diff --git a/kernel/sched/task_work_sched.h b/kernel/sched/task_work_sched.h
>> new file mode 100644
>> index 0000000000000..e235da456427f
>> --- /dev/null
>> +++ b/kernel/sched/task_work_sched.h
>> @@ -0,0 +1,14 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Scheduler internal task_work methods
>> + */
>> +#ifndef _KERNEL_TASK_WORK_SCHED_H
>> +#define _KERNEL_TASK_WORK_SCHED_H
>> +
>> +#include <linux/task_work.h>
>> +#include <linux/sched.h>
>> +
>> +struct callback_head *
>> +task_work_cancel_locked(struct task_struct *task, task_work_func_t func);
>> +
>> +#endif
>
>
> Do we really need that exposed? Can't we squirrel that way in
> kernel/sched/sched.h and forget about it?
>

Nah that's not required, I thought a clean cut header would be neater but
given its single user, tossing that to sched.h looks better.

>> +struct callback_head *
>> +task_work_cancel_locked(struct task_struct *task, task_work_func_t func)
>> +{
>> +	lockdep_assert_held(&task->pi_lock);
>
> I'm thinking that lockde_assert wants to live in your _locked function
> above.
>

Quite so!