usb: cdc-acm: handle unlinked urb in acm read callback

PATCH In review Lu Baolu 3 participants 6 messages Dec 30, 2015 – Jan 20, 2016

Subsystems (3)

Stable branch USB USB ACM driver

lore

Share & tools

lore

Lu Baolu

author

2015-12-30 04:59 UTC #0 lore raw

PATCH v3 1/1 usb: cdc-acm: handle unlinked urb in acm read callback

In current acm driver, the bulk-in callback function ignores the URBs unlinked in usb core.

This causes unexpected data loss in some cases. For example, runtime suspend entry will unlinked all urbs and set urb->status to -ENOENT even those urbs might have data not processed yet. Hence, data loss occurs.

This patch lets bulk-in callback function handle unlinked urbs to avoid data loss.

Signed-off-by Tang Jian Qiang <[email protected]>

Signed-off-by Lu Baolu <[email protected]>

Cc [email protected]

Acked-by Oliver Neukum <[email protected]>

---
 drivers/usb/class/cdc-acm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

change log:
v1->v2:
add Acked-by: Oliver Neukum <[email protected]>.

v2->v3:
add the change log.

drivers/usb/class/cdc-acm.c +2 -1

index 26ca4f9..8cd193b 100644
@@ -428,7 +428,8 @@ static void acm_read_bulk_callback(struct urb *urb)
 		set_bit(rb->index, &acm->read_urbs_free);
 		dev_dbg(&acm->data->dev, "%s - non-zero urb status: %d\n",
 							__func__, status);
-		return;
+		if ((status != -ENOENT) || (urb->actual_length == 0))
+			return;
 	}
 
 	usb_mark_last_busy(acm->dev);

Signature · 1 line

2.1.4

Tang, Jianqiang

Acked-by Oliver Neukum on v3 1/1

2016-01-20 07:08 UTC #1 lore raw

Hi Greg,

Sorry to disturb you although I know you are very busy :)
One question is about this patch, does the patch already in the process of merge into kernel? Or need modify still?

Thanks a lot!

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Lu Baolu
Sent: Wednesday, December 30, 2015 12:59 PM
To: Oliver Neukum <[email protected]>; Greg Kroah-Hartman <[email protected]>
Cc: [email protected]; [email protected]; Lu Baolu <[email protected]>; Tang, Jianqiang <[email protected]>; [email protected]
Subject: [PATCH v3 1/1] usb: cdc-acm: handle unlinked urb in acm read callback

In current acm driver, the bulk-in callback function ignores the URBs unlinked in usb core.

This patch lets bulk-in callback function handle unlinked urbs to avoid data loss.

Signed-off-by Tang Jian Qiang <[email protected]>

Signed-off-by Lu Baolu <[email protected]>

Cc [email protected]

Acked-by Oliver Neukum <[email protected]>

---
 drivers/usb/class/cdc-acm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

change log:
v1->v2:
add Acked-by: Oliver Neukum <[email protected]>.

v2->v3:
add the change log.

drivers/usb/class/cdc-acm.c +2 -1

@@ -428,7 +428,8 @@ static void acm_read_bulk_callback(struct urb *urb)
 		set_bit(rb->index, &acm->read_urbs_free);
 		dev_dbg(&acm->data->dev, "%s - non-zero urb status: %d\n",
 							__func__, status);
-		return;
+		if ((status != -ENOENT) || (urb->actual_length == 0))
+			return;
 	}
 
 	usb_mark_last_busy(acm->dev);

--
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Lu Baolu

author

2016-01-20 07:22 UTC #2 lore raw

Hi Jianqiang,

I think you need to wait until the release of 4.5-rc1.

Thanks,
Baolu

Quoted 56 lines · Jianqiang

Hi Greg,
Sorry to disturb you although I know you are very busy :)
One question is about this patch, does the patch already in the process of merge into kernel? Or need modify still?
Thanks a lot!
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Lu Baolu
Sent: Wednesday, December 30, 2015 12:59 PM
To: Oliver Neukum <[email protected]>; Greg Kroah-Hartman <[email protected]>
Cc: [email protected]; [email protected]; Lu Baolu <[email protected]>; Tang, Jianqiang <[email protected]>; [email protected]
Subject: [PATCH v3 1/1] usb: cdc-acm: handle unlinked urb in acm read callback
In current acm driver, the bulk-in callback function ignores the URBs unlinked in usb core.
This causes unexpected data loss in some cases. For example, runtime suspend entry will unlinked all urbs and set urb->status to -ENOENT even those urbs might have data not processed yet. Hence, data loss occurs.
This patch lets bulk-in callback function handle unlinked urbs to avoid data loss.
Signed-off-by Tang Jian Qiang <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Cc [email protected]
Acked-by Oliver Neukum <[email protected]>
---
 drivers/usb/class/cdc-acm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
