linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Alan Stern <stern@rowland.harvard.edu>,
	Oliver Neukum <oneukum@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org, Johan Hovold <johan@kernel.org>
Subject: [2/5] USB: serial: clean up throttle handling
Date: Thu, 25 Apr 2019 18:05:37 +0200	[thread overview]
Message-ID: <20190425160540.10036-3-johan@kernel.org> (raw)

Clean up the throttle implementation by dropping the redundant
throttle_req flag which was a remnant from back when there was only a
single read URB.

Also convert the throttled flag to an atomic bit flag.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/generic.c | 34 ++++++++--------------------------
 include/linux/usb/serial.h   |  5 +----
 2 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 0fff4968ea1b..67cef3ef1e5f 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -106,12 +106,8 @@ void usb_serial_generic_deregister(void)
 int usb_serial_generic_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	int result = 0;
-	unsigned long flags;
 
-	spin_lock_irqsave(&port->lock, flags);
-	port->throttled = 0;
-	port->throttle_req = 0;
-	spin_unlock_irqrestore(&port->lock, flags);
+	clear_bit(USB_SERIAL_THROTTLED, &port->flags);
 
 	if (port->bulk_in_size)
 		result = usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
@@ -375,7 +371,6 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 {
 	struct usb_serial_port *port = urb->context;
 	unsigned char *data = urb->transfer_buffer;
-	unsigned long flags;
 	bool stopped = false;
 	int status = urb->status;
 	int i;
@@ -429,15 +424,10 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 	if (stopped)
 		return;
 
-	/* Throttle the device if requested by tty */
-	spin_lock_irqsave(&port->lock, flags);
-	port->throttled = port->throttle_req;
-	if (!port->throttled) {
-		spin_unlock_irqrestore(&port->lock, flags);
-		usb_serial_generic_submit_read_urb(port, i, GFP_ATOMIC);
-	} else {
-		spin_unlock_irqrestore(&port->lock, flags);
-	}
+	if (test_bit(USB_SERIAL_THROTTLED, &port->flags))
+		return;
+
+	usb_serial_generic_submit_read_urb(port, i, GFP_ATOMIC);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_read_bulk_callback);
 
@@ -485,23 +475,16 @@ EXPORT_SYMBOL_GPL(usb_serial_generic_write_bulk_callback);
 void usb_serial_generic_throttle(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	unsigned long flags;
 
-	spin_lock_irqsave(&port->lock, flags);
-	port->throttle_req = 1;
-	spin_unlock_irqrestore(&port->lock, flags);
+	set_bit(USB_SERIAL_THROTTLED, &port->flags);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_throttle);
 
 void usb_serial_generic_unthrottle(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	int was_throttled;
 
-	spin_lock_irq(&port->lock);
-	was_throttled = port->throttled;
-	port->throttled = port->throttle_req = 0;
-	spin_unlock_irq(&port->lock);
+	clear_bit(USB_SERIAL_THROTTLED, &port->flags);
 
 	/*
 	 * Matches the smp_mb__after_atomic() in
@@ -509,8 +492,7 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty)
 	 */
 	smp_mb();
 
-	if (was_throttled)
-		usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
+	usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_unthrottle);
 
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 1c19f77ed541..d8bdab8f3c26 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -28,6 +28,7 @@
 
 /* USB serial flags */
 #define USB_SERIAL_WRITE_BUSY	0
+#define USB_SERIAL_THROTTLED	1
 
 /**
  * usb_serial_port: structure for the specific ports of a device.
@@ -67,8 +68,6 @@
  * @flags: usb serial port flags
  * @write_wait: a wait_queue_head_t used by the port.
  * @work: work queue entry for the line discipline waking up.
- * @throttled: nonzero if the read urb is inactive to throttle the device
- * @throttle_req: nonzero if the tty wants to throttle us
  * @dev: pointer to the serial device
  *
  * This structure is used by the usb-serial core and drivers for the specific
@@ -115,8 +114,6 @@ struct usb_serial_port {
 	unsigned long		flags;
 	wait_queue_head_t	write_wait;
 	struct work_struct	work;
-	char			throttled;
-	char			throttle_req;
 	unsigned long		sysrq; /* sysrq timeout */
 	struct device		dev;
 };

WARNING: multiple messages have this Message-ID (diff)
From: Johan Hovold <johan@kernel.org>
To: Alan Stern <stern@rowland.harvard.edu>,
	Oliver Neukum <oneukum@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org, Johan Hovold <johan@kernel.org>
Subject: [PATCH 2/5] USB: serial: clean up throttle handling
Date: Thu, 25 Apr 2019 18:05:37 +0200	[thread overview]
Message-ID: <20190425160540.10036-3-johan@kernel.org> (raw)
Message-ID: <20190425160537.hShxciSxMoKv8X0_CqOsUhLOTgnL00vyHt13NQcLZL0@z> (raw)
In-Reply-To: <20190425160540.10036-1-johan@kernel.org>

Clean up the throttle implementation by dropping the redundant
throttle_req flag which was a remnant from back when there was only a
single read URB.

Also convert the throttled flag to an atomic bit flag.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/generic.c | 34 ++++++++--------------------------
 include/linux/usb/serial.h   |  5 +----
 2 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 0fff4968ea1b..67cef3ef1e5f 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -106,12 +106,8 @@ void usb_serial_generic_deregister(void)
 int usb_serial_generic_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	int result = 0;
-	unsigned long flags;
 
-	spin_lock_irqsave(&port->lock, flags);
-	port->throttled = 0;
-	port->throttle_req = 0;
-	spin_unlock_irqrestore(&port->lock, flags);
+	clear_bit(USB_SERIAL_THROTTLED, &port->flags);
 
 	if (port->bulk_in_size)
 		result = usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
