Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9693b4d6 authored by Jonathan Avila's avatar Jonathan Avila Committed by Amir Vajid
Browse files

PM / devfreq: Introduce an event lock 



Currently, concurrent writes to sysfs entries leave the	possibility
for race conditions within the devfreq framework.  For example,
concurrently executing max_freq_store and governor_store can result
in attempting to perform an update_devfreq() before the new governor's
start handler can be executed.

A more concrete case is a race between polling_interval_store and
governor_store.  Because no lock is used after calling into the event
handler of the old governor and there's nothing preventing work from
being queued after the monitor is stopped, it's possible to
accidentally cause delayed work to be queued on the governor being
switched to.  This can be seen if you create two threads, one which
changes a device's governor between simple_ondemand and performance,
and one which changes its polling interval between 45 and 50.

All of these races can be addressed with the introduction of a lock
that prevents sysfs operations from interleaving in this fashion.

Change-Id: Ia6887dcb2d69dc2576837a6c09fed55a28943abc
Signed-off-by: default avatarJonathan Avila <avilaj@codeaurora.org>
[avajid@codeaurora.org: renamed to event lock and only used when CONFIG_QCOM_DEVFREQ_ICC is enabled]
Signed-off-by: default avatarAmir Vajid <avajid@codeaurora.org>
parent 52ab3535

drivers/devfreq/devfreq.c

+13 −1
Original line number Diff line number Diff line
@@ -595,6 +595,7 @@ static void devfreq_dev_release(struct device *dev) 
		devfreq->profile->exit(devfreq->dev.parent);



	mutex_destroy(&devfreq->lock);

	event_mutex_destroy(devfreq);

	kfree(devfreq);

}
@@ -637,6 +638,7 @@ struct devfreq *devfreq_add_device(struct device *dev, 
	}



	mutex_init(&devfreq->lock);

	event_mutex_init(devfreq);

	mutex_lock(&devfreq->lock);

	devfreq->dev.parent = dev;

	devfreq->dev.class = devfreq_class;
@@ -1145,12 +1147,13 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, 
		goto out;

	}



	event_mutex_lock(df);

	if (df->governor) {

		ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);

		if (ret) {

			dev_warn(dev, "%s: Governor %s not stopped(%d)\n",

				 __func__, df->governor->name, ret);

			goto out;

			goto gov_stop_out;

		}

	}

	prev_governor = df->governor;
@@ -1171,6 +1174,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, 
			df->governor = NULL;

		}

	}



gov_stop_out:

	event_mutex_unlock(df);

out:

	mutex_unlock(&devfreq_list_lock);
@@ -1265,8 +1271,10 @@ static ssize_t polling_interval_store(struct device *dev, 
	if (ret != 1)

		return -EINVAL;



	event_mutex_lock(df);

	df->governor->event_handler(df, DEVFREQ_GOV_INTERVAL, &value);

	ret = count;

	event_mutex_unlock(df);



	return ret;

}
@@ -1283,6 +1291,7 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr, 
	if (ret != 1)

		return -EINVAL;



	event_mutex_lock(df);

	mutex_lock(&df->lock);



	if (value) {
@@ -1305,6 +1314,7 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr, 
	ret = count;

unlock:

	mutex_unlock(&df->lock);

	event_mutex_unlock(df);

	return ret;

}
@@ -1327,6 +1337,7 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, 
	if (ret != 1)

		return -EINVAL;



	event_mutex_lock(df);

	mutex_lock(&df->lock);



	if (value) {
@@ -1349,6 +1360,7 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr, 
	ret = count;

unlock:

	mutex_unlock(&df->lock);

	event_mutex_unlock(df);

	return ret;

}

static DEVICE_ATTR_RW(min_freq);

include/linux/devfreq.h

+28 −0
Original line number Diff line number Diff line
@@ -149,6 +149,9 @@ struct devfreq { 
	struct list_head node;



	struct mutex lock;

#ifdef CONFIG_QCOM_DEVFREQ_ICC

	struct mutex event_lock;

#endif

	struct device dev;

	struct devfreq_dev_profile *profile;

	const struct devfreq_governor *governor;
@@ -185,6 +188,31 @@ struct devfreq_freqs { 
	unsigned long new;

};



static inline void event_mutex_init(struct devfreq *devfreq)

{

#ifdef CONFIG_QCOM_DEVFREQ_ICC

	mutex_init(&devfreq->event_lock);

#endif

}

static inline void event_mutex_destroy(struct devfreq *devfreq)

{

#ifdef CONFIG_QCOM_DEVFREQ_ICC

	mutex_destroy(&devfreq->event_lock);

#endif

}

static inline void event_mutex_lock(struct devfreq *devfreq)

{

#ifdef CONFIG_QCOM_DEVFREQ_ICC

	mutex_lock(&devfreq->event_lock);

#endif

}

static inline void event_mutex_unlock(struct devfreq *devfreq)

{

#ifdef CONFIG_QCOM_DEVFREQ_ICC

	mutex_unlock(&devfreq->event_lock);

#endif

}



#if defined(CONFIG_PM_DEVFREQ)

extern struct devfreq *devfreq_add_device(struct device *dev,

				  struct devfreq_dev_profile *profile,