> -----Original Message-----
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of Anthony PERARD
> Sent: 13 December 2018 11:34
> To: Olaf Hering <o...@aepfle.de>
> Cc: Kevin Wolf <kw...@redhat.com>; Stefano Stabellini
> <sstabell...@kernel.org>; open list:Block layer core <qemu-
> bl...@nongnu.org>; qemu-de...@nongnu.org; Max Reitz <mre...@redhat.com>;
> open list:X86 <xen-devel@lists.xenproject.org>
> Subject: Re: [Xen-devel] [PATCH v1] xen_disk: fix memory leak
>
> On Tue, Dec 11, 2018 at 05:02:24PM +0100, Olaf Hering wrote:
> > There are some code paths that clobber ioreq->buf, which leads to a huge
> > memory leak after a few hours of runtime. One code path is
> > qemu_aio_complete, which might be called recursive. Another one is
>
> I think it's s/recursive/recursively/.
>
> > ioreq_reset, which might clobber ioreq->buf as well.
> >
> > Add wrappers to free ioreq->buf before reassignment.
> >
> > Signed-off-by: Olaf Hering <o...@aepfle.de>
>
> That patch seems fine, with a few coding style issues, and is going to
> be needed to be forward ported to Paul's reimplementation (not yet
> merged).
I already posted a patch from Tim Smith (re-based to the new xen-block
datapath) that should fix this issue.
Paul
>
> > ---
> > hw/block/xen_disk.c | 22 +++++++++++++++++-----
> > 1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> > index 36eff94f84..e15eefe625 100644
> > --- a/hw/block/xen_disk.c
> > +++ b/hw/block/xen_disk.c
> > @@ -103,12 +103,24 @@ struct XenBlkDev {
> >
> > /* ------------------------------------------------------------- */
> >
> > +static void ioreq_buf_alloc(struct ioreq *ioreq, size_t alignment)
>
> You have the parameter `alignment` but don't actually use it, I don't
> think it's needed.
>
> > +{
> > + if (ioreq->buf)
> > + qemu_vfree(ioreq->buf);
>
> You could call ioreq_buf_free here instead of duplicating the code.
>
> > + ioreq->buf = qemu_memalign(XC_PAGE_SIZE, ioreq->size);
> > +}
> > +static void ioreq_buf_free(struct ioreq *ioreq)
> > +{
> > + if (ioreq->buf)
> > + qemu_vfree(ioreq->buf);
> > + ioreq->buf = NULL;
> > +}
>
> Thanks,
>
> --
> Anthony PERARD
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