change log:
v1->v2:
add Acked-by: Oliver Neukum <[email protected]>.
v2->v3:
add the change log.
drivers/usb/class/cdc-acm.c +2 -2
@@ -428,7 +428,8 @@ static void acm_read_bulk_callback(struct urb *urb)
 		set_bit(rb->index, &acm->read_urbs_free);
 		dev_dbg(&acm->data->dev, "%s - non-zero urb status: %d\n",
 							__func__, status);
-		return;
+		if ((status != -ENOENT) || (urb->actual_length == 0))
+			return;
 	}
 
 	usb_mark_last_busy(acm->dev);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Tang, Jianqiang

2016-01-20 07:27 UTC #3 lore raw

Hi,
But I did not see this patch in the list of 4.5-rc1 posted by Greg.

git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.5-rc1

Thanks!

-----Original Message-----
From: Lu Baolu [mailto:[email protected]]
Sent: Wednesday, January 20, 2016 3:22 PM
To: Tang, Jianqiang <[email protected]>; Greg Kroah-Hartman <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; Oliver Neukum <[email protected]>
Subject: Re: [PATCH v3 1/1] usb: cdc-acm: handle unlinked urb in acm read callback

Hi Jianqiang,

I think you need to wait until the release of 4.5-rc1.

Thanks,
Baolu

Quoted 56 lines · Jianqiang

Hi Greg,
Sorry to disturb you although I know you are very busy :)
One question is about this patch, does the patch already in the process of merge into kernel? Or need modify still?
Thanks a lot!
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Lu Baolu
Sent: Wednesday, December 30, 2015 12:59 PM
To: Oliver Neukum <[email protected]>; Greg Kroah-Hartman <[email protected]>
Cc: [email protected]; [email protected]; Lu Baolu <[email protected]>; Tang, Jianqiang <[email protected]>; [email protected]
Subject: [PATCH v3 1/1] usb: cdc-acm: handle unlinked urb in acm read callback
In current acm driver, the bulk-in callback function ignores the URBs unlinked in usb core.
This causes unexpected data loss in some cases. For example, runtime suspend entry will unlinked all urbs and set urb->status to -ENOENT even those urbs might have data not processed yet. Hence, data loss occurs.
This patch lets bulk-in callback function handle unlinked urbs to avoid data loss.
Signed-off-by Tang Jian Qiang <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Cc [email protected]
Acked-by Oliver Neukum <[email protected]>
---
 drivers/usb/class/cdc-acm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
change log:
v1->v2:
add Acked-by: Oliver Neukum <[email protected]>.
v2->v3:
add the change log.
drivers/usb/class/cdc-acm.c +2 -2
@@ -428,7 +428,8 @@ static void acm_read_bulk_callback(struct urb *urb)
 		set_bit(rb->index, &acm->read_urbs_free);
 		dev_dbg(&acm->data->dev, "%s - non-zero urb status: %d\n",
 							__func__, status);
-		return;
+		if ((status != -ENOENT) || (urb->actual_length == 0))
+			return;
 	}
 
 	usb_mark_last_busy(acm->dev);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Greg Kroah-Hartman

maintainer

2016-01-20 07:33 UTC #4 lore raw

Quoted 4 lines · Jianqiang

Hi,
But I did not see this patch in the list of 4.5-rc1 posted by Greg.
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/ tags/usb-4.5-rc1

No, I can't do anything until after 4.5-rc1 is out, then I will review this patch and apply it to the proper branch.

thanks,

greg k-h

Tang, Jianqiang

2016-01-20 07:35 UTC #5 lore raw

Sure, thanks for your response.
Just be sure this patch will be merged.
We will wait.
Thanks!

-----Original Message-----
From: Greg Kroah-Hartman [mailto:[email protected]]
Sent: Wednesday, January 20, 2016 3:34 PM
To: Tang, Jianqiang <[email protected]>
Cc: Lu Baolu <[email protected]>; [email protected]; [email protected]; [email protected]; Oliver Neukum <[email protected]>
Subject: Re: [PATCH v3 1/1] usb: cdc-acm: handle unlinked urb in acm read callback

Quoted 5 lines · Jianqiang

Hi,
But I did not see this patch in the list of 4.5-rc1 posted by Greg.
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/
tags/usb-4.5-rc1

No, I can't do anything until after 4.5-rc1 is out, then I will review this patch and apply it to the proper branch.

thanks,

greg k-h