@@ -375,7 +371,6 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 {
 	struct usb_serial_port *port = urb->context;
 	unsigned char *data = urb->transfer_buffer;
-	unsigned long flags;
 	bool stopped = false;
 	int status = urb->status;
 	int i;
@@ -429,15 +424,10 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
 	if (stopped)
 		return;
 
-	/* Throttle the device if requested by tty */
-	spin_lock_irqsave(&port->lock, flags);
-	port->throttled = port->throttle_req;
-	if (!port->throttled) {
-		spin_unlock_irqrestore(&port->lock, flags);
-		usb_serial_generic_submit_read_urb(port, i, GFP_ATOMIC);
-	} else {
-		spin_unlock_irqrestore(&port->lock, flags);
-	}
+	if (test_bit(USB_SERIAL_THROTTLED, &port->flags))
+		return;
+
+	usb_serial_generic_submit_read_urb(port, i, GFP_ATOMIC);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_read_bulk_callback);
 
@@ -485,23 +475,16 @@ EXPORT_SYMBOL_GPL(usb_serial_generic_write_bulk_callback);
 void usb_serial_generic_throttle(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	unsigned long flags;
 
-	spin_lock_irqsave(&port->lock, flags);
-	port->throttle_req = 1;
-	spin_unlock_irqrestore(&port->lock, flags);
+	set_bit(USB_SERIAL_THROTTLED, &port->flags);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_throttle);
 
 void usb_serial_generic_unthrottle(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;
-	int was_throttled;
 
-	spin_lock_irq(&port->lock);
-	was_throttled = port->throttled;
-	port->throttled = port->throttle_req = 0;
-	spin_unlock_irq(&port->lock);
+	clear_bit(USB_SERIAL_THROTTLED, &port->flags);
 
 	/*
 	 * Matches the smp_mb__after_atomic() in
@@ -509,8 +492,7 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty)
 	 */
 	smp_mb();
 
-	if (was_throttled)
-		usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
+	usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
 }
 EXPORT_SYMBOL_GPL(usb_serial_generic_unthrottle);
 
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 1c19f77ed541..d8bdab8f3c26 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -28,6 +28,7 @@
 
 /* USB serial flags */
 #define USB_SERIAL_WRITE_BUSY	0
+#define USB_SERIAL_THROTTLED	1
 
 /**
  * usb_serial_port: structure for the specific ports of a device.
@@ -67,8 +68,6 @@
  * @flags: usb serial port flags
  * @write_wait: a wait_queue_head_t used by the port.
  * @work: work queue entry for the line discipline waking up.
- * @throttled: nonzero if the read urb is inactive to throttle the device
- * @throttle_req: nonzero if the tty wants to throttle us
  * @dev: pointer to the serial device
  *
  * This structure is used by the usb-serial core and drivers for the specific
@@ -115,8 +114,6 @@ struct usb_serial_port {
 	unsigned long		flags;
 	wait_queue_head_t	write_wait;
 	struct work_struct	work;
-	char			throttled;
-	char			throttle_req;
 	unsigned long		sysrq; /* sysrq timeout */
 	struct device		dev;
 };
-- 
2.21.0


         reply	other threads:[~2019-04-25 16:05 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-25 16:05 [PATCH 0/5] USB: fix tty unthrottle races Johan Hovold
2019-04-25 16:05 ` [1/5] USB: serial: fix " Johan Hovold
2019-04-25 16:05   ` [PATCH 1/5] " Johan Hovold
2019-04-29  9:50   ` [1/5] " Oliver Neukum
2019-04-29  9:50     ` [PATCH 1/5] " Oliver Neukum
2019-04-29 10:03     ` [1/5] " Johan Hovold
2019-04-29 10:03       ` [PATCH 1/5] " Johan Hovold
2019-05-13 10:43   ` Johan Hovold
2019-05-13 10:56     ` Greg Kroah-Hartman
2019-05-13 11:46       ` Johan Hovold
2019-05-13 12:51         ` Greg Kroah-Hartman
2019-05-13 12:59           ` Johan Hovold
2019-05-14 12:53             ` Sasha Levin
2019-05-14 12:57               ` Johan Hovold
2019-04-25 16:05 ` Johan Hovold [this message]
2019-04-25 16:05   ` [PATCH 2/5] USB: serial: clean up throttle handling Johan Hovold
2019-04-25 16:05 ` [3/5] USB: serial: generic: drop unnecessary goto Johan Hovold
2019-04-25 16:05   ` [PATCH 3/5] " Johan Hovold
2019-04-25 16:05 ` [4/5] USB: cdc-acm: fix unthrottle races Johan Hovold
2019-04-25 16:05   ` [PATCH 4/5] " Johan Hovold
2019-04-29 10:09   ` [4/5] " Oliver Neukum
2019-04-29 10:09     ` [PATCH 4/5] " Oliver Neukum
2019-04-25 16:05 ` [5/5] USB: cdc-acm: clean up throttle handling Johan Hovold
2019-04-25 16:05   ` [PATCH 5/5] " Johan Hovold
2019-04-29 10:10   ` [5/5] " Oliver Neukum
2019-04-29 10:10     ` [PATCH 5/5] " Oliver Neukum
2019-04-25 20:58 ` [PATCH 0/5] USB: fix tty unthrottle races Alan Stern
2019-04-26  4:55   ` Johan Hovold
2019-04-29  9:30 ` Johan Hovold

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190425160540.10036-3-johan@kernel.org \
    --to=johan@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=oneukum@suse.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).