From fe41515b7ce7c7e5f572f29eb40046a1c57ce3b1 Mon Sep 17 00:00:00 2001 From: Rama Aparna Mallavarapu Date: Wed, 10 Oct 2018 14:53:32 -0700 Subject: [PATCH] PM / devfreq: Fix race condition between suspend/resume and governor_store There is a race condition when the event governor_store is being executed from sysfs and the device issues a suspend. The devfreq data structures would become stale when the suspend tries to access them in the middle of the governor_store operation. Fix this issue by taking a lock around suspend and resume operations so that these operations are not concurrent with the other events from sysfs. Also rename the sysfs_lock as event_lock since the same lock is used for non sysfs operations like suspend and resume as well. Change-Id: Ifa0e93915a920cec3e0429966328a1128d61098b Signed-off-by: Rama Aparna Mallavarapu --- drivers/devfreq/devfreq.c | 34 ++++++++++++++++++++++------------ include/linux/devfreq.h | 2 +- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index 5dd0acaa799f..ffcea42fd734 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -516,7 +516,7 @@ static void _remove_devfreq(struct devfreq *devfreq) devfreq->profile->exit(devfreq->dev.parent); mutex_destroy(&devfreq->lock); - mutex_destroy(&devfreq->sysfs_lock); + mutex_destroy(&devfreq->event_lock); kfree(devfreq); } @@ -573,7 +573,7 @@ struct devfreq *devfreq_add_device(struct device *dev, } mutex_init(&devfreq->lock); - mutex_init(&devfreq->sysfs_lock); + mutex_init(&devfreq->event_lock); mutex_lock(&devfreq->lock); devfreq->dev.parent = dev; devfreq->dev.class = devfreq_class; @@ -784,14 +784,19 @@ EXPORT_SYMBOL(devm_devfreq_remove_device); */ int devfreq_suspend_device(struct devfreq *devfreq) { + int ret; + if (!devfreq) return -EINVAL; if (!devfreq->governor) return 0; - return devfreq->governor->event_handler(devfreq, + mutex_lock(&devfreq->event_lock); + ret = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_SUSPEND, NULL); + mutex_unlock(&devfreq->event_lock); + return ret; } EXPORT_SYMBOL(devfreq_suspend_device); @@ -805,14 +810,19 @@ EXPORT_SYMBOL(devfreq_suspend_device); */ int devfreq_resume_device(struct devfreq *devfreq) { + int ret; + if (!devfreq) return -EINVAL; if (!devfreq->governor) return 0; - return devfreq->governor->event_handler(devfreq, + mutex_lock(&devfreq->event_lock); + ret = devfreq->governor->event_handler(devfreq, DEVFREQ_GOV_RESUME, NULL); + mutex_unlock(&devfreq->event_lock); + return ret; } EXPORT_SYMBOL(devfreq_resume_device); @@ -972,7 +982,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, goto out; } - mutex_lock(&df->sysfs_lock); + mutex_lock(&df->event_lock); if (df->governor) { ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL); if (ret) { @@ -998,7 +1008,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, } gov_stop_out: - mutex_unlock(&df->sysfs_lock); + mutex_unlock(&df->event_lock); out: mutex_unlock(&devfreq_list_lock); @@ -1093,10 +1103,10 @@ static ssize_t polling_interval_store(struct device *dev, if (ret != 1) return -EINVAL; - mutex_lock(&df->sysfs_lock); + mutex_lock(&df->event_lock); df->governor->event_handler(df, DEVFREQ_GOV_INTERVAL, &value); ret = count; - mutex_unlock(&df->sysfs_lock); + mutex_unlock(&df->event_lock); return ret; } @@ -1114,7 +1124,7 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr, if (ret != 1) return -EINVAL; - mutex_lock(&df->sysfs_lock); + mutex_lock(&df->event_lock); mutex_lock(&df->lock); max = df->max_freq; if (value && max && value > max) { @@ -1127,7 +1137,7 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr, ret = count; unlock: mutex_unlock(&df->lock); - mutex_unlock(&df->sysfs_lock); + mutex_unlock(&df->event_lock); return ret; } @@ -1143,7 +1153,7 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, if (ret != 1) return -EINVAL; - mutex_lock(&df->sysfs_lock); + mutex_lock(&df->event_lock); mutex_lock(&df->lock); min = df->min_freq; if (value && min && value < min) { @@ -1156,7 +1166,7 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, ret = count; unlock: mutex_unlock(&df->lock); - mutex_unlock(&df->sysfs_lock); + mutex_unlock(&df->event_lock); return ret; } diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h index eb84ce6b9a48..85d62224fe46 100644 --- a/include/linux/devfreq.h +++ b/include/linux/devfreq.h @@ -167,7 +167,7 @@ struct devfreq { struct list_head node; struct mutex lock; - struct mutex sysfs_lock; + struct mutex event_lock; struct device dev; struct devfreq_dev_profile *profile; const struct devfreq_governor *governor;