[PATCH 0/2] Fix requests loss during virtio-blk device suspend

[PATCH 0/2] Fix requests loss during virtio-blk device suspend

[Yi Sun]

When a virtio-blk device performs a large number of IO operations and
requires sleep at the same time, some requests may never be successfully
processed. 

It must be ensured that no requests in virtqueues before deleting.
The request becoming complete status means that the request has been removed
from the virtqueue.

Yi Sun (2):
  blk-mq: introduce blk_mq_tagset_wait_request_completed()
  virtio-blk: Ensure no requests in virtqueues before deleting vqs.

 block/blk-mq-tag.c         | 29 +++++++++++++++++++++++++++++
 drivers/block/virtio_blk.c |  6 ++++--
 include/linux/blk-mq.h     |  1 +
 3 files changed, 34 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/2] blk-mq: introduce blk_mq_tagset_wait_request_completed()

[Yi Sun]

In some cases, it is necessary to wait for all requests to become complete
status before performing other operations. Otherwise, these requests will never
be processed successfully.

For example, when the virtio device is in hibernation, the virtqueues
will be deleted. It must be ensured that virtqueue is not in use before being deleted.
Otherwise the requests in the virtqueue will be lost. This function can ensure
that all requests have been taken out of the virtqueues.

Prepare for fixing this kind of issue by introducing
blk_mq_tagset_wait_request_completed().

Signed-off-by: Yi Sun <yi.sun@unisoc.com>
---
 block/blk-mq-tag.c     | 29 +++++++++++++++++++++++++++++
 include/linux/blk-mq.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index cc57e2dd9a0b..cb0ef5f66c61 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -479,6 +479,35 @@ void blk_mq_tagset_wait_completed_request(struct blk_mq_tag_set *tagset)
 }
 EXPORT_SYMBOL(blk_mq_tagset_wait_completed_request);
 
+static bool blk_mq_tagset_count_inflight_rqs(struct request *rq, void *data)
+{
+	unsigned int *count = data;
+
+	if (blk_mq_request_started(rq) && !blk_mq_request_completed(rq))
+		(*count)++;
+	return true;
+}
+
+/**
+ * blk_mq_tagset_wait_request_completed - Wait for all inflight requests
+ * to become completed.
+ *
+ * Note: This function has to be run after all IO queues are shutdown.
+ */
+void blk_mq_tagset_wait_request_completed(struct blk_mq_tag_set *tagset)
+{
+	while (true) {
+		unsigned int count = 0;
+
+		blk_mq_tagset_busy_iter(tagset,
+				blk_mq_tagset_count_inflight_rqs, &count);
+		if (!count)
+			break;
+		msleep(20);
+	}
+}
+EXPORT_SYMBOL(blk_mq_tagset_wait_request_completed);
+
 /**
  * blk_mq_queue_tag_busy_iter - iterate over all requests with a driver tag
  * @q:		Request queue to examine.
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index a676e116085f..17768e154193 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -891,6 +891,7 @@ void blk_mq_run_hw_queues(struct request_queue *q, bool async);
 void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs);
 void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
 		busy_tag_iter_fn *fn, void *priv);
+void blk_mq_tagset_wait_request_completed(struct blk_mq_tag_set *tagset);
 void blk_mq_tagset_wait_completed_request(struct blk_mq_tag_set *tagset);
 void blk_mq_freeze_queue(struct request_queue *q);
 void blk_mq_unfreeze_queue(struct request_queue *q);
-- 
2.25.1

Re: [PATCH 1/2] blk-mq: introduce blk_mq_tagset_wait_request_completed()

[kernel test robot]

Hi Yi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on linus/master v6.8-rc1 next-20240123]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yi-Sun/blk-mq-introduce-blk_mq_tagset_wait_request_completed/20240122-192222
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20240122110722.690223-2-yi.sun%40unisoc.com
patch subject: [PATCH 1/2] blk-mq: introduce blk_mq_tagset_wait_request_completed()
config: i386-buildonly-randconfig-002-20240123 (https://download.01.org/0day-ci/archive/20240124/202401240242.k3K7SkiZ-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240124/202401240242.k3K7SkiZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401240242.k3K7SkiZ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> block/blk-mq-tag.c:498: warning: Function parameter or struct member 'tagset' not described in 'blk_mq_tagset_wait_request_completed'


vim +498 block/blk-mq-tag.c

   490	
   491	/**
   492	 * blk_mq_tagset_wait_request_completed - Wait for all inflight requests
   493	 * to become completed.
   494	 *
   495	 * Note: This function has to be run after all IO queues are shutdown.
   496	 */
   497	void blk_mq_tagset_wait_request_completed(struct blk_mq_tag_set *tagset)
 > 498	{
   499		while (true) {
   500			unsigned int count = 0;
   501	
   502			blk_mq_tagset_busy_iter(tagset,
   503					blk_mq_tagset_count_inflight_rqs, &count);
   504			if (!count)
   505				break;
   506			msleep(20);
   507		}
   508	}
   509	EXPORT_SYMBOL(blk_mq_tagset_wait_request_completed);
   510	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] blk-mq: introduce blk_mq_tagset_wait_request_completed()

