drivers/hwmon/occ: Add reference count and switch to non-blocking read
PATCH v2 1/1 drivers/hwmon/occ: Add reference count and switch to non-blocking read
From: Eddie James <[email protected]>
It's possible for the sbefifo ops to hang entirely if the system goes down (checkstop, etc). If a hwmon sysfs operation is hung, then unbind may hang as it waits for the kernfs mutex to remove hwmon attributes.
This change switches the p9 occ op to use a non-blocking read with a retry on -EAGAIN. This continues forever until either user cancels or driver sets the cancel bit in remove(). In addition, we need to switch our occ structure to be dynamically allocated separate from the device, as the device resources will be freed immediately after remove(), while hwmon ops may still be ocurring. This necessitates reference counting so we can free the occ structure once all users are done.
Signed-off-by Eddie James <[email protected]>
--- drivers/hwmon/occ/common.c | 48 +++++++++++++++++++++++++++++++++++++--------- drivers/hwmon/occ/common.h | 2 ++ drivers/hwmon/occ/p8_i2c.c | 23 +++++++++++++++++++--- drivers/hwmon/occ/p9_sbe.c | 37 ++++++++++++++++++++++++++++++----- 4 files changed, 93 insertions(+), 17 deletions(-)
drivers/hwmon/occ/common.c +39 -9
@@ -118,6 +118,12 @@ struct extended_sensor {u8 data[6 ]; } __packed; +/* not available in 4.10 */ +static bool kobject_get_unless_zero (struct kobject *kobj) +{ +return kref_get_unless_zero (&kobj->kref); +} +void occ_parse_poll_response (struct occ *occ) {unsigned int i, offset =0 , size =0 ;
@@ -216,7 +222,8 @@ int occ_poll(struct occ *occ) done:/* notify userspace if we change error state and have an error */ -if (occ->error != error && occ->error && occ->error_attr_name) +if (occ->error != error && occ->error && occ->error_attr_name && + !occ->cancel)sysfs_notify (&occ->bus_dev->kobj,NULL , occ->error_attr_name);return rc;
@@ -224,19 +231,28 @@ int occ_poll(struct occ *occ)int occ_set_user_power_cap (struct occ *occ,u16 user_power_cap) { -int rc, error = occ->error; +int rc, error;u8 cmd[8 ];u16 checksum =0x24 ;__be16 user_power_cap_be; -struct occ_poll_response_header *header = - (struct occ_poll_response_header *)occ->resp.data; +struct occ_poll_response_header *header; -if (!(header->status &OCC_STAT_MASTER )) -return -EPERM ; +if (!kobject_get_unless_zero (&occ->kobj)) +return -ENODEV ; -if (!(header->status &OCC_STAT_ACTIVE )) -return -EACCES ; + header = (struct occ_poll_response_header *)occ->resp.data; +if (!(header->status &OCC_STAT_MASTER )) { + rc = -EPERM ; + goto done; + } + +if (!(header->status &OCC_STAT_ACTIVE )) { + rc = -EACCES ; + goto done; + } + + error = occ->error; user_power_cap_be =cpu_to_be16 (user_power_cap); cmd[0 ] =0 ;
@@ -255,9 +271,12 @@ int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap)mutex_unlock (&occ->lock);/* notify userspace if we change error state and have an error*/ -if (occ->error != error && occ->error && occ->error_attr_name) +if (occ->error != error && occ->error && occ->error_attr_name && + !occ->cancel)sysfs_notify (&occ->bus_dev->kobj,NULL , occ->error_attr_name); +done: +kobject_put (&occ->kobj);return rc; }
@@ -265,6 +284,9 @@ int occ_update_response(struct occ *occ) {int rc =0 ; +if (!kobject_get_unless_zero (&occ->kobj)) +return -ENODEV ; +mutex_lock (&occ->lock);if (time_after (jiffies, occ->last_update +OCC_UPDATE_FREQUENCY )) {
@@ -272,7 +294,15 @@ int occ_update_response(struct occ *occ) occ->last_update = jiffies; } +/* +* Make sure we fail if canceled so we don't access occ structure in +* subsequent functions. +*/ +if (occ->cancel) + rc = -ECANCELED ; +mutex_unlock (&occ->lock); +kobject_put (&occ->kobj);return rc; }
drivers/hwmon/occ/common.h +2 -0
@@ -98,9 +98,11 @@ struct occ_attribute { };struct occ { +struct kobject kobj;struct device *bus_dev;struct device *hwmon; +bool cancel;int error;unsigned int error_count;unsigned long last_safe;
drivers/hwmon/occ/p8_i2c.c +20 -3
@@ -13,6 +13,7 @@#include <linux/i2c.h> #include <linux/init.h> #include <linux/module.h> +#include <linux/slab.h> struct p8_i2c_occ {struct occ occ;
@@ -21,6 +22,18 @@ struct p8_i2c_occ {#define to_p8_i2c_occ (x) container_of((x), struct p8_i2c_occ, occ) +static void p8_i2c_occ_release (struct kobject *kobj) +{ + struct occ *occ = container_of(kobj, struct occ, kobj); +struct p8_i2c_occ *p8_i2c_occ =to_p8_i2c_occ (occ); + +kfree (p8_i2c_occ); +} + +static struct kobj_type p8_i2c_occ_type = { + .release = p8_i2c_occ_release, +}; +static int p8_i2c_occ_getscom (struct i2c_client *client,u32 address,u8 *data) {ssize_t rc;
@@ -198,15 +211,15 @@ static int p8_i2c_occ_probe(struct i2c_client *client, {int rc;struct occ *occ; -struct p8_i2c_occ *p8_i2c_occ =devm_kzalloc (&client->dev, -sizeof (*p8_i2c_occ), -GFP_KERNEL ); +struct p8_i2c_occ *p8_i2c_occ =kzalloc (sizeof (*p8_i2c_occ), +GFP_KERNEL );if (!p8_i2c_occ)return -ENOMEM ; p8_i2c_occ->client = client; occ = &p8_i2c_occ->occ; +kobject_init (&occ->kobj, &p8_i2c_occ_type); occ->bus_dev = &client->dev; occ->groups[0 ] = &occ->group; occ->poll_cmd_data =0x10 ;
@@ -257,10 +270,14 @@ static int p8_i2c_occ_remove(struct i2c_client *client) {struct occ *occ =dev_get_drvdata (&client->dev); + occ->cancel = true; +occ_remove_status_attrs (occ);atomic_dec (&occ_num_occs); +kobject_put (&occ->kobj); +return 0 ; }
drivers/hwmon/occ/p9_sbe.c +32 -5
@@ -14,8 +14,11 @@#include <linux/platform_device.h> #include <linux/occ.h> #include <linux/sched.h> +#include <linux/slab.h> #include <linux/workqueue.h> +#define OCC_CMD_INCOMPLETE_MS 10 +struct p9_sbe_occ {struct occ occ;struct device *sbe;
@@ -23,6 +26,18 @@ struct p9_sbe_occ {#define to_p9_sbe_occ (x) container_of((x), struct p9_sbe_occ, occ) +static void p9_sbe_occ_release (struct kobject *kobj) +{ + struct occ *occ = container_of(kobj, struct occ, kobj); +struct p9_sbe_occ *p9_sbe_occ =to_p9_sbe_occ (occ); + +kfree (p9_sbe_occ); +} + +static struct kobj_type p9_sbe_occ_type = { + .release = p9_sbe_occ_release, +}; +static int p9_sbe_occ_send_cmd (struct occ *occ,u8 *cmd) {int rc, error;
@@ -34,7 +49,7 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd) start = jiffies; retry: - client =occ_drv_open (p9_sbe_occ->sbe,0 ); + client =occ_drv_open (p9_sbe_occ->sbe,O_NONBLOCK );if (!client) { rc = -ENODEV ; goto assign;
@@ -44,7 +59,15 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd)if (rc <0 ) goto err; - rc =occ_drv_read (client, (char *)resp,sizeof (*resp)); +do { + rc =occ_drv_read (client, (char *)resp,sizeof (*resp)); +if (rc != -EAGAIN ) +break ; + +set_current_state (TASK_INTERRUPTIBLE ); +schedule_timeout (msecs_to_jiffies (OCC_CMD_INCOMPLETE_MS )); + }while (!occ->cancel); +if (rc <0 ) goto err;
@@ -135,15 +158,15 @@ static int p9_sbe_occ_probe(struct platform_device *pdev) {int rc;struct occ *occ; -struct p9_sbe_occ *p9_sbe_occ =devm_kzalloc (&pdev->dev, -sizeof (*p9_sbe_occ), -GFP_KERNEL ); +struct p9_sbe_occ *p9_sbe_occ =kzalloc (sizeof (*p9_sbe_occ), +GFP_KERNEL );if (!p9_sbe_occ)return -ENOMEM ; p9_sbe_occ->sbe = pdev->dev.parent; occ = &p9_sbe_occ->occ; +kobject_init (&occ->kobj, &p9_sbe_occ_type); occ->bus_dev = &pdev->dev; occ->groups[0 ] = &occ->group; occ->poll_cmd_data =0x20 ;
@@ -165,10 +188,14 @@ static int p9_sbe_occ_remove(struct platform_device *pdev) {struct occ *occ =platform_get_drvdata (pdev); + occ->cancel = true; +occ_remove_status_attrs (occ);atomic_dec (&occ_num_occs); +kobject_put (&occ->kobj); +return 0 ; }
Signature · 1 line
1.8.3.1
2017-09-20 5:49 GMT+09:30 Eddie James <[email protected]>:
drivers/hwmon/occ/common.c struct extended_sensor { · line 121
Quoted 9 lines
drivers/hwmon/occ/common.c +1
-0@@ -118,6 +118,12 @@ struct extended_sensor {u8 data[6 ]; } __packed; +/* not available in 4.10 */
I'd prefer to backport an upstream patch.
It looks like 4.10 does have kobject_get_unless_zero, it's just not exported. I will backport c70c176ff8c3 "kobject: Export kobject_get_unless_zero()" from upstream and you can drop this hunk.
Quoted 5 lines
drivers/hwmon/occ/common.c +5
-0+static bool kobject_get_unless_zero (struct kobject *kobj) +{ +return kref_get_unless_zero (&kobj->kref); +} +