Skip to contentSkip to conversation

iio: trigger: free trigger resource correctly

PATCH Superseded Alison Schofield 2 participants 3 messages Jan 18, 2017

Subsystems (3)
Staging IIO Staging - industrial io

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Alison Schofield

author
#0 lore raw

PATCH 1/1 iio: trigger: free trigger resource correctly

Using iio_trigger_put() to free a trigger leads to release of a resource we never held. Replace with iio_trigger_free().

Signed-off-by Alison Schofield <[email protected]>

---
Patches to use devm_* funcs are ready to follow this for the interrupt & bfin-timer triggers.

 drivers/iio/trigger/iio-trig-interrupt.c          | 4 ++--
 drivers/iio/trigger/iio-trig-sysfs.c              | 2 +-
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/iio/trigger/iio-trig-interrupt.c +2 -2
index 572bc6f..b18e50d 100644
@@ -84,7 +84,7 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 error_free_trig_info:
 	kfree(trig_info);
 error_put_trigger:
-	iio_trigger_put(trig);
+	iio_trigger_free(trig);
 error_ret:
 	return ret;
 }

@@ -99,7 +99,7 @@ static int iio_interrupt_trigger_remove(struct platform_device *pdev)
 	iio_trigger_unregister(trig);
 	free_irq(trig_info->irq, trig);
 	kfree(trig_info);
-	iio_trigger_put(trig);
+	iio_trigger_free(trig);
 
 	return 0;
 }
drivers/iio/trigger/iio-trig-sysfs.c +1 -1
index 3dfab2b..202e8b8 100644
@@ -174,7 +174,7 @@ static int iio_sysfs_trigger_probe(int id)
 	return 0;
 
 out2:
-	iio_trigger_put(t->trig);
+	iio_trigger_free(t->trig);
 free_t:
 	kfree(t);
 out1:
drivers/staging/iio/trigger/iio-trig-bfin-timer.c +2 -2
index 9658f20..4e0b4ee 100644
@@ -260,7 +260,7 @@ static int iio_bfin_tmr_trigger_probe(struct platform_device *pdev)
 out1:
 	iio_trigger_unregister(st->trig);
 out:
-	iio_trigger_put(st->trig);
+	iio_trigger_free(st->trig);
 	return ret;
 }
 

@@ -273,7 +273,7 @@ static int iio_bfin_tmr_trigger_remove(struct platform_device *pdev)
 		peripheral_free(st->t->pin);
 	free_irq(st->irq, st);
 	iio_trigger_unregister(st->trig);
-	iio_trigger_put(st->trig);
+	iio_trigger_free(st->trig);
 
 	return 0;
 }
Signature · 1 line
2.1.4
#1 lore raw
Quoted 2 lines · Alison Schofield

Using iio_trigger_put() to free a trigger leads to release of a resource we never held. Replace with iio_trigger_free().

They're basically the same except iio_trigger_put() puts the module and the device and free only puts the device.

I've looked at this briefly, but I can't figure out how iio_trigger_get/ iio_trigger_put is supposed to be used. There isn't any documentation. I'm trying to review this code, but I can't figure out where we *are* supposed to be doing the put.

For example, iio_device_unregister_trigger_consumer() takes a put, but iio_device_register_trigger_consumer() doesn't do a get... It's all very confusing.

You seem like you know what's going on. Can we get some documentation?

Quoted 16 lines
Signed-off-by Alison Schofield <[email protected]>

---
Patches to use devm_* funcs are ready to follow this for the interrupt & bfin-timer triggers.

 drivers/iio/trigger/iio-trig-interrupt.c          | 4 ++--
 drivers/iio/trigger/iio-trig-sysfs.c              | 2 +-
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/iio/trigger/iio-trig-interrupt.c +0 -0
index 572bc6f..b18e50d 100644

drivers/iio/trigger/iio-trig-interrupt.c static int iio_interrupt_trigger_probe(struct platform_device *pdev) · line 87

Quoted 5 lines
drivers/iio/trigger/iio-trig-interrupt.c +1 -1
@@ -84,7 +84,7 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 error_free_trig_info:
 	kfree(trig_info);
 error_put_trigger:
-	iio_trigger_put(trig);
+	iio_trigger_free(trig);

We could rename this label.

regards,
dan carpenter

author
#2 lore raw
Quoted 12 lines · Dan Carpenter
Quoted 2 lines · Alison Schofield

Using iio_trigger_put() to free a trigger leads to release of a resource we never held. Replace with iio_trigger_free().

They're basically the same except iio_trigger_put() puts the module and the device and free only puts the device.

I've looked at this briefly, but I can't figure out how iio_trigger_get/ iio_trigger_put is supposed to be used. There isn't any documentation. I'm trying to review this code, but I can't figure out where we *are* supposed to be doing the put.

Quoted 6 lines

For example, iio_device_unregister_trigger_consumer() takes a put, but iio_device_register_trigger_consumer() doesn't do a get... It's all very confusing.

You seem like you know what's going on. Can we get some documentation?

Dan,
Although I'm comfortable within the bounds of this fix (pair the alloc & free's and do not _put the module resource we never _get'd) beyond that, not so much. I'm just figuring it out myself.

linux-iio experts...pointers, help?

alisons

Quoted 28 lines
Quoted 16 lines
Signed-off-by Alison Schofield <[email protected]>

---
Patches to use devm_* funcs are ready to follow this for the interrupt & bfin-timer triggers.

 drivers/iio/trigger/iio-trig-interrupt.c          | 4 ++--
 drivers/iio/trigger/iio-trig-sysfs.c              | 2 +-
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/iio/trigger/iio-trig-interrupt.c +0 -0
index 572bc6f..b18e50d 100644

drivers/iio/trigger/iio-trig-interrupt.c static int iio_interrupt_trigger_probe(struct platform_device *pdev) · line 87

Quoted 5 lines
drivers/iio/trigger/iio-trig-interrupt.c +1 -1
@@ -84,7 +84,7 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 error_free_trig_info:
 	kfree(trig_info);
 error_put_trigger:
-	iio_trigger_put(trig);
+	iio_trigger_free(trig);

We could rename this label.

regards,
dan carpenter