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

[Leonardo Bras]

On Thu, Feb 26, 2026 at 12:49:18PM -0300, Marcelo Tosatti wrote:
> 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.


Yes, but really rarely, and contention even more rare.
It also should not break anything, as locking will make sure access to that 
per-cpu value is being serialized.

I was wondering on the extra cost of migrate_* on the hot path being an 
undesired effect for other code evaluating switching to QPW. But maybe it's 
not that bad, and we get rid of that very rare contention.

Thanks!
Leo

> 
> > - 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
> > 
> > 
>