Re: [PATCH 4/4] sched/cache: Introduce task_struct->sched_cache_grp

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Sep 14, 2026 at 10:34:20PM +0800, Chen Yu wrote:
> On Thu, Sep 10, 2026 at 03:50:56PM -0700, Tim Chen wrote:
> > On Thu, 2026-09-10 at 21:19 +0200, Peter Zijlstra wrote:
> > > On Thu, Sep 10, 2026 at 10:46:12AM -0700, Tim Chen wrote:
> > > > @@ -882,6 +882,20 @@ static int exec_mmap(struct linux_binprm *bprm)
> > > > +
> > > > +		old_grp = rcu_dereference_protected(tsk->sched_cache_grp, true);
> > > > +
> > > 
> > > Also: rcu_dereference_protected(.c = true) is another offence, that's
> > > just wrong.
> 
> Got it, will fix it.
> If I understand correctly, the "c" should never pass an explicit const value?

Right, that is a bit of an anti-pattern. The 'c' thing is there to
validate your claim of this in fact being an update side, eg. you
holding the write lock and all that.

> The original idea was that, in exec_mmap(), the current running task is the only
> writer to change tsk->sched_cache_grp, so the condition is true,
> 
> To fix it, it might be turned into the following for now:
> rcu_dereference_protected(tsk->sched_cache_grp, tsk == current);
> like deref_curr_numa_group().
> 
> In the future if task tagging is supported, the race condition of multiple
> updates would require a pi_lock for this task, and it will become:
> rcu_dereference_protected(tsk->sched_cache_grp, lockdep_is_held(&p->pi_lock));
> like what we have done in the prctl based cache aware scheduling patch set:
> https://lore.kernel.org/lkml/50fe2db1a62ea2376a87d0c14778b1ff456d11ec.1787955777.git.tim.c.chen@linux.intel.com/

Right, it might also make sense to have a helper, like:

#define rcu_deref_sched_cache_grp(tsk) \
	rcu_dereference_protected((tsk)->sched_cache_grp, (tsk) == current)

such that you don't have to keep repeating this thing over and over.
Also consider if rcu_dereference_protected() makes sense, I tend to
unconditionally use rcu_dereference_*check().

The only architecture where it really makes a difference is Alpha, and
nobody cares about that anymore.