[Keith Busch]

On Mon, Jan 22, 2024 at 07:07:21PM +0800, Yi Sun wrote:
> In some cases, it is necessary to wait for all requests to become complete
> status before performing other operations. Otherwise, these requests will never
> be processed successfully.
> 
> For example, when the virtio device is in hibernation, the virtqueues
> will be deleted. It must be ensured that virtqueue is not in use before being deleted.
> Otherwise the requests in the virtqueue will be lost. This function can ensure
> that all requests have been taken out of the virtqueues.
> 
> Prepare for fixing this kind of issue by introducing
> blk_mq_tagset_wait_request_completed().

Does blk_mq_freeze_queue() not work for your use case? I think that
should work unless you have some driver specific requests entered that
don't ever get released.
 
> +static bool blk_mq_tagset_count_inflight_rqs(struct request *rq, void *data)
> +{
> +	unsigned int *count = data;
> +
> +	if (blk_mq_request_started(rq) && !blk_mq_request_completed(rq))
> +		(*count)++;
> +	return true;
> +}
> +
> +/**
> + * blk_mq_tagset_wait_request_completed - Wait for all inflight requests
> + * to become completed.
> + *
> + * Note: This function has to be run after all IO queues are shutdown.
> + */
> +void blk_mq_tagset_wait_request_completed(struct blk_mq_tag_set *tagset)
> +{
> +	while (true) {
> +		unsigned int count = 0;
> +
> +		blk_mq_tagset_busy_iter(tagset,
> +				blk_mq_tagset_count_inflight_rqs, &count);

If the tagset is shared, then one active user can prevent this from ever
completing. It sounds like your use case cares about just one specific
request_queue, not all of them.

> +		if (!count)
> +			break;
> +		msleep(20);
> +	}
> +}
> +EXPORT_SYMBOL(blk_mq_tagset_wait_request_completed);

Re: [PATCH 1/2] blk-mq: introduce blk_mq_tagset_wait_request_completed()

[yi sun]

In my case, I want all hw queues owned by this device to be clean.
Because in the virtio device, each hw queue corresponds to a virtqueue,
and all virtqueues will be deleted when vdev suspends.

The blk_mq_tagset_wait_request_completed() function can ensure that
the device has processed all in_flight requests , and these requests have
become the complete state. I don’t understand the blk_mq_freeze_queue()
function very well. Can the function ensure that all requests have become
complete status? How should I use the function to achieve the same effect?

ps: requests become in_flight status in virtio_queue_rq() and become complete
status in virtblk_done().

Yi

