diff mbox series

[v2] tty: serial: dz: convert atomic_* to refcount_* APIs for irq_guard

Message ID Y6a1iJpxV0xV+NhP@qemulion
State New
Headers
Series [v2] tty: serial: dz: convert atomic_* to refcount_* APIs for irq_guard |

Commit Message

Deepak R Varma Dec. 24, 2022, 8:17 a.m. UTC 
  The refcount_* APIs are designed to address known issues with the
atomic_t APIs for reference counting. They provide following distinct
advantages:
   - protect the reference counters from overflow/underflow
   - avoid use-after-free errors
   - provide improved memory ordering guarantee schemes
   - neater and safer.
Hence, replace the atomic_* APIs by their equivalent refcount_t
API functions.

This patch proposal address the following warnings generated by
the atomic_as_refcounter.cocci coccinelle script
atomic_add_return(-1, ...)

Signed-off-by: Deepak R Varma <drv@mailo.com>
---

Changes in v2:
   1. Separate the combined change into one variable per patch as
      suggested by gregkh@linuxfoundation.org
   2. There was additional feedback on validating the change as it appeared to
      modify the existing logic. However, I found that the logic does not
      change with the proposed refcount_* APIs used in this change. Hence that
      feedback is not applied in this version.

Please Note:
   The patch is compile tested using dec_station.defconfig for MIPS architecture.


 drivers/tty/serial/dz.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

--
2.34.1

Comments

Greg KH Dec. 24, 2022, 8:37 a.m. UTC | #1 
On Sat, Dec 24, 2022 at 01:47:12PM +0530, Deepak R Varma wrote:
> The refcount_* APIs are designed to address known issues with the
> atomic_t APIs for reference counting. They provide following distinct
> advantages:
>    - protect the reference counters from overflow/underflow
>    - avoid use-after-free errors
>    - provide improved memory ordering guarantee schemes
>    - neater and safer.
> Hence, replace the atomic_* APIs by their equivalent refcount_t
> API functions.
> 
> This patch proposal address the following warnings generated by
> the atomic_as_refcounter.cocci coccinelle script
> atomic_add_return(-1, ...)
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---

This depends on the previous patch you sent, so why wasn't this a patch
series?  Always send patches as a series, otherwise I would have
attempted to apply this one first and it would have failed.  You will
recieve 0-day reports about that happening over the next few days as our
testing infrastructure will also be confused.

Please fix and resend all changes you wish to have done to this driver
together.

thanks,

greg k-h
Deepak R Varma Dec. 24, 2022, 8:40 a.m. UTC | #2 
On Sat, Dec 24, 2022 at 09:37:43AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Dec 24, 2022 at 01:47:12PM +0530, Deepak R Varma wrote:
> > The refcount_* APIs are designed to address known issues with the
> > atomic_t APIs for reference counting. They provide following distinct
> > advantages:
> >    - protect the reference counters from overflow/underflow
> >    - avoid use-after-free errors
> >    - provide improved memory ordering guarantee schemes
> >    - neater and safer.
> > Hence, replace the atomic_* APIs by their equivalent refcount_t
> > API functions.
> >
> > This patch proposal address the following warnings generated by
> > the atomic_as_refcounter.cocci coccinelle script
> > atomic_add_return(-1, ...)
> >
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> > ---
>
> This depends on the previous patch you sent, so why wasn't this a patch
> series?  Always send patches as a series, otherwise I would have
> attempted to apply this one first and it would have failed.  You will
> recieve 0-day reports about that happening over the next few days as our
> testing infrastructure will also be confused.

My apologies. I will send in the patches in a series and highlight the
dependency as well. Sorry.

./drv

>
> Please fix and resend all changes you wish to have done to this driver
> together.
>
> thanks,
>
> greg k-h
diff mbox series

Patch

diff --git a/drivers/tty/serial/dz.c b/drivers/tty/serial/dz.c
index b70edc248f8b..0aa59a9beeb7 100644
--- a/drivers/tty/serial/dz.c
+++ b/drivers/tty/serial/dz.c
@@ -46,7 +46,6 @@ 
 #include <linux/tty.h>
 #include <linux/tty_flip.h>

-#include <linux/atomic.h>
 #include <linux/refcount.h>
 #include <linux/io.h>
 #include <asm/bootinfo.h>
@@ -77,7 +76,7 @@  struct dz_port {
 struct dz_mux {
 	struct dz_port		dport[DZ_NB_PORT];
 	refcount_t		map_guard;
-	atomic_t		irq_guard;
+	refcount_t		irq_guard;
 	int			initialised;
 };

@@ -400,18 +399,16 @@  static int dz_startup(struct uart_port *uport)
 	struct dz_port *dport = to_dport(uport);
 	struct dz_mux *mux = dport->mux;
 	unsigned long flags;
-	int irq_guard;
 	int ret;
 	u16 tmp;

-	irq_guard = atomic_add_return(1, &mux->irq_guard);
-	if (irq_guard != 1)
+	refcount_inc(&mux->irq_guard);
+	if (refcount_read(&mux->irq_guard) != 1)
 		return 0;

-	ret = request_irq(dport->port.irq, dz_interrupt,
-			  IRQF_SHARED, "dz", mux);
+	ret = request_irq(dport->port.irq, dz_interrupt, IRQF_SHARED, "dz", mux);
 	if (ret) {
-		atomic_add(-1, &mux->irq_guard);
+		refcount_dec(&mux->irq_guard);
 		printk(KERN_ERR "dz: Cannot get IRQ %d!\n", dport->port.irq);
 		return ret;
 	}
@@ -441,15 +438,13 @@  static void dz_shutdown(struct uart_port *uport)
 	struct dz_port *dport = to_dport(uport);
 	struct dz_mux *mux = dport->mux;
 	unsigned long flags;
-	int irq_guard;
 	u16 tmp;

 	spin_lock_irqsave(&dport->port.lock, flags);
 	dz_stop_tx(&dport->port);
 	spin_unlock_irqrestore(&dport->port.lock, flags);

-	irq_guard = atomic_add_return(-1, &mux->irq_guard);
-	if (!irq_guard) {
+	if (refcount_dec_and_test(&mux->irq_guard)) {
 		/* Disable interrupts.  */
 		tmp = dz_in(dport, DZ_CSR);
 		tmp &= ~(DZ_RIE | DZ_TIE);