| Message ID | 20241105153316.378-9-mario.limonciello@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for binding ACPI platform profile to multiple drivers | expand |
On Tue, 5 Nov 2024, Mario Limonciello wrote: > Migrate away from using an interruptible mutex to scoped_cond_guard > in all functions. > > Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/acpi/platform_profile.c | 114 ++++++++++++-------------------- > 1 file changed, 44 insertions(+), 70 deletions(-) > > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c > index 4454c4a903c8f..fd1c4e9dccf0a 100644 > --- a/drivers/acpi/platform_profile.c > +++ b/drivers/acpi/platform_profile.c > @@ -27,25 +27,21 @@ static ssize_t platform_profile_choices_show(struct device *dev, > char *buf) > { > int len = 0; > - int err, i; > - > - err = mutex_lock_interruptible(&profile_lock); > - if (err) > - return err; > - > - if (!cur_profile) { > - mutex_unlock(&profile_lock); > - return -ENODEV; > - } > - > - for_each_set_bit(i, cur_profile->choices, PLATFORM_PROFILE_LAST) { > - if (len == 0) > - len += sysfs_emit_at(buf, len, "%s", profile_names[i]); > - else > - len += sysfs_emit_at(buf, len, " %s", profile_names[i]); > + int i; > + > + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { > + if (!cur_profile) > + return -ENODEV; > + > + for_each_set_bit(i, cur_profile->choices, PLATFORM_PROFILE_LAST) { > + if (len == 0) > + len += sysfs_emit_at(buf, len, "%s", profile_names[i]); > + else > + len += sysfs_emit_at(buf, len, " %s", profile_names[i]); > + } > } > len += sysfs_emit_at(buf, len, "\n"); > - mutex_unlock(&profile_lock); > + > return len; > } > > @@ -56,20 +52,15 @@ static ssize_t platform_profile_show(struct device *dev, > enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED; > int err; > > - err = mutex_lock_interruptible(&profile_lock); > - if (err) > - return err; > + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { > + if (!cur_profile) > + return -ENODEV; > > - if (!cur_profile) { > - mutex_unlock(&profile_lock); > - return -ENODEV; > + err = cur_profile->profile_get(cur_profile, &profile); > + if (err) > + return err; > } > > - err = cur_profile->profile_get(cur_profile, &profile); > - mutex_unlock(&profile_lock); > - if (err) > - return err; > - > /* Check that profile is valid index */ > if (WARN_ON((profile < 0) || (profile >= ARRAY_SIZE(profile_names)))) > return -EIO; > @@ -88,28 +79,20 @@ static ssize_t platform_profile_store(struct device *dev, > if (i < 0) > return -EINVAL; > > - err = mutex_lock_interruptible(&profile_lock); > - if (err) > - return err; > + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { > + if (!cur_profile) > + return -ENODEV; > > - if (!cur_profile) { > - mutex_unlock(&profile_lock); > - return -ENODEV; > - } > + /* Check that platform supports this profile choice */ > + if (!test_bit(i, cur_profile->choices)) > + return -EOPNOTSUPP; > > - /* 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(cur_profile, i); > + if (err) > + return err; > } > > - err = cur_profile->profile_set(cur_profile, i); > - if (!err) > - sysfs_notify(acpi_kobj, NULL, "platform_profile"); > - > - mutex_unlock(&profile_lock); > - if (err) > - return err; > + sysfs_notify(acpi_kobj, NULL, "platform_profile"); Hi Mario, While I think it's fine, could you please mention about this move outside of the lock in the commit message. Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> -- i. > return count; > } > > @@ -140,36 +123,27 @@ int platform_profile_cycle(void) > enum platform_profile_option next; > int err; > > - err = mutex_lock_interruptible(&profile_lock); > - if (err) > - return err; > + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { > + if (!cur_profile) > + return -ENODEV; > > - if (!cur_profile) { > - mutex_unlock(&profile_lock); > - return -ENODEV; > - } > + err = cur_profile->profile_get(cur_profile, &profile); > + if (err) > + return err; > > - err = cur_profile->profile_get(cur_profile, &profile); > - if (err) { > - mutex_unlock(&profile_lock); > - return err; > - } > + next = find_next_bit_wrap(cur_profile->choices, PLATFORM_PROFILE_LAST, > + profile + 1); > > - next = find_next_bit_wrap(cur_profile->choices, PLATFORM_PROFILE_LAST, > - profile + 1); > + if (WARN_ON(next == PLATFORM_PROFILE_LAST)) > + return -EINVAL; > > - if (WARN_ON(next == PLATFORM_PROFILE_LAST)) { > - mutex_unlock(&profile_lock); > - return -EINVAL; > + err = cur_profile->profile_set(cur_profile, next); > + if (err) > + return err; > } > > - err = cur_profile->profile_set(cur_profile, next); > - mutex_unlock(&profile_lock); > - > - if (!err) > - sysfs_notify(acpi_kobj, NULL, "platform_profile"); > - > - return err; > + sysfs_notify(acpi_kobj, NULL, "platform_profile"); > + return 0; > } > EXPORT_SYMBOL_GPL(platform_profile_cycle); > >
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index 4454c4a903c8f..fd1c4e9dccf0a 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -27,25 +27,21 @@ static ssize_t platform_profile_choices_show(struct device *dev, char *buf) { int len = 0; - int err, i; - - err = mutex_lock_interruptible(&profile_lock); - if (err) - return err; - - if (!cur_profile) { - mutex_unlock(&profile_lock); - return -ENODEV; - } - - for_each_set_bit(i, cur_profile->choices, PLATFORM_PROFILE_LAST) { - if (len == 0) - len += sysfs_emit_at(buf, len, "%s", profile_names[i]); - else - len += sysfs_emit_at(buf, len, " %s", profile_names[i]); + int i; + + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { + if (!cur_profile) + return -ENODEV; + + for_each_set_bit(i, cur_profile->choices, PLATFORM_PROFILE_LAST) { + if (len == 0) + len += sysfs_emit_at(buf, len, "%s", profile_names[i]); + else + len += sysfs_emit_at(buf, len, " %s", profile_names[i]); + } } len += sysfs_emit_at(buf, len, "\n"); - mutex_unlock(&profile_lock); + return len; } @@ -56,20 +52,15 @@ static ssize_t platform_profile_show(struct device *dev, enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED; int err; - err = mutex_lock_interruptible(&profile_lock); - if (err) - return err; + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { + if (!cur_profile) + return -ENODEV; - if (!cur_profile) { - mutex_unlock(&profile_lock); - return -ENODEV; + err = cur_profile->profile_get(cur_profile, &profile); + if (err) + return err; } - err = cur_profile->profile_get(cur_profile, &profile); - mutex_unlock(&profile_lock); - if (err) - return err; - /* Check that profile is valid index */ if (WARN_ON((profile < 0) || (profile >= ARRAY_SIZE(profile_names)))) return -EIO; @@ -88,28 +79,20 @@ static ssize_t platform_profile_store(struct device *dev, if (i < 0) return -EINVAL; - err = mutex_lock_interruptible(&profile_lock); - if (err) - return err; + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { + if (!cur_profile) + return -ENODEV; - if (!cur_profile) { - mutex_unlock(&profile_lock); - return -ENODEV; - } + /* Check that platform supports this profile choice */ + if (!test_bit(i, cur_profile->choices)) + return -EOPNOTSUPP; - /* 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(cur_profile, i); + if (err) + return err; } - err = cur_profile->profile_set(cur_profile, i); - if (!err) - sysfs_notify(acpi_kobj, NULL, "platform_profile"); - - mutex_unlock(&profile_lock); - if (err) - return err; + sysfs_notify(acpi_kobj, NULL, "platform_profile"); return count; } @@ -140,36 +123,27 @@ int platform_profile_cycle(void) enum platform_profile_option next; int err; - err = mutex_lock_interruptible(&profile_lock); - if (err) - return err; + scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) { + if (!cur_profile) + return -ENODEV; - if (!cur_profile) { - mutex_unlock(&profile_lock); - return -ENODEV; - } + err = cur_profile->profile_get(cur_profile, &profile); + if (err) + return err; - err = cur_profile->profile_get(cur_profile, &profile); - if (err) { - mutex_unlock(&profile_lock); - return err; - } + next = find_next_bit_wrap(cur_profile->choices, PLATFORM_PROFILE_LAST, + profile + 1); - next = find_next_bit_wrap(cur_profile->choices, PLATFORM_PROFILE_LAST, - profile + 1); + if (WARN_ON(next == PLATFORM_PROFILE_LAST)) + return -EINVAL; - if (WARN_ON(next == PLATFORM_PROFILE_LAST)) { - mutex_unlock(&profile_lock); - return -EINVAL; + err = cur_profile->profile_set(cur_profile, next); + if (err) + return err; } - err = cur_profile->profile_set(cur_profile, next); - mutex_unlock(&profile_lock); - - if (!err) - sysfs_notify(acpi_kobj, NULL, "platform_profile"); - - return err; + sysfs_notify(acpi_kobj, NULL, "platform_profile"); + return 0; } EXPORT_SYMBOL_GPL(platform_profile_cycle);