Re: [PATCH v5 2/3] ACPI: platform-profile: Add platform profile support

["Rafael J. Wysocki" <rafael@kernel.org>]

On Tue, Dec 8, 2020 at 7:26 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Dec 2, 2020 at 6:16 PM Mark Pearson <markpearson@lenovo.com> wrote:

[cut]

> > +
> > +static ssize_t platform_profile_store(struct device *dev,
> > +                           struct device_attribute *attr,
> > +                           const char *buf, size_t count)
> > +{
> > +       int err, i;
> > +
> > +       err = mutex_lock_interruptible(&profile_lock);
> > +       if (err)
> > +               return err;
> > +
> > +       if (!cur_profile) {
> > +               mutex_unlock(&profile_lock);
> > +               return -ENODEV;
> > +       }
> > +
> > +       /* Scan for a matching profile */
> > +       i = sysfs_match_string(profile_names, buf);
> > +       if (i < 0) {
> > +               mutex_unlock(&profile_lock);
> > +               return -EINVAL;
> > +       }
> > +
> > +       /* Check that platform supports this profile choice */
> > +       if (!test_bit(i, cur_profile->choices)) {
> > +               mutex_unlock(&profile_lock);
> > +               return -EOPNOTSUPP;
> > +       }
> > +
> > +       err = cur_profile->profile_set(i);
>
> What if this gets a signal in the middle of the ->profile_set()
> execution?  Is this always guaranteed to work?

I got this backwards, sorry.

The "interruptible" variant is used to allow the waiters to be
interrupted, so I guess the concern is that ->profile_set() may get
stuck or just take too much time?

> > +       mutex_unlock(&profile_lock);
> > +       if (err)
> > +               return err;
> > +       return count;
> > +}
> > +