Re: [PATCH 3/3] thermal: intel: hfi: Enable interface only when required

[Jiri Pirko <jiri@resnulli.us>]

Fri, Feb 02, 2024 at 02:00:46PM CET, stanislaw.gruszka@linux.intel.com wrote:
>On Fri, Feb 02, 2024 at 01:36:09PM +0100, Jiri Pirko wrote:
>> Wed, Jan 31, 2024 at 01:05:35PM CET, stanislaw.gruszka@linux.intel.com wrote:
>> 
>> [...]
>> 
>> 
>> >+static int hfi_netlink_notify(struct notifier_block *nb, unsigned long state,
>> >+			      void *_notify)
>> >+{
>> >+	struct netlink_notify *notify = _notify;
>> >+	struct hfi_instance *hfi_instance;
>> >+	smp_call_func_t func;
>> >+	unsigned int cpu;
>> >+	int i;
>> >+
>> >+	if (notify->protocol != NETLINK_GENERIC)
>> >+		return NOTIFY_DONE;
>> >+
>> >+	switch (state) {
>> >+	case NETLINK_CHANGE:
>> >+	case NETLINK_URELEASE:
>> >+		mutex_lock(&hfi_instance_lock);
>> >+
>> 
>> What's stopping other thread from mangling the listeners here?
>
>Nothing. But if the listeners will be changed, we will get next notify.
>Serialization by the mutex is needed to assure that the last setting will win,
>so we do not end with HFI disabled when there are listeners or vice versa.

Okay. Care to put a note somewhere?

>
>> >+		if (thermal_group_has_listeners(THERMAL_GENL_EVENT_GROUP))
>> >+			func = hfi_do_enable;
>> >+		else
>> >+			func = hfi_do_disable;
>> >+
>> >+		for (i = 0; i < max_hfi_instances; i++) {
>> >+			hfi_instance = &hfi_instances[i];
>> >+			if (cpumask_empty(hfi_instance->cpus))
>> >+				continue;
>> >+
>> >+			cpu = cpumask_any(hfi_instance->cpus);
>> >+			smp_call_function_single(cpu, func, hfi_instance, true);
>> >+		}
>> >+
>> >+		mutex_unlock(&hfi_instance_lock);
>> >+		return NOTIFY_OK;
>> >+	}
>> >+
>> >+	return NOTIFY_DONE;
>> >+}
>> 
>> [...]
>