From owner-freebsd-hackers@freebsd.org Sun Mar 10 13:10:51 2019 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 6B5A01532E6D for ; Sun, 10 Mar 2019 13:10:51 +0000 (UTC) (envelope-from hps@selasky.org) Received: from mail.turbocat.net (turbocat.net [IPv6:2a01:4f8:c17:6c4b::2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id E1FB275929 for ; Sun, 10 Mar 2019 13:10:50 +0000 (UTC) (envelope-from hps@selasky.org) Received: from hps2016.home.selasky.org (unknown [176.74.212.121]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id 6EF6B260297; Sun, 10 Mar 2019 14:10:47 +0100 (CET) Subject: Re: USB stack getting confused To: Konstantin Belousov Cc: Warner Losh , FreeBSD Hackers , "O'Connor, Daniel" References: <3B29D870-41F9-46AF-B9F3-03106DEC417D@dons.net.au> <20190309152613.GM2492@kib.kiev.ua> <20190309162640.GN2492@kib.kiev.ua> <20190309192330.GO2492@kib.kiev.ua> <20190310094758.GP2492@kib.kiev.ua> <35f69493-4bbb-4142-b61a-3e90adc8777b@selasky.org> <20190310102629.GQ2492@kib.kiev.ua> From: Hans Petter Selasky Message-ID: <40bf77e0-47a5-6edc-b5d0-58e3c44988ac@selasky.org> Date: Sun, 10 Mar 2019 14:10:23 +0100 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <20190310102629.GQ2492@kib.kiev.ua> Content-Type: multipart/mixed; boundary="------------0E1FE1B809FEA2746CA2885F" Content-Language: en-US X-Rspamd-Queue-Id: E1FB275929 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.97 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.97)[-0.973,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Mar 2019 13:10:51 -0000 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--