All of lore.kernel.org
 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: [5/5] USB: cdc-acm: clean up throttle handling
Date: Thu, 25 Apr 2019 18:05:40 +0200	[thread overview]
Message-ID: <20190425160540.10036-6-johan@kernel.org> (raw)

Clean up the throttle implementation by dropping the redundant
throttle_req flag which was a remnant from back when USB serial had only
a single read URB, something which was later carried over to cdc-acm.

Also convert the throttled flag to an atomic bit flag.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/class/cdc-acm.c | 33 ++++++++-------------------------
 drivers/usb/class/cdc-acm.h |  3 +--
 2 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index c03aa8550980..183b41753c98 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -468,7 +468,6 @@ static void acm_read_bulk_callback(struct urb *urb)
 {
 	struct acm_rb *rb = urb->context;
 	struct acm *acm = rb->instance;
-	unsigned long flags;
 	int status = urb->status;
 	bool stopped = false;
 	bool stalled = false;
@@ -525,15 +524,10 @@ static void acm_read_bulk_callback(struct urb *urb)
 		return;
 	}
 
-	/* throttle device if requested by tty */
-	spin_lock_irqsave(&acm->read_lock, flags);
-	acm->throttled = acm->throttle_req;
-	if (!acm->throttled) {
-		spin_unlock_irqrestore(&acm->read_lock, flags);
-		acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
-	} else {
-		spin_unlock_irqrestore(&acm->read_lock, flags);
-	}
+	if (test_bit(ACM_THROTTLED, &acm->flags))
+		return;
+
+	acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
 }
 
 /* data interface wrote those outgoing bytes */
@@ -670,10 +664,7 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty)
 	/*
 	 * Unthrottle device in case the TTY was closed while throttled.
 	 */
-	spin_lock_irq(&acm->read_lock);
-	acm->throttled = 0;
-	acm->throttle_req = 0;
-	spin_unlock_irq(&acm->read_lock);
+	clear_bit(ACM_THROTTLED, &acm->flags);
 
 	retval = acm_submit_read_urbs(acm, GFP_KERNEL);
 	if (retval)
@@ -841,27 +832,19 @@ static void acm_tty_throttle(struct tty_struct *tty)
 {
 	struct acm *acm = tty->driver_data;
 
-	spin_lock_irq(&acm->read_lock);
-	acm->throttle_req = 1;
-	spin_unlock_irq(&acm->read_lock);
+	set_bit(ACM_THROTTLED, &acm->flags);
 }
 
 static void acm_tty_unthrottle(struct tty_struct *tty)
 {
 	struct acm *acm = tty->driver_data;
-	unsigned int was_throttled;
 
-	spin_lock_irq(&acm->read_lock);
-	was_throttled = acm->throttled;
-	acm->throttled = 0;
-	acm->throttle_req = 0;
-	spin_unlock_irq(&acm->read_lock);
+	clear_bit(ACM_THROTTLED, &acm->flags);
 
 	/* Matches the smp_mb__after_atomic() in acm_read_bulk_callback(). */
 	smp_mb();
 
-	if (was_throttled)
-		acm_submit_read_urbs(acm, GFP_KERNEL);
+	acm_submit_read_urbs(acm, GFP_KERNEL);
 }
 
 static int acm_tty_break_ctl(struct tty_struct *tty, int state)
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 515aad0847ee..ca1c026382c2 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -108,6 +108,7 @@ struct acm {
 	unsigned long flags;
 #		define EVENT_TTY_WAKEUP	0
 #		define EVENT_RX_STALL	1
+#		define ACM_THROTTLED	2
 	struct usb_cdc_line_coding line;		/* bits, stop, parity */
 	struct work_struct work;			/* work queue entry for line discipline waking up */
 	unsigned int ctrlin;				/* input control lines (DCD, DSR, RI, break, overruns) */
@@ -122,8 +123,6 @@ struct acm {
 	unsigned int ctrl_caps;				/* control capabilities from the class specific header */
 	unsigned int susp_count;			/* number of suspended interfaces */
 	unsigned int combined_interfaces:1;		/* control and data collapsed */
-	unsigned int throttled:1;			/* actually throttled */
-	unsigned int throttle_req:1;			/* throttle requested */
 	u8 bInterval;
 	struct usb_anchor delayed;			/* writes queued for a device about to be woken */
 	unsigned long quirks;

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 5/5] USB: cdc-acm: clean up throttle handling
Date: Thu, 25 Apr 2019 18:05:40 +0200	[thread overview]
Message-ID: <20190425160540.10036-6-johan@kernel.org> (raw)
Message-ID: <20190425160540.2RkjxLSb7ydhrd8t0-typaBnS4-ITdK6aQEcUZmaBSc@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 USB serial had only
a single read URB, something which was later carried over to cdc-acm.

Also convert the throttled flag to an atomic bit flag.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/class/cdc-acm.c | 33 ++++++++-------------------------
 drivers/usb/class/cdc-acm.h |  3 +--
 2 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index c03aa8550980..183b41753c98 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -468,7 +468,6 @@ static void acm_read_bulk_callback(struct urb *urb)
 {
 	struct acm_rb *rb = urb->context;
 	struct acm *acm = rb->instance;
-	unsigned long flags;
 	int status = urb->status;
 	bool stopped = false;
 	bool stalled = false;
@@ -525,15 +524,10 @@ static void acm_read_bulk_callback(struct urb *urb)
 		return;
 	}
 
-	/* throttle device if requested by tty */
-	spin_lock_irqsave(&acm->read_lock, flags);
-	acm->throttled = acm->throttle_req;
-	if (!acm->throttled) {
-		spin_unlock_irqrestore(&acm->read_lock, flags);
-		acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
-	} else {
-		spin_unlock_irqrestore(&acm->read_lock, flags);
-	}
+	if (test_bit(ACM_THROTTLED, &acm->flags))
+		return;
+
+	acm_submit_read_urb(acm, rb->index, GFP_ATOMIC);
 }
 
 /* data interface wrote those outgoing bytes */