On Wed, Jan 24, 2024 at 3:14 AM Keith Busch <kbusch@kernel.org> wrote:
>
> On Mon, Jan 22, 2024 at 07:07:21PM +0800, Yi Sun wrote:
> > In some cases, it is necessary to wait for all requests to become complete
> > status before performing other operations. Otherwise, these requests will never
> > be processed successfully.
> >
> > For example, when the virtio device is in hibernation, the virtqueues
> > will be deleted. It must be ensured that virtqueue is not in use before being deleted.
> > Otherwise the requests in the virtqueue will be lost. This function can ensure
> > that all requests have been taken out of the virtqueues.
> >
> > Prepare for fixing this kind of issue by introducing
> > blk_mq_tagset_wait_request_completed().
>
> Does blk_mq_freeze_queue() not work for your use case? I think that
> should work unless you have some driver specific requests entered that
> don't ever get released.
>
> > +static bool blk_mq_tagset_count_inflight_rqs(struct request *rq, void *data)
> > +{
> > +     unsigned int *count = data;
> > +
> > +     if (blk_mq_request_started(rq) && !blk_mq_request_completed(rq))
> > +             (*count)++;
> > +     return true;
> > +}
> > +
> > +/**
> > + * blk_mq_tagset_wait_request_completed - Wait for all inflight requests
> > + * to become completed.
> > + *
> > + * Note: This function has to be run after all IO queues are shutdown.
> > + */
> > +void blk_mq_tagset_wait_request_completed(struct blk_mq_tag_set *tagset)
> > +{
> > +     while (true) {
> > +             unsigned int count = 0;
> > +
> > +             blk_mq_tagset_busy_iter(tagset,
> > +                             blk_mq_tagset_count_inflight_rqs, &count);
>
> If the tagset is shared, then one active user can prevent this from ever
> completing. It sounds like your use case cares about just one specific
> request_queue, not all of them.
>
> > +             if (!count)
> > +                     break;
> > +             msleep(20);
> > +     }
> > +}
> > +EXPORT_SYMBOL(blk_mq_tagset_wait_request_completed);

Re: [PATCH 1/2] blk-mq: introduce blk_mq_tagset_wait_request_completed()

[Keith Busch]

On Wed, Jan 24, 2024 at 07:22:21PM +0800, yi sun wrote:
> In my case, I want all hw queues owned by this device to be clean.
> Because in the virtio device, each hw queue corresponds to a virtqueue,
> and all virtqueues will be deleted when vdev suspends.
> 
> The blk_mq_tagset_wait_request_completed() function can ensure that
> the device has processed all in_flight requests , and these requests have
> become the complete state. I don´t understand the blk_mq_freeze_queue()
> function very well. Can the function ensure that all requests have become
> complete status? How should I use the function to achieve the same effect?

Yeah, just do something like this:

---
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5bf98fd6a651a..2f69675abdf29 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -1593,14 +1593,14 @@ static int virtblk_freeze(struct virtio_device *vdev)
 {
        struct virtio_blk *vblk = vdev->priv;

+       blk_mq_freeze_queue(vblk->disk->queue);
+
        /* Ensure we don't receive any more interrupts */
        virtio_reset_device(vdev);

        /* Make sure no work handler is accessing the device. */
        flush_work(&vblk->config_work);

-       blk_mq_quiesce_queue(vblk->disk->queue);
-
        vdev->config->del_vqs(vdev);
        kfree(vblk->vqs);

--

[PATCH 2/2] virtio-blk: Ensure no requests in virtqueues before deleting vqs.

[Yi Sun]

Ensure no remaining requests in virtqueues before resetting vdev and
deleting virtqueues. Otherwise these requests will never be completed.
It may cause the system to become unresponsive. So it is better to place
blk_mq_quiesce_queue() in front of virtio_reset_device().

Function blk_mq_quiesce_queue() can ensure that requests have become
in_flight status, but it cannot guarantee that requests have been
processed by the device. Virtqueues should never be deleted before
all requests become complete status.

New function blk_mq_tagset_wait_request_completed() ensure that all
requests in virtqueues become complete status.

Signed-off-by: Yi Sun <yi.sun@unisoc.com>
---
 drivers/block/virtio_blk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 3b6b9abb8ce1..380f009953dd 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -1595,14 +1595,16 @@ static int virtblk_freeze(struct virtio_device *vdev)
 {
 	struct virtio_blk *vblk = vdev->priv;
 
+	/* Ensure no requests in virtqueues before deleting vqs. */
+	blk_mq_quiesce_queue(vblk->disk->queue);
+	blk_mq_tagset_wait_request_completed(vblk->disk->queue->tag_set);
+
 	/* Ensure we don't receive any more interrupts */
 	virtio_reset_device(vdev);
 
 	/* Make sure no work handler is accessing the device. */
 	flush_work(&vblk->config_work);
 
-	blk_mq_quiesce_queue(vblk->disk->queue);
-
 	vdev->config->del_vqs(vdev);
 	kfree(vblk->vqs);
 
-- 
2.25.1

Re: [PATCH 2/2] virtio-blk: Ensure no requests in virtqueues before deleting vqs.

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 664 bytes --]

On Mon, Jan 22, 2024 at 07:07:22PM +0800, Yi Sun wrote:
> Ensure no remaining requests in virtqueues before resetting vdev and
> deleting virtqueues. Otherwise these requests will never be completed.
> It may cause the system to become unresponsive. So it is better to place
> blk_mq_quiesce_queue() in front of virtio_reset_device().

QEMU's virtio-blk device implementation completes all requests during
device reset. Most device implementations have to do the same to avoid
leaving dangling requests in flight across device reset.

Which device implementation are you using and why is it safe for the
device is simply drop requests across device reset?

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] virtio-blk: Ensure no requests in virtqueues before deleting vqs.

