Module Name:    src
Committed By:   riastradh
Date:           Thu Mar  3 06:09:33 UTC 2022

Modified Files:
        src/sys/dev/usb: usbdi.c usbdi.h

Log Message:
usbdi(9): New usbd_suspend_pipe, usbd_resume_pipe.

- New usbd_suspend_pipe to persistently stop transfers on a pipe and
  cancel pending ones or wait for their callbacks to finish.
  Idempotent.

- New usbd_resume_pipe to allow transfers again.  Idempotent, but no
  new xfers may be submitted before repeating this.

  This way it is safe to usbd_abort_pipe in two threads concurrently,
  e.g. if one thread is closing a device while another is revoking it
  -- but the threads have to agree on when it is done being aborted
  before starting to use it again.

- Existing usbd_abort_pipe now does suspend then resume.  No change
  in semantics so drivers that relied on being able to submit
  transfers again won't be broken any worse than the already are
  broken.

This allows drivers to avoid races such as:

        /* read */
        if (sc->sc_dying)
                return ENXIO;
        /* (*) */
        err = usbd_bulk_transfer(...);

        /* detach or or close */
        sc->sc_dying = true;
        usbd_abort_pipe(...);
        wait_for_io_to_drain(...);

The detach or close logic might happen at the same time as (*), with
no way to stop the bulk transfer before it starts, leading to
deadlock when detach/close waits for I/O operations like read to
drain.  Instead, the close routine can use usbd_suspend_pipe, and the
usbd_bulk_transfer is guaranteed to fail.

But some drivers such as ucom(4) don't close and reopen pipes after
aborting them -- they open on attach and close on detach, and just
abort when the /dev node is closed, expecting that xfers will
continue to work when next opened.  These drivers can instead use
usbd_suspend_pipe on close and usbd_resume_pipe on open.  Perhaps it
would be better to make them open pipes on open and close pipes on
close, but these functions make for a less intrusive transition.


To generate a diff of this commit:
cvs rdiff -u -r1.227 -r1.228 src/sys/dev/usb/usbdi.c
cvs rdiff -u -r1.106 -r1.107 src/sys/dev/usb/usbdi.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/usb/usbdi.c
diff -u src/sys/dev/usb/usbdi.c:1.227 src/sys/dev/usb/usbdi.c:1.228
--- src/sys/dev/usb/usbdi.c:1.227	Thu Mar  3 06:08:50 2022
+++ src/sys/dev/usb/usbdi.c	Thu Mar  3 06:09:33 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: usbdi.c,v 1.227 2022/03/03 06:08:50 riastradh Exp $	*/
+/*	$NetBSD: usbdi.c,v 1.228 2022/03/03 06:09:33 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.227 2022/03/03 06:08:50 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.228 2022/03/03 06:09:33 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -776,13 +776,29 @@ void
 usbd_abort_pipe(struct usbd_pipe *pipe)
 {
 
-	KASSERT(pipe != NULL);
+	usbd_suspend_pipe(pipe);
+	usbd_resume_pipe(pipe);
+}
+
+void
+usbd_suspend_pipe(struct usbd_pipe *pipe)
+{
 
 	usbd_lock_pipe(pipe);
 	usbd_ar_pipe(pipe);
 	usbd_unlock_pipe(pipe);
 }
 
+void
+usbd_resume_pipe(struct usbd_pipe *pipe)
+{
+
+	usbd_lock_pipe(pipe);
+	KASSERT(SIMPLEQ_EMPTY(&pipe->up_queue));
+	pipe->up_aborting = 0;
+	usbd_unlock_pipe(pipe);
+}
+
 usbd_status
 usbd_clear_endpoint_stall(struct usbd_pipe *pipe)
 {
@@ -1003,7 +1019,6 @@ usbd_ar_pipe(struct usbd_pipe *pipe)
 			/* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */
 		}
 	}
-	pipe->up_aborting = 0;
 	SDT_PROBE1(usb, device, pipe, abort__done,  pipe);
 }
 

Index: src/sys/dev/usb/usbdi.h
diff -u src/sys/dev/usb/usbdi.h:1.106 src/sys/dev/usb/usbdi.h:1.107
--- src/sys/dev/usb/usbdi.h:1.106	Thu Mar  3 06:06:52 2022
+++ src/sys/dev/usb/usbdi.h	Thu Mar  3 06:09:33 2022
@@ -1,4 +1,4 @@
-/*	$NetBSD: usbdi.h,v 1.106 2022/03/03 06:06:52 riastradh Exp $	*/
+/*	$NetBSD: usbdi.h,v 1.107 2022/03/03 06:09:33 riastradh Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/usbdi.h,v 1.18 1999/11/17 22:33:49 n_hibma Exp $	*/
 
 /*
@@ -125,6 +125,9 @@ usb_endpoint_descriptor_t *usbd_interfac
 void usbd_abort_pipe(struct usbd_pipe *);
 void usbd_abort_default_pipe(struct usbd_device *);
 
+void usbd_suspend_pipe(struct usbd_pipe *);
+void usbd_resume_pipe(struct usbd_pipe *);
+
 usbd_status usbd_clear_endpoint_stall(struct usbd_pipe *);
 void usbd_clear_endpoint_stall_async(struct usbd_pipe *);
 

Reply via email to