Message ID | 149086238428.3338.4129463785289801461.stgit@bahia.lan |
---|---|
State | New |
Headers | show |
On 03/30/2017 03:26 AM, Greg Kurz wrote: > If a client tries to flush the same outstanding request several times, only > the first flush completes. Subsequent ones keep waiting for the request > completion in v9fs_flush() and, therefore, leak a PDU. This will cause QEMU > to hang when draining active PDUs the next time the device is reset. Since this fixes a hang, is it 2.9 material? > > Let have each flush request wake up the next one if any. The last waiter > frees the cancelled PDU. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > hw/9pfs/9p.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index 48babce836b6..ef47a0a5ad6f 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -2387,8 +2387,10 @@ static void coroutine_fn v9fs_flush(void *opaque) > * Wait for pdu to complete. > */ > qemu_co_queue_wait(&cancel_pdu->complete, NULL); > - cancel_pdu->cancelled = 0; > - pdu_free(cancel_pdu); > + if (!qemu_co_queue_next(&cancel_pdu->complete)) { > + cancel_pdu->cancelled = 0; > + pdu_free(cancel_pdu); > + } > } > pdu_complete(pdu, 7); > } > > >
On Thu, 30 Mar 2017 08:18:34 -0500 Eric Blake <eblake@redhat.com> wrote: > On 03/30/2017 03:26 AM, Greg Kurz wrote: > > If a client tries to flush the same outstanding request several times, only > > the first flush completes. Subsequent ones keep waiting for the request > > completion in v9fs_flush() and, therefore, leak a PDU. This will cause QEMU > > to hang when draining active PDUs the next time the device is reset. > > Since this fixes a hang, is it 2.9 material? > Yes, definitely, I just forgot to add the for-2.9 tag :) > > > > Let have each flush request wake up the next one if any. The last waiter > > frees the cancelled PDU. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > hw/9pfs/9p.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > Reviewed-by: Eric Blake <eblake@redhat.com> > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > index 48babce836b6..ef47a0a5ad6f 100644 > > --- a/hw/9pfs/9p.c > > +++ b/hw/9pfs/9p.c > > @@ -2387,8 +2387,10 @@ static void coroutine_fn v9fs_flush(void *opaque) > > * Wait for pdu to complete. > > */ > > qemu_co_queue_wait(&cancel_pdu->complete, NULL); > > - cancel_pdu->cancelled = 0; > > - pdu_free(cancel_pdu); > > + if (!qemu_co_queue_next(&cancel_pdu->complete)) { > > + cancel_pdu->cancelled = 0; > > + pdu_free(cancel_pdu); > > + } > > } > > pdu_complete(pdu, 7); > > } > > > > > > >
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 48babce836b6..ef47a0a5ad6f 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -2387,8 +2387,10 @@ static void coroutine_fn v9fs_flush(void *opaque) * Wait for pdu to complete. */ qemu_co_queue_wait(&cancel_pdu->complete, NULL); - cancel_pdu->cancelled = 0; - pdu_free(cancel_pdu); + if (!qemu_co_queue_next(&cancel_pdu->complete)) { + cancel_pdu->cancelled = 0; + pdu_free(cancel_pdu); + } } pdu_complete(pdu, 7); }
If a client tries to flush the same outstanding request several times, only the first flush completes. Subsequent ones keep waiting for the request completion in v9fs_flush() and, therefore, leak a PDU. This will cause QEMU to hang when draining active PDUs the next time the device is reset. Let have each flush request wake up the next one if any. The last waiter frees the cancelled PDU. Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/9pfs/9p.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)