[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] 9pfs: Correctly handle cancelled requests
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH v2] 9pfs: Correctly handle cancelled requests |
Date: |
Wed, 13 Dec 2017 14:57:47 +0100 |
On Tue, 12 Dec 2017 15:14:06 -0800 (PST)
Stefano Stabellini <address@hidden> wrote:
> On Fri, 8 Dec 2017, Greg Kurz wrote:
> > Cc'ing Stefano using a more appropriate address :)
>
> Thanks Greg for the ping, one comment inline below.
>
[...]
>
> I think we also need to:
>
> /* this is required to consume the request */
> ring->intf->out_cons = ring->out_cons;
> xen_wmb();
>
> /* this is required to complete the cancellation of the pdu operation */
> ring->inprogress = false;
>
> /* this is required to let the other end know that there is more
> * space on the ring available */
> xenevtchn_notify(ring->evtchndev, ring->local_port);
>
That makes me think that we should also probably notify the guest with
virtio since this isn't a reset... :)
> /* trigger the read of the next request */
> qemu_bh_schedule(ring->bh);
>
>
> Which is basically almost everything else from xen_9pfs_push_and_notify,
> except the modification of in_prod, because we are not going to write
> back anything to the ring.
>
... in which case, the same statement stands for virtio_9pfs_push_and_notify()
We should probably unify push_and_notify and discard into a single operation:
- void (*push_and_notify)(V9fsPDU *pdu);
+ void (*return_to_guest)(V9fsPDU *pdu, bool discard);
Let's do this in a separate patch, and respin this fix on top.
>
>
> > > > +}
> > > > +
> > > > static const struct V9fsTransport xen_9p_transport = {
> > > > .pdu_vmarshal = xen_9pfs_pdu_vmarshal,
> > > > .pdu_vunmarshal = xen_9pfs_pdu_vunmarshal,
> > > > .init_in_iov_from_pdu = xen_9pfs_init_in_iov_from_pdu,
> > > > .init_out_iov_from_pdu = xen_9pfs_init_out_iov_from_pdu,
> > > > .push_and_notify = xen_9pfs_push_and_notify,
> > > > + .discard = xen_9pfs_discard,
> > > > };
> > > >
> > > > static int xen_9pfs_init(struct XenDevice *xendev)
>