Re: Query about timer wheel API

[imran.f.khan@oracle.com]

Hello Hillf,
On 24/12/2024 9:41 pm, Hillf Danton wrote:
> On Tue, 24 Dec 2024 01:20:48 +1100 imran.f.khan@oracle.com
>>
>> static int param_set_queue_work_on_cpu(const char *val, const struct kernel_param *kp)
>> {
>> 	int cpu, this_cpu, i;
>> 	struct delayed_work *dwork = NULL;
>>
>> 	if (!mutex_trylock(&mutex))
>> 		return -EBUSY;
>>
>> 	cpu = simple_strtoul(val, NULL, 0);
>> 	/*if (!cpu_present(cpu)) 
>> 		return -EINVAL;*/
>>
>> 	for (i = 0; i < NUM_WORK_ITEMS; i++) {
>> 		dwork = kzalloc(sizeof(struct delayed_work), GFP_KERNEL);
>> 		if(dwork) {
>> 			this_cpu = get_cpu();
> 
> See if checking cpu works for you.
> 
> 			if (cpu >= nr_cpu_ids || !cpu_online(cpu)) {
>  				put_cpu();
> 				pr_err("%s cpu%d invalid\n", __func__, cpu);
> 				break;
> 			}
> 

the query was not about why its (not) working with my module. The test module, in its
current form, is just to show that a timer-wheel timer could be inserted in timer list
of an offlined CPU.

What you have suggested, we are already doing it in RDS code (mentioned in my earlier messages).
Also just using cpu_online may not be enough, unless we do it under get/put_online_cpus.

If you see, my query was more towards, what should "add_timer_on" do for such cases or
can we  have another function like try_add_timer_on that tries to put the timer on specific
CPU, but puts it else where if that CPU is offline. Is it worth having such an interface 
or should we stick to the current approach of fixing this on the caller side.

Thanks,
Imran

>> 			INIT_DELAYED_WORK(dwork, delayed_work_func);
>> 			queue_delayed_work_on(cpu, system_wq, dwork, msecs_to_jiffies(10000));
>> 			pr_err("Submitted dwork 0x%px on %s cpu#%d \n", dwork, cpu_online(cpu)?"online":"offline", cpu);
>> 			put_cpu();
>> 		}
>>
>> 	}
>> 	mutex_unlock(&mutex);
>> 	return 0;
>> }