Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 10 Mar 2019 14:10:23 +0100
From:      Hans Petter Selasky <hps@selasky.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Warner Losh <imp@bsdimp.com>, FreeBSD Hackers <freebsd-hackers@freebsd.org>, "O'Connor, Daniel" <darius@dons.net.au>
Subject:   Re: USB stack getting confused
Message-ID:  <40bf77e0-47a5-6edc-b5d0-58e3c44988ac@selasky.org>
In-Reply-To: <20190310102629.GQ2492@kib.kiev.ua>
References:  <f3e6e30b-8b62-546b-2b51-e841f2e645bd@selasky.org> <3B29D870-41F9-46AF-B9F3-03106DEC417D@dons.net.au> <20190309152613.GM2492@kib.kiev.ua> <ea6e2690-1ad7-6c06-49e5-c528013f26c0@selasky.org> <20190309162640.GN2492@kib.kiev.ua> <CANCZdfr9jRcXQeZWMPKSMvUB5u7kE0eDvbuKrtGvuUDYOr=n4A@mail.gmail.com> <20190309192330.GO2492@kib.kiev.ua> <fd5038a4-406b-6e4b-bb52-b567b1954ad1@selasky.org> <20190310094758.GP2492@kib.kiev.ua> <35f69493-4bbb-4142-b61a-3e90adc8777b@selasky.org> <20190310102629.GQ2492@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------0E1FE1B809FEA2746CA2885F
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

On 3/10/19 11:26 AM, Konstantin Belousov wrote:
> On Sun, Mar 10, 2019 at 11:18:36AM +0100, Hans Petter Selasky wrote:
>> On 3/10/19 10:47 AM, Konstantin Belousov wrote:
>>>> Yes, I can do that if destroy_dev() ensures that d_close is called for
>>>> all open file handles once and only once before it returns. I think this
>>>> is where the problem comes from.
>>> See above.  For d_close it is impossible, for cdevpriv dtr it is already
>>> there by design.
>>>
>>
>> Yes, cdevpriv_dtr will wait for the final close() from user-space
>> unfortunately. Or am I mistaken?
> 
> You are mistaken.  Cdevpriv destructors are called either on the file close
> (not the last close in d_close sense, just file close) OR during destroy_dev().
> Each destructor/file pair is called exactly once, regardless of the cause.
> 

Can you try the attached patch?

--HPS

--------------0E1FE1B809FEA2746CA2885F
Content-Type: text/x-patch;
 name="usb.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="usb.diff"

Index: sys/dev/usb/controller/usb_controller.c
===================================================================
--- sys/dev/usb/controller/usb_controller.c	(revision 344575)
+++ sys/dev/usb/controller/usb_controller.c	(working copy)
@@ -664,6 +664,33 @@
 		USB_BUS_LOCK(bus);
 	}
 }
+
+void
+usb_device_cleanup(struct usb_device *udev)
+{
+	struct usb_fs_privdata *pd;
+	struct usb_bus *bus;
+	int bus_index;
+	int dev_index;
+
+	bus = udev->bus;
+	bus_index = device_get_unit(bus->bdev);
+	dev_index = udev->device_index;
+
+retry:
+	USB_BUS_LOCK(bus);
+	LIST_FOREACH(pd, &bus->pd_cleanup_list, pd_next) {
+		if (pd->bus_index != bus_index ||
+		    pd->dev_index != dev_index)
+			continue;
+		LIST_REMOVE(pd, pd_next);
+		USB_BUS_UNLOCK(bus);
+
+		usb_destroy_dev_sync(pd);
+		goto retry;
+	}
+	USB_BUS_UNLOCK(bus);
+}
 #endif
 
 static void
Index: sys/dev/usb/usb_device.c
===================================================================
--- sys/dev/usb/usb_device.c	(revision 344575)
+++ sys/dev/usb/usb_device.c	(working copy)
@@ -2322,6 +2322,13 @@
 	    &udev->cs_msg[0], &udev->cs_msg[1]);
 	USB_BUS_UNLOCK(udev->bus);
 
+#if USB_HAVE_UGEN
+	/*
+	 * Destroy character devices belonging to this
+	 * device synchronously:
+	 */
+	usb_device_cleanup(udev);
+#endif
 	/* wait for all references to go away */
 	usb_wait_pending_refs(udev);
 	
Index: sys/dev/usb/usb_device.h
===================================================================
--- sys/dev/usb/usb_device.h	(revision 344575)
+++ sys/dev/usb/usb_device.h	(working copy)
@@ -309,6 +309,7 @@
 usb_error_t	usb_suspend_resume(struct usb_device *udev,
 		    uint8_t do_suspend);
 void	usb_devinfo(struct usb_device *udev, char *dst_ptr, uint16_t dst_len);
+void	usb_device_cleanup(struct usb_device *);
 void	usb_free_device(struct usb_device *, uint8_t);
 void	usb_linux_free_device(struct usb_device *dev);
 uint8_t	usb_peer_can_wakeup(struct usb_device *udev);

--------------0E1FE1B809FEA2746CA2885F--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?40bf77e0-47a5-6edc-b5d0-58e3c44988ac>