Messages in this thread | | | Date | Tue, 11 Oct 2022 14:18:17 +0200 | Subject | Re: [PATCH v3 resend] Bluetooth: virtio_bt: fix device removal | From | Igor Skalkin <> |
| |
Tested, works fine in our projects.
Tested-by: Igor Skalkin <Igor.Skalkin@opensynergy.com>
On 10/10/22 19:14, Michael S. Tsirkin wrote: > Device removal is clearly out of virtio spec: it attempts to remove > unused buffers from a VQ before invoking device reset. To fix, make > open/close NOPs and do all cleanup/setup in probe/remove. > > NB: This is a hacky way to handle this - virtbt_{open,close} as NOP is > not really what a driver is supposed to be doing. These are transport > enable/disable callbacks from the BT core towards the driver. It maps to > a device being enabled/disabled by something like bluetoothd for > example. So if disabled, users expect that no resources/queues are in > use. It does work with all other transports like USB, SDIO, UART etc. > There should be no buffer used if the device is powered off. We also > don’t have any USB URBs in-flight if the transport is not active. > > The way to implement a proper fix would be using vq reset if supported, > or even using a full device reset. > > The cost of the hack is a single skb wasted on an unused bt device. > > NB2: with this fix in place driver still suffers from a race condition > if an interrupt triggers while device is being reset. To fix, in the > virtbt_close() callback we should deactivate all interrupts. To be > fixed. > > squashed fixup: bluetooth: virtio_bt: fix an error code in probe() > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > Message-Id: <20220811080943.198245-1-mst@redhat.com> > --- > > resending due to v3 having been dropped > changes from v2: > tkeaked commit log to make lines shorter > changes from v1: > fixed error handling > > drivers/bluetooth/virtio_bt.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c > index 67c21263f9e0..f6d699fed139 100644 > --- a/drivers/bluetooth/virtio_bt.c > +++ b/drivers/bluetooth/virtio_bt.c > @@ -50,8 +50,11 @@ static int virtbt_add_inbuf(struct virtio_bluetooth *vbt) > > static int virtbt_open(struct hci_dev *hdev) > { > - struct virtio_bluetooth *vbt = hci_get_drvdata(hdev); > + return 0; > +} > > +static int virtbt_open_vdev(struct virtio_bluetooth *vbt) > +{ > if (virtbt_add_inbuf(vbt) < 0) > return -EIO; > > @@ -61,7 +64,11 @@ static int virtbt_open(struct hci_dev *hdev) > > static int virtbt_close(struct hci_dev *hdev) > { > - struct virtio_bluetooth *vbt = hci_get_drvdata(hdev); > + return 0; > +} > + > +static int virtbt_close_vdev(struct virtio_bluetooth *vbt) > +{ > int i; > > cancel_work_sync(&vbt->rx); > @@ -354,8 +361,15 @@ static int virtbt_probe(struct virtio_device *vdev) > goto failed; > } > > + virtio_device_ready(vdev); > + err = virtbt_open_vdev(vbt); > + if (err) > + goto open_failed; > + > return 0; > > +open_failed: > + hci_free_dev(hdev); > failed: > vdev->config->del_vqs(vdev); > return err; > @@ -368,6 +382,7 @@ static void virtbt_remove(struct virtio_device *vdev) > > hci_unregister_dev(hdev); > virtio_reset_device(vdev); > + virtbt_close_vdev(vbt); > > hci_free_dev(hdev); > vbt->hdev = NULL;
Please mind our privacy notice<https://www.opensynergy.com/datenschutzerklaerung/privacy-notice-for-business-partners-pursuant-to-article-13-of-the-general-data-protection-regulation-gdpr/> pursuant to Art. 13 GDPR. // Unsere Hinweise zum Datenschutz gem. Art. 13 DSGVO finden Sie hier.<https://www.opensynergy.com/de/datenschutzerklaerung/datenschutzhinweise-fuer-geschaeftspartner-gem-art-13-dsgvo/>
| |