linux-kernel - Re: Query about timer wheel API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <910a73af-a221-4e5b-ad5b-a2a1bdb8c8c3@oracle.com>
Date: Tue, 24 Dec 2024 23:41:48 +1100
From: imran.f.khan@...cle.com
To: Hillf Danton <hdanton@...a.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Tejun Heo <tj@...nel.org>,
        john.stultz@...aro.org, sboyd@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: Query about timer wheel API

Hello Hillf,
On 24/12/2024 9:41 pm, Hillf Danton wrote:
> On Tue, 24 Dec 2024 01:20:48 +1100 imran.f.khan@...cle.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;
>> }