DF-5656 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 | --- a/sys/bus/u4b/usb_dynamic.c +++ b/sys/bus/u4b/usb_dynamic.c @@ -64,6 +64,16 @@ usb_quirk_ioctl_t *usb_quirk_ioctl_p = &usb_quirk_ioctl_w; devclass_t usb_devclass_ptr = NULL; +/* + * DF-5656: quiescence barrier for the dynamically rebound USB dispatch + * pointers. Consumers hold this lock shared across the pointer read AND + * the indirect call; module unload hooks acquire it exclusive before + * resetting the pointers, which proves no CPU is still executing inside + * the module when the SYSUNINIT returns. This replaces the old fixed + * tsleep(1s) "grace period", which was not a synchronization primitive. + */ +struct lock usb_dyn_lock = LOCK_INITIALIZER("usb dyn dispatch", 0, 0); + static usb_error_t usb_temp_setup_by_index_w(struct usb_device *udev, uint16_t index) { @@ -101,44 +111,44 @@ void usb_quirk_unload(void *arg) { + /* wait for CPUs to exit the loaded functions, if any */ + + lockmgr(&usb_dyn_lock, LK_EXCLUSIVE); + /* reset function pointers */ usb_test_quirk_p = &usb_test_quirk_w; usb_quirk_ioctl_p = &usb_quirk_ioctl_w; - /* wait for CPU to exit the loaded functions, if any */ - - /* XXX this is a tradeoff */ - - tsleep(usb_quirk_unload, 0, "WAIT", hz); + lockmgr(&usb_dyn_lock, LK_RELEASE); } void usb_temp_unload(void *arg) { + /* wait for CPUs to exit the loaded functions, if any */ + + lockmgr(&usb_dyn_lock, LK_EXCLUSIVE); + /* reset function pointers */ usb_temp_get_desc_p = &usb_temp_get_desc_w; usb_temp_setup_by_index_p = &usb_temp_setup_by_index_w; usb_temp_unsetup_p = &usb_temp_unsetup_w; - /* wait for CPU to exit the loaded functions, if any */ - - /* XXX this is a tradeoff */ - - tsleep(usb_quirk_unload, 0, "WAIT", hz); + lockmgr(&usb_dyn_lock, LK_RELEASE); } void usb_bus_unload(void *arg) { - /* reset function pointers */ + /* wait for CPUs to exit the loaded functions, if any */ - usb_devclass_ptr = NULL; + lockmgr(&usb_dyn_lock, LK_EXCLUSIVE); - /* wait for CPU to exit the loaded functions, if any */ + /* reset function pointers */ - /* XXX this is a tradeoff */ + usb_devclass_ptr = NULL; - tsleep(usb_quirk_unload, 0, "WAIT", hz); + lockmgr(&usb_dyn_lock, LK_RELEASE); } --- a/sys/bus/u4b/usb_dynamic.h +++ b/sys/bus/u4b/usb_dynamic.h @@ -45,6 +45,15 @@ /* global function pointers */ +extern struct lock usb_dyn_lock; /* DF-5656 quiescence barrier */ + +/* + * Consumers must hold the dispatch lock shared across BOTH the pointer + * read and the indirect call, so an unloading module can prove quiescence. + */ +#define USB_DYN_RLOCK() lockmgr(&usb_dyn_lock, LK_SHARED) +#define USB_DYN_RUNLOCK() lockmgr(&usb_dyn_lock, LK_RELEASE) + extern usb_handle_req_t *usb_temp_get_desc_p; extern usb_temp_setup_by_index_t *usb_temp_setup_by_index_p; extern usb_temp_unsetup_t *usb_temp_unsetup_p; --- a/sys/bus/u4b/usb_dev.c +++ b/sys/bus/u4b/usb_dev.c @@ -1707,7 +1707,9 @@ case USB_QUIRK_NAME_GET: case USB_DEV_QUIRK_ADD: case USB_DEV_QUIRK_REMOVE: + USB_DYN_RLOCK(); err = usb_quirk_ioctl_p(cmd, data, fflag, td); + USB_DYN_RUNLOCK(); break; case USB_GET_TEMPLATE: *(int *)data = usb_template; --- a/sys/bus/u4b/usb_device.c +++ b/sys/bus/u4b/usb_device.c @@ -1724,7 +1724,9 @@ udev->curr_config_index = USB_UNCONFIG_INDEX; /* Setup USB descriptors */ + USB_DYN_RLOCK(); err = (usb_temp_setup_by_index_p) (udev, usb_template); + USB_DYN_RUNLOCK(); if (err) { DPRINTFN(0, "setting up USB template failed maybe the USB " "template module has not been loaded\n"); @@ -2183,7 +2185,9 @@ usbd_transfer_unsetup(udev->ctrl_xfer, USB_CTRL_XFER_MAX); /* template unsetup, if any */ + USB_DYN_RLOCK(); (usb_temp_unsetup_p) (udev); + USB_DYN_RUNLOCK(); /* * Make sure that our clear-stall messages are not queued @@ -2611,7 +2615,9 @@ /* search global quirk table, if any */ + USB_DYN_RLOCK(); found = (usb_test_quirk_p) (&uaa->info, quirk); + USB_DYN_RUNLOCK(); return (found); } --- a/sys/bus/u4b/usb_request.c +++ b/sys/bus/u4b/usb_request.c @@ -472,6 +472,7 @@ */ usbd_sr_unlock(udev); + USB_DYN_RLOCK(); hr_func = usbd_get_hr_func(udev); if (hr_func != NULL) { @@ -485,6 +486,7 @@ DPRINTFN(1, "The handle request function " "does not support writing data!\n"); err = USB_ERR_INVAL; + USB_DYN_RUNLOCK(); goto done; } } @@ -495,6 +497,8 @@ err = (hr_func) (udev, req, &desc, &temp); USB_BUS_UNLOCK(udev->bus); + USB_DYN_RUNLOCK(); + if (err) goto done; @@ -1236,6 +1240,7 @@ ptr = NULL; len = 0; + USB_DYN_RLOCK(); hr_func = usbd_get_hr_func(udev); if (hr_func == NULL) @@ -1245,6 +1250,7 @@ err = (hr_func) (udev, &req, &ptr, &len); USB_BUS_UNLOCK(udev->bus); } + USB_DYN_RUNLOCK(); if (err) ptr = NULL; |