@@ -670,10 +664,7 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty)
 	/*
 	 * Unthrottle device in case the TTY was closed while throttled.
 	 */
-	spin_lock_irq(&acm->read_lock);
-	acm->throttled = 0;
-	acm->throttle_req = 0;
-	spin_unlock_irq(&acm->read_lock);
+	clear_bit(ACM_THROTTLED, &acm->flags);
 
 	retval = acm_submit_read_urbs(acm, GFP_KERNEL);
 	if (retval)
@@ -841,27 +832,19 @@ static void acm_tty_throttle(struct tty_struct *tty)
 {
 	struct acm *acm = tty->driver_data;
 
-	spin_lock_irq(&acm->read_lock);
-	acm->throttle_req = 1;
-	spin_unlock_irq(&acm->read_lock);
+	set_bit(ACM_THROTTLED, &acm->flags);
 }
 
 static void acm_tty_unthrottle(struct tty_struct *tty)
 {
 	struct acm *acm = tty->driver_data;
-	unsigned int was_throttled;
 
-	spin_lock_irq(&acm->read_lock);
-	was_throttled = acm->throttled;
-	acm->throttled = 0;
-	acm->throttle_req = 0;
-	spin_unlock_irq(&acm->read_lock);
+	clear_bit(ACM_THROTTLED, &acm->flags);
 
 	/* Matches the smp_mb__after_atomic() in acm_read_bulk_callback(). */
 	smp_mb();
 
-	if (was_throttled)
-		acm_submit_read_urbs(acm, GFP_KERNEL);
+	acm_submit_read_urbs(acm, GFP_KERNEL);
 }
 
 static int acm_tty_break_ctl(struct tty_struct *tty, int state)
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 515aad0847ee..ca1c026382c2 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -108,6 +108,7 @@ struct acm {
 	unsigned long flags;
 #		define EVENT_TTY_WAKEUP	0
 #		define EVENT_RX_STALL	1
+#		define ACM_THROTTLED	2
 	struct usb_cdc_line_coding line;		/* bits, stop, parity */
 	struct work_struct work;			/* work queue entry for line discipline waking up */
 	unsigned int ctrlin;				/* input control lines (DCD, DSR, RI, break, overruns) */
@@ -122,8 +123,6 @@ struct acm {
 	unsigned int ctrl_caps;				/* control capabilities from the class specific header */
 	unsigned int susp_count;			/* number of suspended interfaces */
 	unsigned int combined_interfaces:1;		/* control and data collapsed */
-	unsigned int throttled:1;			/* actually throttled */
-	unsigned int throttle_req:1;			/* throttle requested */
 	u8 bInterval;
 	struct usb_anchor delayed;			/* writes queued for a device about to be woken */
 	unsigned long quirks;
-- 
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 Johan Hovold [this message]
2019-04-25 16:05 ` [PATCH 5/5] USB: cdc-acm: clean up throttle handling Johan Hovold
  -- strict thread matches above, loose matches on Subject: below --
2019-04-29 10:10 [5/5] " Oliver Neukum
2019-04-29 10:10 ` [PATCH 5/5] " Oliver Neukum
2019-04-29 10:09 [4/5] USB: cdc-acm: fix unthrottle races Oliver Neukum
2019-04-29 10:09 ` [PATCH 4/5] " Oliver Neukum
2019-04-29 10:03 [1/5] USB: serial: " Johan Hovold
2019-04-29 10:03 ` [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-25 16:05 [4/5] USB: cdc-acm: " Johan Hovold
2019-04-25 16:05 ` [PATCH 4/5] " 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 [2/5] USB: serial: clean up throttle handling Johan Hovold
2019-04-25 16:05 ` [PATCH 2/5] " Johan Hovold
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-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 20:58 ` [PATCH 0/5] USB: fix tty " 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-6-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 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.