Re: [PATCH v2 4/4] sched/numa: do not let VMA PID activity gate promotion

[David Hildenbrand (Arm)]

On 9/11/26 02:18, Gregory Price wrote:
> From: "Gregory Price (Meta)" <gourry@xxxxxxxxxx>
> 
> Commit fc137c0ddab2 ("sched/numa: enhance vma scanning logic")
> skips VMAs without recent PID activity. Since only NUMA hint faults record
> that activity, the filter can suppress the fault needed to promote hot
> slow-tier memory.
> 
> In tiering mode, scan PID-inactive VMAs using promotion-only scans.
> Reevaluate this choice whenever the scanner visits a VMA and pass it with
> each protection walk.
> 
> Record socket-placement scans in prev_placement_scan_seq. Promotion-only
> scans still update prev_scan_seq, but no longer postpone the starvation
> fallback for placement scans.
> 
> On a host with 768 GB of DRAM and 256 GB of CXL memory running two roughly
> 430 GB database workloads, a large shmem VMA occupied each scan while 2,537
> other VMAs covering 84 GB were skipped as inactive. A hot 20 GB hash table
> remained entirely on CXL before this change and was split evenly between
> DRAM and CXL afterwards.
> 
> Fixes: fc137c0ddab2 ("sched/numa: enhance vma scanning logic")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>
> ---
>  include/linux/mm_types.h |  7 ++++++
>  kernel/sched/fair.c      | 48 ++++++++++++++++++++++++++--------------
>  2 files changed, 39 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5413bd10fff2..9f042d6ad465 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -803,6 +803,13 @@ struct vma_numab_state {
>  	 * A VMA is not eligible for scanning if prev_scan_seq == numa_scan_seq
>  	 */
>  	int prev_scan_seq;
> +
> +	/*
> +	 * MM scan sequence ID when the VMA was last scanned for placement.
> +	 * The starvation horizon in vma_is_accessed() counts against this, so
> +	 * promotion-only scans cannot postpone placement indefinitely.
> +	 */
> +	int prev_placement_scan_seq;
>  };
>  
>  #ifdef __HAVE_PFNMAP_TRACKING
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e636e8de53f1..6d1da13a2ef5 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4085,6 +4085,10 @@ static void reset_ptenuma_scan(struct task_struct *p)
>  	p->mm->numa_scan_offset = 0;
>  }
>  
> +/*
> + * Decide whether this VMA should be sampled for NUMA placement.  In addition
> + * to recent accesses, periodically allow a scan to avoid starvation.
> + */
>  static bool vma_is_accessed(struct mm_struct *mm, struct vm_area_struct *vma)
>  {
>  	unsigned long pids;
> @@ -4101,22 +4105,13 @@ static bool vma_is_accessed(struct mm_struct *mm, struct vm_area_struct *vma)
>  	if (test_bit(hash_32(current->pid, ilog2(BITS_PER_LONG)), &pids))
>  		return true;
>  
> -	/*
> -	 * Complete a scan that has already started regardless of PID access, or
> -	 * some VMAs may never be scanned in multi-threaded applications:
> -	 */
> -	if (mm->numa_scan_offset > vma->vm_start) {
> -		trace_sched_skip_vma_numa(mm, vma, NUMAB_SKIP_IGNORE_PID);
> -		return true;
> -	}
> -
>  	/*
>  	 * This vma has not been accessed for a while, and if the number
>  	 * the threads in the same process is low, which means no other
>  	 * threads can help scan this vma, force a vma scan.
>  	 */
>  	if (READ_ONCE(mm->numa_scan_seq) >
> -	   (vma->numab_state->prev_scan_seq + get_nr_threads(current)))
> +	   (vma->numab_state->prev_placement_scan_seq + get_nr_threads(current)))
>  		return true;
>  
>  	return false;
> @@ -4142,7 +4137,7 @@ static void task_numa_work(struct callback_head *work)
>  	unsigned int numab_mode = READ_ONCE(sysctl_numa_balancing_mode);
>  	bool vma_pids_skipped;
>  	bool vma_pids_forced = false;
> -	bool promo_only;
> +	bool accessed, scan_started, promo_only;
>  
>  	WARN_ON_ONCE(p != container_of(work, struct task_struct, numa_work));
>  
> @@ -4278,6 +4273,7 @@ static void task_numa_work(struct callback_head *work)
>  			 * first scan:
>  			 */
>  			 vma->numab_state->prev_scan_seq = mm->numa_scan_seq - 1;
> +			 vma->numab_state->prev_placement_scan_seq = mm->numa_scan_seq - 1;
>  		}
>  
>  		/*
> @@ -4307,17 +4303,30 @@ static void task_numa_work(struct callback_head *work)
>  		}
>  
>  		/*
> -		 * Do not scan the VMA if task has not accessed it, unless no other
> -		 * VMA candidate exists.
> +		 * The PID filter must not gate promotion.  Scan PID-inactive
> +		 * VMAs in tiering mode using promotion-only scans.
>  		 */
> -		if (!vma_pids_forced && !vma_is_accessed(mm, vma)) {
> +		accessed = vma_is_accessed(mm, vma);
> +		scan_started = mm->numa_scan_offset > vma->vm_start;
> +
> +		if (!vma_pids_forced && !accessed && !scan_started &&
> +		    !(numab_mode & NUMA_BALANCING_MEMORY_TIERING)) {
>  			vma_pids_skipped = true;
>  			trace_sched_skip_vma_numa(mm, vma, NUMAB_SKIP_PID_INACTIVE);
>  			continue;
>  		}
> +		if (!vma_pids_forced && !accessed &&
> +		    !(numab_mode & NUMA_BALANCING_MEMORY_TIERING) &&
> +		    scan_started)
> +			trace_sched_skip_vma_numa(mm, vma, NUMAB_SKIP_IGNORE_PID);
>  
> +		/*
> +		 * In combined mode, only VMAs the task uses need placement
> +		 * samples. Without tiering, every VMA reaching here does.
> +		 */
>  		promo_only = !(numab_mode & NUMA_BALANCING_NORMAL) ||
> -			     vma_is_ro_file(vma);
> +			     ((numab_mode & NUMA_BALANCING_MEMORY_TIERING) &&
> +			      !accessed) || vma_is_ro_file(vma);
>  
>  		do {
>  			start = max(start, vma->vm_start);
> @@ -4345,8 +4354,15 @@ static void task_numa_work(struct callback_head *work)
>  			cond_resched();
>  		} while (end != vma->vm_end);
>  
> -		/* VMA scan is complete, do not scan until next sequence. */
> +		/*
> +		 * VMA scan is complete, do not scan until next sequence.  A
> +		 * promotion-only scan reached the end of the VMA but did not
> +		 * sample placement, so it does not count towards the starvation
> +		 * horizon in vma_is_accessed().
> +		 */
>  		vma->numab_state->prev_scan_seq = mm->numa_scan_seq;
> +		if (!promo_only)
> +			vma->numab_state->prev_placement_scan_seq = mm->numa_scan_seq;


This is all rather ... messy. I mean, just look at that promo_only() computation ...

There must be a cleaner way ;)
-- 
Cheers,

David