From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Igor Skalkin <igor.skalkin@opensynergy.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
linux-kernel@vger.kernel.org,
Dan Carpenter <dan.carpenter@oracle.com>,
Hulk Robot <hulkci@huawei.com>,
Yang Yingliang <yangyingliang@huawei.com>,
Marcel Holtmann <marcel@holtmann.org>,
Johan Hedberg <johan.hedberg@gmail.com>,
linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH v3 resend] Bluetooth: virtio_bt: fix device removal
Date: Tue, 11 Oct 2022 12:57:30 -0700 [thread overview]
Message-ID: <CABBYNZJ9cj2RuJSyKcfV33N4kn1aqNQdJLFj8T5PEeJpGtRxWw@mail.gmail.com> (raw)
In-Reply-To: <794b9160-9647-7157-c84f-e278a375716e@opensynergy.com>
Hi Michael,
On Tue, Oct 11, 2022 at 5:18 AM Igor Skalkin
<igor.skalkin@opensynergy.com> wrote:
>
> 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>
Ive pushed this one.
> > ---
> >
> > 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/>
--
Luiz Augusto von Dentz
prev parent reply other threads:[~2022-10-11 19:57 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-11 9:46 [PATCH v3] Bluetooth: virtio_bt: fix device removal Michael S. Tsirkin
2022-08-15 18:07 ` Luiz Augusto von Dentz
2022-08-15 20:23 ` Michael S. Tsirkin
2022-10-07 13:00 ` Michael S. Tsirkin
2022-10-07 19:33 ` Luiz Augusto von Dentz
2022-10-10 17:15 ` Michael S. Tsirkin
2022-10-10 17:14 ` [PATCH v3 resend] " Michael S. Tsirkin
2022-10-11 12:18 ` Igor Skalkin
2022-10-11 19:57 ` Luiz Augusto von Dentz [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABBYNZJ9cj2RuJSyKcfV33N4kn1aqNQdJLFj8T5PEeJpGtRxWw@mail.gmail.com \
--to=luiz.dentz@gmail.com \
--cc=dan.carpenter@oracle.com \
--cc=hulkci@huawei.com \
--cc=igor.skalkin@opensynergy.com \
--cc=johan.hedberg@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcel@holtmann.org \
--cc=mst@redhat.com \
--cc=yangyingliang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).