Re: [RFC patch v3 01/20] sched: Cache aware load-balancing

[Tim Chen <tim.c.chen@linux.intel.com>]

On Fri, 2025-07-04 at 00:59 +0530, Shrikanth Hegde wrote:
> 
> Hi Tim, Chen,
> skimming through the series and will try to go through in coming days.
> 
> > 

Thanks for taking a look.  Some further comments on top of Chen Yu's response.

[snip]
> >   
> > +#ifdef CONFIG_SCHED_CACHE
> > +	struct callback_head		cache_work;
> > +#endif
> > +
> >   #ifdef CONFIG_RSEQ
> >   	struct rseq __user *rseq;
> >   	u32 rseq_len;
> > diff --git a/init/Kconfig b/init/Kconfig
> > index bf3a920064be..e2509127b6f9 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -953,6 +953,10 @@ config NUMA_BALANCING
> >   
> >   	  This system will be inactive on UMA systems.
> >   
> > +config SCHED_CACHE
> > +	bool "Cache aware scheduler"
> > +	default y
> > +
> 
> Should it depend on EXPERT?
> IMO this could add quite a bit of overhead and maybe n by default?
> 

We do have a SCHED_CACHE scheduler feature in the later patches.  
So the feature could be turned on/off at run time by admin who don't want
to incur this overhead.

> >   config NUMA_BALANCING_DEFAULT_ENABLED
> >   	bool "Automatically enable NUMA aware memory/task placement"
> >   	default y
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index 168681fc4b25..da1387823b9e 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -1332,6 +1332,9 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
> >   	if (mm_alloc_cid(mm, p))
> >   		goto fail_cid;
> >   

[snip]
> 
> > +
> > +static inline void __update_mm_sched(struct rq *rq, struct mm_sched *pcpu_sched)
> > +{
> > +	lockdep_assert_held(&rq->cpu_epoch_lock);
> > +
> > +	unsigned long n, now = jiffies;
> > +	long delta = now - rq->cpu_epoch_next;
> > +
> > +	if (delta > 0) {
> > +		n = (delta + EPOCH_PERIOD - 1) / EPOCH_PERIOD;
> > +		rq->cpu_epoch += n;
> > +		rq->cpu_epoch_next += n * EPOCH_PERIOD;
> > +		__shr_u64(&rq->cpu_runtime, n);
> 
> Another doubt i had, does this occupancy works when there is CPU bandwidth controller running?
> A 50% occupancy may have different meaning when CPU bandwidth is set to 50%?

The occupancy is used to compare tasks occupancy within a process.  With bandwidth
controller set to 50%, it just mean that all tasks in the process will run 50% less,
but the relative occupancy ratio between tasks should still remain the same.

[snip]
> 
> > +	}
> > 
> > +
> > +static void task_cache_work(struct callback_head *work)
> > +{
> > +	struct task_struct *p = current;
> > +	struct mm_struct *mm = p->mm;
> > +	unsigned long m_a_occ = 0;
> > +	int cpu, m_a_cpu = -1;
> > +	cpumask_var_t cpus;
> > +
> > +	WARN_ON_ONCE(work != &p->cache_work);
> > +
> > +	work->next = work;
> > +
> > +	if (p->flags & PF_EXITING)
> > +		return;
> > +
> > +	if (!alloc_cpumask_var(&cpus, GFP_KERNEL))
> > +		return;
> > +
> > +	scoped_guard (cpus_read_lock) {
> > +		cpumask_copy(cpus, cpu_online_mask);
> > +
> 
> As pointed out already, this is going to be costly in multi NUMA systems. Any cross NUMA access of
> CPUs data is going to add overhead to system bus bandwidth and this happening at tick could be costly.
> 

We'll consider restricting the scan on preferred NUMA node (if numa balancing is running), which
should greatly reduce the overhead. 

> Also, taking cpu_read_lock does preempt_disable, this could add to large preemptoff?
> We need to measure the time it takes on large system. Will try and get back with that number

Tim