Re: [PATCH V1 2/2] xen/grant-table: Use unpopulated contiguous pages instead of real RAM ones

[Stefano Stabellini]

On Mon, 20 Jun 2022, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> 
> Depends on CONFIG_XEN_UNPOPULATED_ALLOC. If enabled then unpopulated
> contiguous pages will be allocated for grant mapping into instead of
> ballooning out real RAM pages.
> 
> Also fallback to allocate DMAable pages (balloon out real RAM pages)
> if we failed to allocate unpopulated contiguous pages. Use recently
> introduced is_xen_unpopulated_page() in gnttab_dma_free_pages() to know
> what API to use for freeing pages.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> ---
> Please note, I haven't re-checked yet the use-case where the xen-swiotlb
> is involved (proposed by Stefano):
> https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2206031348230.2783803@ubuntu-linux-20-04-desktop/
> I will re-check that for next version and add corresponding comment
> in the code.

Great. The patch looks good so far.


> Changes RFC -> V1:
>    - update commit subject/description
>    - rework to avoid introducing alternative implementation
>      of gnttab_dma_alloc(free)_pages(), use IS_ENABLED()
>    - implement a fallback to real RAM pages if we failed to allocate
>      unpopulated contiguous pages (resolve initial TODO)
>    - update according to the API renaming (s/dma/contiguous)
> ---
>  drivers/xen/grant-table.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index 738029d..15e426b 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -1047,6 +1047,23 @@ int gnttab_dma_alloc_pages(struct gnttab_dma_alloc_args *args)
>  	size_t size;
>  	int i, ret;
>  
> +	if (IS_ENABLED(CONFIG_XEN_UNPOPULATED_ALLOC)) {
> +		ret = xen_alloc_unpopulated_contiguous_pages(args->dev, args->nr_pages,
> +				args->pages);
> +		if (ret < 0)
> +			goto fallback;
> +
> +		ret = gnttab_pages_set_private(args->nr_pages, args->pages);
> +		if (ret < 0)
> +			goto fail;
> +
> +		args->vaddr = page_to_virt(args->pages[0]);
> +		args->dev_bus_addr = page_to_phys(args->pages[0]);
> +
> +		return ret;
> +	}
> +
> +fallback:
>  	size = args->nr_pages << PAGE_SHIFT;
>  	if (args->coherent)
>  		args->vaddr = dma_alloc_coherent(args->dev, size,
> @@ -1103,6 +1120,13 @@ int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args)
>  
>  	gnttab_pages_clear_private(args->nr_pages, args->pages);
>  
> +	if (IS_ENABLED(CONFIG_XEN_UNPOPULATED_ALLOC) &&
> +			is_xen_unpopulated_page(args->pages[0])) {
> +		xen_free_unpopulated_contiguous_pages(args->dev, args->nr_pages,
> +				args->pages);
> +		return 0;
> +	}
> +
>  	for (i = 0; i < args->nr_pages; i++)
>  		args->frames[i] = page_to_xen_pfn(args->pages[i]);
>  
> -- 
> 2.7.4
>