Re: [Minios-devel] [UNIKRAFT PATCH] plat/xen/drivers/blk: Fix freeing zero pages bug in blkfront_ring_fini

[Costin Lupu <costin.lupu@xxxxxxxxx>]

Hi Radu,

I'm gonna rename it to BLK_RING_PAGES_NUM on upstreaming because I think
it makes more sense like that. Other than that,

Reviewed-by: Costin Lupu <costin.lupu@xxxxxxxxx>

On 3/6/20 6:08 PM, Radu Nicolau wrote:
> An assert would cause the blkfront_ring_fini to fail because the function
> was freeing zero pages.
> Introduced the BLK_NUM_PAGES_RING which defines the number of pages a
> ring uses.
> 
> Signed-off-by: Radu Nicolau <radunicolau102@xxxxxxxxx>
> ---
>  plat/xen/drivers/blk/blkfront.c | 4 ++--
>  plat/xen/drivers/blk/blkfront.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/plat/xen/drivers/blk/blkfront.c b/plat/xen/drivers/blk/blkfront.c
> index 49d2257..498b3dd 100644
> --- a/plat/xen/drivers/blk/blkfront.c
> +++ b/plat/xen/drivers/blk/blkfront.c
> @@ -562,7 +562,7 @@ static int blkfront_ring_init(struct uk_blkdev_queue 
> *queue)
>  
>       UK_ASSERT(queue);
>       dev = queue->dev;
> -     sring = uk_palloc(queue->a, 1);
> +     sring = uk_palloc(queue->a, BLK_NUM_PAGES_RING);
>       if (!sring)
>               return -ENOMEM;
>  
> @@ -587,7 +587,7 @@ static void blkfront_ring_fini(struct uk_blkdev_queue 
> *queue)
>       }
>  
>       if (queue->ring.sring != NULL)
> -             uk_pfree(queue->a, queue->ring.sring, 0);
> +             uk_pfree(queue->a, queue->ring.sring, BLK_NUM_PAGES_RING);
>  }
>  
>  #if CONFIG_XEN_BLKFRONT_GREFPOOL
> diff --git a/plat/xen/drivers/blk/blkfront.h b/plat/xen/drivers/blk/blkfront.h
> index 9d2ad67..52a825a 100644
> --- a/plat/xen/drivers/blk/blkfront.h
> +++ b/plat/xen/drivers/blk/blkfront.h
> @@ -52,6 +52,8 @@
>  #include <common/gnttab.h>
>  #include <common/events.h>
>  
> +#define BLK_NUM_PAGES_RING 1
> +
>  #if CONFIG_XEN_BLKFRONT_GREFPOOL
>  /**
>   * Structure used to describe a list of blkfront_gref elements.
>