[yi sun]

On Mon, Jan 22, 2024 at 11:43 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Mon, Jan 22, 2024 at 07:07:22PM +0800, Yi Sun wrote:
> > Ensure no remaining requests in virtqueues before resetting vdev and
> > deleting virtqueues. Otherwise these requests will never be completed.
> > It may cause the system to become unresponsive. So it is better to place
> > blk_mq_quiesce_queue() in front of virtio_reset_device().
>
> QEMU's virtio-blk device implementation completes all requests during
> device reset. Most device implementations have to do the same to avoid
> leaving dangling requests in flight across device reset.
>
> Which device implementation are you using and why is it safe for the
> device is simply drop requests across device reset?
>
> Stefan

Virtio-blk device implementation completes all requests during device reset, but
this can only ensure that the device has finished using virtqueue. We should
also consider the driver's use of virtqueue.

I caught such an example. Before del_vqs, the request had been processed by
the device, but it had not been processed by the driver. Although I am
using kernel5.4,
I think this problem may also occur with the latest version of kernel.

The debug code I added is as follows:
virtblk_freeze()
{
        vdev reset();
        quiesce queue();
        if (virtqueue->num_free != 1024) //1024 is the init value.
                BUG_ON(1);
        vdev del_vqs();
}

BUG_ON triggered the dump, the analysis is as follows:

There is one request left in request_queue.
crash_arm64> struct request ffffff81f0560000 | grep -e state -e __data_len
  __data_len = 20480,
  state = MQ_RQ_IN_FLIGHT,

crash_arm64> vring_virtqueue.packed,last_used_idx,broken,vq 0xffffff8086f92900 |
grep -e num -e used_wrap_counter -e last_used_idx -e broken -e
num_free -e desc_state -e "desc ="
        num = 1024,
        desc = 0xffffff8085ff8000,
      used_wrap_counter = false,
      desc_state = 0xffffff8085610000,
  last_used_idx = 487,
  broken = false,
    num_free = 1017,

Find desc based on last_used_idx. Through flags, we can know that the request
has been processed by the device, but it is still in flight state
because it has not
had time to run virtblk_done().
crash_arm> vring_packed_desc ffffff8085ff9e70
struct vring_packed_desc {
  addr = 10474619192,
  len = 20481,
  id = 667,
  flags = 2
}

I'm using a closed source virtual machine, so I can't see the source
code for it,
but I'm guessing it's similar to qemu.

After the device completes the request, we must also ensure that the driver can
complete the request in virtblk_done().

Re: [PATCH 2/2] virtio-blk: Ensure no requests in virtqueues before deleting vqs.

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 4039 bytes --]

Hi Michael,
This could potentially affect other VIRTIO drivers too. Please see
below.

