Re: [PATCH 2/3] drm/xe: Introduce a helper to free sg table

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Mar 21, 2024 at 11:55:19PM -0400, Oak Zeng wrote:
> Introduce xe_userptr_free_sg helper to dma-unmap all
> addresses in userptr's sg table and free sg table.
> 
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> Suggested by: Matthew Brost <matthew.brost@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_hmm.c | 30 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_hmm.h |  1 +
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hmm.c b/drivers/gpu/drm/xe/xe_hmm.c
> index 4011207630a5..427c6bc49949 100644
> --- a/drivers/gpu/drm/xe/xe_hmm.c
> +++ b/drivers/gpu/drm/xe/xe_hmm.c
> @@ -3,6 +3,7 @@
>   * Copyright © 2024 Intel Corporation
>   */
>  
> +#include <linux/scatterlist.h>
>  #include <linux/mmu_notifier.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/memremap.h>
> @@ -107,6 +108,32 @@ static int xe_build_sg(struct xe_device *xe, struct hmm_range *range,
>  	return ret;
>  }
>  
> +/**
> + * xe_userptr_free_sg() - Free the scatter gather table of userptr
> + *
> + * @uvma: the userptr vma which hold the scatter gather table
> + *
> + * With function xe_userptr_populate_range, we allocate storage of
> + * the userptr sg table. This is a helper function to free this
> + * sg table, and dma unmap the address in the table.
> + */
> +void xe_userptr_free_sg(struct xe_userptr_vma *uvma)

See my comment about passing in xe_userptr_vma as an argument in the
previous patch, anyways can live with it.

But s/xe_userptr_free_sg/xe_hmm_userptr_free_sg?

Also I'd squash into the previous patch.

Otherwise LGMT.

Matt

> +{
> +	struct xe_userptr *userptr = &uvma->userptr;
> +	struct xe_vma *vma = &uvma->vma;
> +	bool write = !xe_vma_read_only(vma);
> +	struct xe_vm *vm = xe_vma_vm(vma);
> +	struct xe_device *xe = vm->xe;
> +	struct device *dev = xe->drm.dev;
> +
> +	xe_assert(xe, userptr->sg);
> +	dma_unmap_sgtable(dev, userptr->sg,
> +			write ? DMA_BIDIRECTIONAL : DMA_TO_DEVICE, 0);
> +
> +	sg_free_table(userptr->sg);
> +	userptr->sg = NULL;
> +}
> +
>  /**
>   * xe_userptr_populate_range() - Populate physical pages of a virtual
>   * address range
> @@ -156,6 +183,9 @@ int xe_userptr_populate_range(struct xe_userptr_vma *uvma)
>  	if (notifier_seq == userptr->notifier_seq)
>  		return 0;
>  
> +	if (userptr->sg)
> +		xe_userptr_free_sg(uvma);
> +
>  	npages = xe_npages_in_range(start, end);
>  	pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
>  	if (unlikely(!pfns))
> diff --git a/drivers/gpu/drm/xe/xe_hmm.h b/drivers/gpu/drm/xe/xe_hmm.h
> index 91686a751711..7bb49bbde5a4 100644
> --- a/drivers/gpu/drm/xe/xe_hmm.h
> +++ b/drivers/gpu/drm/xe/xe_hmm.h
> @@ -15,3 +15,4 @@ static inline int xe_userptr_populate_range(struct xe_userptr_vma *uvma)
>  	return -ENODEV;
>  }
>  #endif
> +void xe_userptr_free_sg(struct xe_userptr_vma *uvma);
> -- 
> 2.26.3
>