Re: [PATCH v2 07/18] mm/ksw: add atomic watch on/off operations

[Jinchao Wang <wangjinchao600@gmail.com>]

On Thu, Sep 04, 2025 at 08:46:59AM +0200, Peter Zijlstra wrote:
> On Thu, Sep 04, 2025 at 08:21:04AM +0800, Jinchao Wang wrote:
> 
> > +static DEFINE_PER_CPU(call_single_data_t,
> > +		      watch_csd) = CSD_INIT(ksw_watch_on_local_cpu,
> > +					    &watch_info);
> > +
> 
> > +int ksw_watch_on(u64 watch_addr, u64 watch_len)
> > +{
> > +	unsigned long flags;
> > +	int cpu;
> > +
> > +	if (!watch_addr) {
> > +		pr_err("KSW: watch with invalid address\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	spin_lock_irqsave(&watch_lock, flags);
> > +
> > +	/*
> > +	 * check if already watched
> > +	 */
> > +	if (watch_info.addr != 0 && // not uninit
> > +	    watch_info.addr != (unsigned long)&watch_holder && // installed
> > +	    watch_addr != (unsigned long)&watch_holder) { //not restore
> > +		spin_unlock_irqrestore(&watch_lock, flags);
> > +		return -EBUSY;
> > +	}
> > +
> > +	watch_info.addr = watch_addr;
> > +	watch_info.len = watch_len;
> > +
> > +	spin_unlock_irqrestore(&watch_lock, flags);
> > +
> > +	if (watch_addr == (unsigned long)&watch_holder)
> > +		pr_debug("KSW: watch off starting\n");
> > +	else
> > +		pr_debug("KSW: watch on starting\n");
> > +
> > +	for_each_online_cpu(cpu) {
> > +		if (cpu == raw_smp_processor_id()) {
> > +			ksw_watch_on_local_cpu(&watch_info);
> > +		} else {
> > +			call_single_data_t *csd = &per_cpu(watch_csd, cpu);
> > +
> > +			smp_call_function_single_async(cpu, csd);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> 
> What do you think happens when two ksw_watch_on() instances run
> concurrently?
> 
> What happens if a CPU comes online/offline concurrently with
> ksw_watch_on()?
Thanks for pointing that out. I did not think much about it.
I will think more and address it in the next series.