On Tue, Jan 23, 2024 at 11:27:40AM +0800, yi sun wrote:
> On Mon, Jan 22, 2024 at 11:43 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Mon, Jan 22, 2024 at 07:07:22PM +0800, Yi Sun wrote:
> > > Ensure no remaining requests in virtqueues before resetting vdev and
> > > deleting virtqueues. Otherwise these requests will never be completed.
> > > It may cause the system to become unresponsive. So it is better to place
> > > blk_mq_quiesce_queue() in front of virtio_reset_device().
> >
> > QEMU's virtio-blk device implementation completes all requests during
> > device reset. Most device implementations have to do the same to avoid
> > leaving dangling requests in flight across device reset.
> >
> > Which device implementation are you using and why is it safe for the
> > device is simply drop requests across device reset?
> >
> > Stefan
> 
> Virtio-blk device implementation completes all requests during device reset, but
> this can only ensure that the device has finished using virtqueue. We should
> also consider the driver's use of virtqueue.
> 
> I caught such an example. Before del_vqs, the request had been processed by
> the device, but it had not been processed by the driver. Although I am
> using kernel5.4,
> I think this problem may also occur with the latest version of kernel.
> 
> The debug code I added is as follows:
> virtblk_freeze()
> {
>         vdev reset();
>         quiesce queue();
>         if (virtqueue->num_free != 1024) //1024 is the init value.
>                 BUG_ON(1);
>         vdev del_vqs();
> }
> 
> BUG_ON triggered the dump, the analysis is as follows:
> 
> There is one request left in request_queue.
> crash_arm64> struct request ffffff81f0560000 | grep -e state -e __data_len
>   __data_len = 20480,
>   state = MQ_RQ_IN_FLIGHT,
> 
> crash_arm64> vring_virtqueue.packed,last_used_idx,broken,vq 0xffffff8086f92900 |
> grep -e num -e used_wrap_counter -e last_used_idx -e broken -e
> num_free -e desc_state -e "desc ="
>         num = 1024,
>         desc = 0xffffff8085ff8000,
>       used_wrap_counter = false,
>       desc_state = 0xffffff8085610000,
>   last_used_idx = 487,
>   broken = false,
>     num_free = 1017,
> 
> Find desc based on last_used_idx. Through flags, we can know that the request
> has been processed by the device, but it is still in flight state
> because it has not
> had time to run virtblk_done().
> crash_arm> vring_packed_desc ffffff8085ff9e70
> struct vring_packed_desc {
>   addr = 10474619192,
>   len = 20481,
>   id = 667,
>   flags = 2
> }
> 
> I'm using a closed source virtual machine, so I can't see the source
> code for it,
> but I'm guessing it's similar to qemu.
> 
> After the device completes the request, we must also ensure that the driver can
> complete the request in virtblk_done().
> 

Okay, I think your approach of waiting for requests before
virtio_device_reset() makes sense. blk_mq_complete_request() is async
(might be deferred to an IPI or softirq) so it's not enough for
virtblk_done() to run before virtio_device_reset() returns. There is no
guarantee that virtblk_request_done() runs before virtio_device_reset()
returns.

A separate issue about virtio_device_reset():

Are you using virtio-mmio? virtio-mmio's vm_reset() doesn't offer the
same guarantees as virtio-pci's reset functions. virtio-pci guarantees
that irqs sent by the device during the reset operation complete and the
irq handlers run before virtio_device_reset() returns. virtio-mmio does
not.

(On top of this, virtio_device_reset() now has
CONFIG_VIRTIO_HARDEN_NOTIFICATION which changes the semantics. Drivers
cannot expect to complete any in-flight requests in
virtio_device_reset() when complied with this config option.)

Other drivers may be affected by these inconsistent
virtio_device_reset() semantics. I haven't audited the code.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] virtio-blk: Ensure no requests in virtqueues before deleting vqs.

[yi sun]

Yes, I'm using virtio-mmio.

