Re: [PATCH 3/4] swap: apply new queue_percpu_work_on() interface

[Marcelo Tosatti <mtosatti@redhat.com>]

On Fri, Feb 06, 2026 at 10:06:28PM -0300, Leonardo Bras wrote:
> > +	cpu = smp_processor_id();
> 
> Wondering if for these cases it would make sense to have something like:
> 
> qpw_get_local_cpu() and 
> qpw_put_local_cpu() 
> 
> so we could encapsulate these migrate_{en,dis}able()
> and the smp_processor_id().
> 
> Or even,
> 
> int qpw_local_lock() {
> 	migrate_disable();
> 	cpu = smp_processor_id();
> 	qpw_lock(..., cpu);
> 
> 	return cpu;
> }
> 
> and
> 
> qpw_local_unlock(cpu){
> 	qpw_unlock(...,cpu);
> 	migrate_enable();
> } 
> 
> so it's more direct to convert the local-only cases.
> 
> What do you think?

Switched to local_qpw_lock variants.

> >  {
> > -	local_lock(&cpu_fbatches.lock);
> > -	lru_add_drain_cpu(smp_processor_id());
> 
> and here ?

Fixed lack of migrate_disable/migrate_enable, thanks!

> > @@ -950,7 +954,7 @@ void lru_cache_disable(void)
> >  #ifdef CONFIG_SMP
> >  	__lru_add_drain_all(true);
> >  #else
> > -	lru_add_mm_drain();
> 
> and here, I wonder

This is !CONFIG_SMP, so smp_processor_id is always 0.

> >  	drain_pages(cpu);
> >  
> >  	/*
> > 
> > 
> 
> TBH, I am still trying to understand if we need the migrate_{en,dis}able():
> - There is a data dependency beween cpu being filled and being used.
> - If we get the cpu, and then migrate to a different cpu, the operation 
>   will still be executed with the data from that starting cpu 

Yes, but on a remote CPU. What prevents the original CPU from accessing 
its per-CPU local data, therefore racing with the code executing on the
remote CPU.

> - But maybe the compiler tries to optize this because the processor number 
>   can be on a register and of easy access, which would break this.
> 
> Maybe a READ_ONCE() on smp_processor_id() should suffice?
> 
> Other than that, all the conversions done look correct.
> 
> That being said, I understand very little about mm code, so let's hope we 
> get proper feedback from those who do :) 
> 
> Thanks!
> Leo
> 
>