linux-kernel - Re: [PATCH v8 12/18] unwind deferred: Use SRCU unwind_deferred_task

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: <CAEf4BzYrz8-aBW0q7wwMOyO3v0ByLuFBRLtBQhSe1fesXwrPWw@mail.gmail.com>
Date: Mon, 12 May 2025 09:17:19 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org, 
	bpf@...r.kernel.org, x86@...nel.org, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Josh Poimboeuf <jpoimboe@...nel.org>, 
	Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, Jiri Olsa <jolsa@...nel.org>, 
	Namhyung Kim <namhyung@...nel.org>
Subject: Re: [PATCH v8 12/18] unwind deferred: Use SRCU unwind_deferred_task_work()

On Sat, May 10, 2025 at 6:41 AM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Fri, 9 May 2025 14:49:37 -0700
> Andrii Nakryiko <andrii.nakryiko@...il.com> wrote:
>
> > > @@ -133,13 +135,15 @@ static void unwind_deferred_task_work(struct callback_head *head)
> > >
> > >         timestamp = info->timestamp;
> > >
> > > -       guard(mutex)(&callback_mutex);
> > > -       list_for_each_entry(work, &callbacks, list) {
> > > +       idx = srcu_read_lock(&unwind_srcu);
> >
> > nit: you could have used guard(srcu)(&unwind_srcu) ?
>
> Then it would be a scope_guard() as it is only needed for the list. I
> prefer using guard() when it is most of the function that is being
> protected. Here it's just the list and nothing else.
>
> One issue I have with guard() is that it tends to "leak". That is, if you
> use it to protect only one thing and then add more after what you are
> protecting, then the guard ends up protecting more than it needs to.
>

Yep, makes sense. I just noticed the use of guard() for mutex before,
so assumes the same could be done for SRCU, but what you are saying
makes sense, no problem.

> If anything, I would make the loop into its own function with the guard()
> then it is more obvious. But for now, I think it's fine as is, unless
> others prefer the switch.
>
> -- Steve
>
> >
> > > +       list_for_each_entry_srcu(work, &callbacks, list,
> > > +                                srcu_read_lock_held(&unwind_srcu)) {
> > >                 if (task->unwind_mask & (1UL << work->bit)) {
> > >                         work->func(work, &trace, timestamp);
> > >                         clear_bit(work->bit, &current->unwind_mask);
> > >                 }
> > >         }
> > > +       srcu_read_unlock(&unwind_srcu, idx);
> > >  }
> > >