On Tue, Jan 23, 2024 at 11:09 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> Hi Michael,
> This could potentially affect other VIRTIO drivers too. Please see
> below.
>
> On Tue, Jan 23, 2024 at 11:27:40AM +0800, yi sun wrote:
> > On Mon, Jan 22, 2024 at 11:43 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > >
> > > On Mon, Jan 22, 2024 at 07:07:22PM +0800, Yi Sun wrote:
> > > > Ensure no remaining requests in virtqueues before resetting vdev and
> > > > deleting virtqueues. Otherwise these requests will never be completed.
> > > > It may cause the system to become unresponsive. So it is better to place
> > > > blk_mq_quiesce_queue() in front of virtio_reset_device().
> > >
> > > QEMU's virtio-blk device implementation completes all requests during
> > > device reset. Most device implementations have to do the same to avoid
> > > leaving dangling requests in flight across device reset.
> > >
> > > Which device implementation are you using and why is it safe for the
> > > device is simply drop requests across device reset?
> > >
> > > Stefan
> >
> > Virtio-blk device implementation completes all requests during device reset, but
> > this can only ensure that the device has finished using virtqueue. We should
> > also consider the driver's use of virtqueue.
> >
> > I caught such an example. Before del_vqs, the request had been processed by
> > the device, but it had not been processed by the driver. Although I am
> > using kernel5.4,
> > I think this problem may also occur with the latest version of kernel.
> >
> > The debug code I added is as follows:
> > virtblk_freeze()
> > {
> >         vdev reset();
> >         quiesce queue();
> >         if (virtqueue->num_free != 1024) //1024 is the init value.
> >                 BUG_ON(1);
> >         vdev del_vqs();
> > }
> >
> > BUG_ON triggered the dump, the analysis is as follows:
> >
> > There is one request left in request_queue.
> > crash_arm64> struct request ffffff81f0560000 | grep -e state -e __data_len
> >   __data_len = 20480,
> >   state = MQ_RQ_IN_FLIGHT,
> >
> > crash_arm64> vring_virtqueue.packed,last_used_idx,broken,vq 0xffffff8086f92900 |
> > grep -e num -e used_wrap_counter -e last_used_idx -e broken -e
> > num_free -e desc_state -e "desc ="
> >         num = 1024,
> >         desc = 0xffffff8085ff8000,
> >       used_wrap_counter = false,
> >       desc_state = 0xffffff8085610000,
> >   last_used_idx = 487,
> >   broken = false,
> >     num_free = 1017,
> >
> > Find desc based on last_used_idx. Through flags, we can know that the request
> > has been processed by the device, but it is still in flight state
> > because it has not
> > had time to run virtblk_done().
> > crash_arm> vring_packed_desc ffffff8085ff9e70
> > struct vring_packed_desc {
> >   addr = 10474619192,
> >   len = 20481,
> >   id = 667,
> >   flags = 2
> > }
> >
> > I'm using a closed source virtual machine, so I can't see the source
> > code for it,
> > but I'm guessing it's similar to qemu.
> >
> > After the device completes the request, we must also ensure that the driver can
> > complete the request in virtblk_done().
> >
>
> Okay, I think your approach of waiting for requests before
> virtio_device_reset() makes sense. blk_mq_complete_request() is async
> (might be deferred to an IPI or softirq) so it's not enough for
> virtblk_done() to run before virtio_device_reset() returns. There is no
> guarantee that virtblk_request_done() runs before virtio_device_reset()
> returns.
>
> A separate issue about virtio_device_reset():
>
> Are you using virtio-mmio? virtio-mmio's vm_reset() doesn't offer the
> same guarantees as virtio-pci's reset functions. virtio-pci guarantees
> that irqs sent by the device during the reset operation complete and the
> irq handlers run before virtio_device_reset() returns. virtio-mmio does
> not.
>
> (On top of this, virtio_device_reset() now has
> CONFIG_VIRTIO_HARDEN_NOTIFICATION which changes the semantics. Drivers
> cannot expect to complete any in-flight requests in
> virtio_device_reset() when complied with this config option.)
>
> Other drivers may be affected by these inconsistent
> virtio_device_reset() semantics. I haven't audited the code.
>
> Stefan