[src/trunk]: src/sys/dev/usb usbdi(9): New usbd_suspend_pipe, usbd_resume_pipe.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/9c5885978791
branches:  trunk
changeset: 362567:9c5885978791
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Thu Mar 03 06:09:33 2022 +0000

description:
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.

diffstat:

 sys/dev/usb/usbdi.c |  23 +++++++++++++++++++----
 sys/dev/usb/usbdi.h |   5 ++++-
 2 files changed, 23 insertions(+), 5 deletions(-)

diffs (76 lines):

diff -r 3522689f54ef -r 9c5885978791 sys/dev/usb/usbdi.c
--- a/sys/dev/usb/usbdi.c       Thu Mar 03 06:09:20 2022 +0000
+++ b/sys/dev/usb/usbdi.c       Thu Mar 03 06:09:33 2022 +0000
@@ -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 @@
 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 @@
                        /* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */
                }
        }
-       pipe->up_aborting = 0;
        SDT_PROBE1(usb, device, pipe, abort__done,  pipe);
 }
 
diff -r 3522689f54ef -r 9c5885978791 sys/dev/usb/usbdi.h
--- a/sys/dev/usb/usbdi.h       Thu Mar 03 06:09:20 2022 +0000
+++ b/sys/dev/usb/usbdi.h       Thu Mar 03 06:09:33 2022 +0000
@@ -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 @@
 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 *);