On Sat, 2020-11-21 at 15:27 +0100, Hans de Goede wrote:
> 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?
Whether the file exists or doesn't, we have ways to monitor it
appearing or disappearing. I can monitor the directory with inotify to
see the file appearing or disappearing, or I can monitor the file with
inotify to see whether it changes. But that doesn't solve the
challenges from user-space.
How do I know whether the computer will have this facility at any
point? Is "none" a placeholder, or a definite answer as to whether the
computer will have support for "platform profile"?
How am I supposed to handle fallbacks, eg. how can I offer support for,
say, changing the "energy performance preference" from the Intel p-
state driver if ACPI platform profile isn't available?
I'm fine with throwing more code at fixing that race, so whatever's
easier for you, it'll be a pain for user-space either way.
