Re: [PATCH v3] ACPI: platform-profile: Add platform profile support

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 11/20/20 8:50 PM, Barnabás Pőcze wrote:
> Hi
> 
> 
> 2020. november 15., vasárnap 1:44 keltezéssel, Mark Pearson írta:
> 
>> [...]
>> +int platform_profile_register(struct platform_profile_handler *pprof)
>> +{
>> +	mutex_lock(&profile_lock);
>> +	/* We can only have one active profile */
>> +	if (cur_profile) {
>> +		mutex_unlock(&profile_lock);
>> +		return -EEXIST;
>> +	}
>> +
>> +	cur_profile = pprof;
>> +	mutex_unlock(&profile_lock);
>> +	return sysfs_create_group(acpi_kobj, &platform_profile_group);
>> +}
>> +EXPORT_SYMBOL_GPL(platform_profile_register);
>> +
>> +int platform_profile_unregister(void)
>> +{
>> +	mutex_lock(&profile_lock);
>> +	sysfs_remove_group(acpi_kobj, &platform_profile_group);
>> +	cur_profile = NULL;
>> +	mutex_unlock(&profile_lock);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(platform_profile_unregister);
>> [...]
> 
> 
> I just realized that the sysfs attributes are only created if a profile provider
> is registered, and it is removed when the provide unregisters itself. I believe
> it would be easier for system daemons if those attributes existed from module load
> to module unload since they can just just open the file and watch it using poll,
> select, etc. If it goes away when the provider unregisters itself, then I believe
> a more complicated mechanism (like inotify) would need to be implemented in the
> daemons to be notified when a new provider is registered. Thus my suggestion
> for the next iteration is to create the sysfs attributes on module load,
> and delete them on unload.
> 
> What do you think?

Actually I asked Mark to move this to the register / unregister time since
having a non functioning files in sysfs is a bit weird.

You make a good point about userspace having trouble figuring when this will
show up though (I'm not worried about removal that will normally never happen).

I would expect all modules to be loaded before any interested daemons load,
but that is an assumption and I must admit that that is a bit racy.

Bastien, what do you think about Barnabás' suggestion to always have the
files present and use poll (POLL_PRI) on it to see when it changes, listing
maybe "none" as available profiles when there is no provider?

Regards,

Hans