On Thu, Nov 17, 2011 at 05:57:06PM +0530, Amit Shah wrote:

On (Thu) 17 Nov 2011 [14:19:09], Michael S. Tsirkin wrote:

quoted

On Thu, Nov 17, 2011 at 05:27:40PM +0530, Amit Shah wrote:

quoted

quoted

+#ifdef CONFIG_PM
+static int virtnet_freeze(struct virtio_device *vdev)
+{
+	struct virtnet_info *vi = vdev->priv;
+
+	netif_device_detach(vi->dev);
+	remove_vq_common(vi);

This stops TX in progress, if any, but not RX
which might use the RX VQ. Then remove_vq_common
might delete this VQ while it's still in use.

So I think we need to call something like napi_disable.
However, the subtle twist is that we need to call that
*after interrupts have been disabled*.
Otherwise we might schedule another napi callback.

resetting the vqs will mean the host won't pass us any data in the
vqs.  Plus we're removing the vqs altogether.  Also, we're disabling
the pci device in virtio_pci.c, so all of these actions will take
care of that, isn't it?

I don't think so.

In addition, once the vqs are taken off, there's no chance for any
other rx to happen, so napi_disable() after plugging off vqs doesn't
make sense.

		Amit

Yes but napi might have been scheduled before remove_vq_common was
called, and run afterwards.

-- 
MST