All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] iio: trigger: Avoid data race
@ 2025-05-28 14:56 ` Gyeyoung Baek
  0 siblings, 0 replies; 6+ messages in thread
From: Gyeyoung Baek @ 2025-05-28  8:01 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel, Gyeyoung Baek

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] iio: trigger: Avoid data race
  2025-05-28 14:56 ` [PATCH v3] " Gyeyoung Baek
  (?)
@ 2025-05-28  9:22 ` Andy Shevchenko
  2025-05-28 13:27   ` Gyeyoung Baek
  -1 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2025-05-28  9:22 UTC (permalink / raw)
  To: Gyeyoung Baek
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] iio: trigger: Avoid data race
  2025-05-28  9:22 ` [PATCH v2] " Andy Shevchenko
@ 2025-05-28 13:27   ` Gyeyoung Baek
  2025-05-28 17:03     ` Jonathan Cameron
  0 siblings, 1 reply; 6+ messages in thread
From: Gyeyoung Baek @ 2025-05-28 13:27 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	linux-iio, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3] iio: trigger: Avoid data race
@ 2025-05-28 14:56 ` Gyeyoung Baek
  0 siblings, 0 replies; 6+ messages in thread
From: Gyeyoung Baek @ 2025-05-28 14:56 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio,
	linux-kernel, Gyeyoung Baek

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.

Fixes: a1a8e1dc111d ("iio:trigger: Fix use_count race condition")
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
Changelog:

v3:
 - Add Fixes tag. 
v2:
 - Edit commit message. 
 - Separate variable declaration from logic.
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] iio: trigger: Avoid data race
  2025-05-28 13:27   ` Gyeyoung Baek
@ 2025-05-28 17:03     ` Jonathan Cameron
  2025-05-29 17:01       ` Gyeyoung Baek
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2025-05-28 17:03 UTC (permalink / raw)
  To: Gyeyoung Baek
  Cc: Andy Shevchenko, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

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  
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] iio: trigger: Avoid data race
  2025-05-28 17:03     ` Jonathan Cameron
@ 2025-05-29 17:01       ` Gyeyoung Baek
  0 siblings, 0 replies; 6+ messages in thread
From: Gyeyoung Baek @ 2025-05-29 17:01 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Andy Shevchenko, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-05-29 17:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-28  8:01 [PATCH v2] iio: trigger: Avoid data race Gyeyoung Baek
2025-05-28 14:56 ` [PATCH v3] " Gyeyoung Baek
2025-05-28  9:22 ` [PATCH v2] " Andy Shevchenko
2025-05-28 13:27   ` Gyeyoung Baek
2025-05-28 17:03     ` Jonathan Cameron
2025-05-29 17:01       ` Gyeyoung Baek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.