[v2] iio: trigger: Avoid data race

[PATCH v2] iio: trigger: Avoid data race

Posted by Gyeyoung Baek 6 months, 3 weeks ago

A data race could occur between `atomic_read()` and `atomic_set()`.
Use `atomic_cmpxchg_relaxed()` to group them atomically.

Previously the main logic was executed when `use_count` was 0.
Now it returns early when `use_count` was not 0.

Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
Changelog:

v2:
 - Edit commit message. 
 - Separate variable declaration from logic. (Andy)
---
 drivers/iio/industrialio-trigger.c | 32 +++++++++++++++---------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 54416a384232..fc799910c74d 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -196,15 +196,15 @@ void iio_trigger_poll(struct iio_trigger *trig)
 {
 	int i;
 
-	if (!atomic_read(&trig->use_count)) {
-		atomic_set(&trig->use_count, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
+	if (atomic_cmpxchg_relaxed(&trig->use_count, 0,
+				   CONFIG_IIO_CONSUMERS_PER_TRIGGER))
+		return;
 
-		for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
-			if (trig->subirqs[i].enabled)
-				generic_handle_irq(trig->subirq_base + i);
-			else
-				iio_trigger_notify_done_atomic(trig);
-		}
+	for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
+		if (trig->subirqs[i].enabled)
+			generic_handle_irq(trig->subirq_base + i);
+		else
+			iio_trigger_notify_done_atomic(trig);
 	}
 }
 EXPORT_SYMBOL(iio_trigger_poll);
@@ -227,15 +227,15 @@ void iio_trigger_poll_nested(struct iio_trigger *trig)
 {
 	int i;
 
-	if (!atomic_read(&trig->use_count)) {
-		atomic_set(&trig->use_count, CONFIG_IIO_CONSUMERS_PER_TRIGGER);
+	if (atomic_cmpxchg_relaxed(&trig->use_count, 0,
+				   CONFIG_IIO_CONSUMERS_PER_TRIGGER))
+		return;
 
-		for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
-			if (trig->subirqs[i].enabled)
-				handle_nested_irq(trig->subirq_base + i);
-			else
-				iio_trigger_notify_done(trig);
-		}
+	for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER; i++) {
+		if (trig->subirqs[i].enabled)
+			handle_nested_irq(trig->subirq_base + i);
+		else
+			iio_trigger_notify_done(trig);
 	}
 }
 EXPORT_SYMBOL(iio_trigger_poll_nested);
-- 
2.43.0

Re: [PATCH v2] iio: trigger: Avoid data race

Posted by Andy Shevchenko 6 months, 3 weeks ago

On Wed, May 28, 2025 at 10:01 AM Gyeyoung Baek <gye976@gmail.com> wrote:
>
> A data race could occur between `atomic_read()` and `atomic_set()`.
> Use `atomic_cmpxchg_relaxed()` to group them atomically.
>
> Previously the main logic was executed when `use_count` was 0.
> Now it returns early when `use_count` was not 0.

This needs Fixes tag, otherwise LGTM,
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] iio: trigger: Avoid data race

Posted by Gyeyoung Baek 6 months, 3 weeks ago

On Wed, May 28, 2025 at 6:22 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Wed, May 28, 2025 at 10:01 AM Gyeyoung Baek <gye976@gmail.com> wrote:
> >
> > A data race could occur between `atomic_read()` and `atomic_set()`.
> > Use `atomic_cmpxchg_relaxed()` to group them atomically.
> >
> > Previously the main logic was executed when `use_count` was 0.
> > Now it returns early when `use_count` was not 0.
>
> This needs Fixes tag, otherwise LGTM,
> Reviewed-by: Andy Shevchenko <andy@kernel.org>

Yes, then I'll send v3, thanks!

> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v2] iio: trigger: Avoid data race

Posted by Jonathan Cameron 6 months, 3 weeks ago

On Wed, 28 May 2025 22:27:25 +0900
Gyeyoung Baek <gye976@gmail.com> wrote:

> On Wed, May 28, 2025 at 6:22 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Wed, May 28, 2025 at 10:01 AM Gyeyoung Baek <gye976@gmail.com> wrote:  
> > >
> > > A data race could occur between `atomic_read()` and `atomic_set()`.
> > > Use `atomic_cmpxchg_relaxed()` to group them atomically.
> > >
> > > Previously the main logic was executed when `use_count` was 0.
> > > Now it returns early when `use_count` was not 0.  
> >
> > This needs Fixes tag, otherwise LGTM,
> > Reviewed-by: Andy Shevchenko <andy@kernel.org>  
> 
> Yes, then I'll send v3, thanks!
Slow down :)

Reply to v1. In general don't rush new versions out so quickly.

> 
> > --
> > With Best Regards,
> > Andy Shevchenko  
>

Re: [PATCH v2] iio: trigger: Avoid data race

Posted by Gyeyoung Baek 6 months, 3 weeks ago

On Thu, May 29, 2025 at 2:03 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Wed, 28 May 2025 22:27:25 +0900
> Gyeyoung Baek <gye976@gmail.com> wrote:
>
> > On Wed, May 28, 2025 at 6:22 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Wed, May 28, 2025 at 10:01 AM Gyeyoung Baek <gye976@gmail.com> wrote:

> >
> > Yes, then I'll send v3, thanks!
> Slow down :)
>
> Reply to v1. In general don't rush new versions out so quickly.

Yes, I should have waited for your review...
I thought the situation was pretty simple,
but there were many details as you pointed out.
I'll be more careful going forward, thanks.

--
Best regards,
Gyeyoung