[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v1] xen_disk: fix memory leak
From: |
Anthony PERARD |
Subject: |
Re: [Qemu-block] [PATCH v1] xen_disk: fix memory leak |
Date: |
Thu, 13 Dec 2018 11:33:33 +0000 |
User-agent: |
Mutt/1.11.1 (2018-12-01) |
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 <address@hidden>
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).
> ---
> 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