On Thu, Mar 20, 2025 at 05:30:02PM +0000, Matthew Auld wrote:
Maybe try rewording this. I find the lower in patch subject / usage
below a bit confusing.
> Lower get/unmap pages to facilitate operating on the lowest level
> pieces, without needing a full drm_gpusvm_range structure. In the next
> patch we want to extract get/unmap/free to operate on a different range
> type.
>
> Signed-off-by: Matthew Auld <matthew.a...@intel.com>
> Cc: Thomas Hellström <thomas.hellst...@linux.intel.com>
> Cc: Matthew Brost <matthew.br...@intel.com>
> ---
> drivers/gpu/drm/drm_gpusvm.c | 90 ++++++++++++++++++++++--------------
> 1 file changed, 55 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index f27731a51f34..2beca5a6dc0a 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1323,38 +1323,28 @@ drm_gpusvm_range_pages_valid_unlocked(struct
> drm_gpusvm *gpusvm,
> return pages_valid;
> }
>
> -/**
> - * drm_gpusvm_range_get_pages() - Get pages for a GPU SVM range
> - * @gpusvm: Pointer to the GPU SVM structure
> - * @range: Pointer to the GPU SVM range structure
> - * @ctx: GPU SVM context
> - *
> - * This function gets pages for a GPU SVM range and ensures they are mapped
> for
> - * DMA access.
> - *
> - * Return: 0 on success, negative error code on failure.
> - */
> -int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm,
> - struct drm_gpusvm_range *range,
> - const struct drm_gpusvm_ctx *ctx)
> +static int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> + struct drm_gpusvm_pages *svm_pages,
> + struct mm_struct *mm,
> + struct mmu_interval_notifier *notifier,
> + unsigned long *notifier_seq,
> + unsigned long mm_start,
> + unsigned long mm_end,
s/mm_start/pages_start ?
s/mm_end/pages_end ?
Matt
> + const struct drm_gpusvm_ctx *ctx)
> {
> - struct drm_gpusvm_pages *svm_pages = &range->pages;
> - struct mmu_interval_notifier *notifier = &range->notifier->notifier;
> struct hmm_range hmm_range = {
> .default_flags = HMM_PFN_REQ_FAULT | (ctx->read_only ? 0 :
> HMM_PFN_REQ_WRITE),
> .notifier = notifier,
> - .start = drm_gpusvm_range_start(range),
> - .end = drm_gpusvm_range_end(range),
> + .start = mm_start,
> + .end = mm_end,
> .dev_private_owner = gpusvm->device_private_page_owner,
> };
> - struct mm_struct *mm = gpusvm->mm;
> struct drm_gpusvm_zdd *zdd;
> unsigned long timeout =
> jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> unsigned long i, j;
> - unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
> - drm_gpusvm_range_end(range));
> + unsigned long npages = npages_in_range(mm_start, mm_end);
> unsigned long num_dma_mapped;
> unsigned int order = 0;
> unsigned long *pfns;
> @@ -1518,7 +1508,7 @@ int drm_gpusvm_range_get_pages(struct drm_gpusvm
> *gpusvm,
> drm_gpusvm_notifier_unlock(gpusvm);
> kvfree(pfns);
> set_seqno:
> - range->notifier_seq = hmm_range.notifier_seq;
> + *notifier_seq = hmm_range.notifier_seq;
>
> return 0;
>
> @@ -1531,8 +1521,48 @@ int drm_gpusvm_range_get_pages(struct drm_gpusvm
> *gpusvm,
> goto retry;
> return err;
> }
> +
> +/**
> + * drm_gpusvm_range_get_pages() - Get pages for a GPU SVM range
> + * @gpusvm: Pointer to the GPU SVM structure
> + * @range: Pointer to the GPU SVM range structure
> + * @ctx: GPU SVM context
> + *
> + * This function gets pages for a GPU SVM range and ensures they are mapped
> for
> + * DMA access.
> + *
> + * Return: 0 on success, negative error code on failure.
> + */
> +int drm_gpusvm_range_get_pages(struct drm_gpusvm *gpusvm,
> + struct drm_gpusvm_range *range,
> + const struct drm_gpusvm_ctx *ctx)
> +{
> + return drm_gpusvm_get_pages(gpusvm, &range->pages, gpusvm->mm,
> + &range->notifier->notifier,
> + &range->notifier_seq,
> + drm_gpusvm_range_start(range),
> + drm_gpusvm_range_end(range), ctx);
> +}
> EXPORT_SYMBOL_GPL(drm_gpusvm_range_get_pages);
>
> +static void drm_gpusvm_unmap_pages(struct drm_gpusvm *gpusvm,
> + unsigned long mm_start, unsigned long mm_end,
> + struct drm_gpusvm_pages *svm_pages,
> + const struct drm_gpusvm_ctx *ctx)
> +{
> + unsigned long npages = npages_in_range(mm_start, mm_end);
> +
> + if (ctx->in_notifier)
> + lockdep_assert_held_write(&gpusvm->notifier_lock);
> + else
> + drm_gpusvm_notifier_lock(gpusvm);
> +
> + __drm_gpusvm_unmap_pages(gpusvm, svm_pages, npages);
> +
> + if (!ctx->in_notifier)
> + drm_gpusvm_notifier_unlock(gpusvm);
> +}
> +
> /**
> * drm_gpusvm_range_unmap_pages() - Unmap pages associated with a GPU SVM
> range
> * @gpusvm: Pointer to the GPU SVM structure
> @@ -1549,19 +1579,9 @@ void drm_gpusvm_range_unmap_pages(struct drm_gpusvm
> *gpusvm,
> struct drm_gpusvm_range *range,
> const struct drm_gpusvm_ctx *ctx)
> {
> - struct drm_gpusvm_pages *svm_pages = &range->pages;
> - unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
> - drm_gpusvm_range_end(range));
> -
> - if (ctx->in_notifier)
> - lockdep_assert_held_write(&gpusvm->notifier_lock);
> - else
> - drm_gpusvm_notifier_lock(gpusvm);
> -
> - __drm_gpusvm_unmap_pages(gpusvm, svm_pages, npages);
> -
> - if (!ctx->in_notifier)
> - drm_gpusvm_notifier_unlock(gpusvm);
> + return drm_gpusvm_unmap_pages(gpusvm, drm_gpusvm_range_start(range),
> + drm_gpusvm_range_end(range),
> + &range->pages, ctx);
> }
> EXPORT_SYMBOL_GPL(drm_gpusvm_range_unmap_pages);
>
> --
> 2.48.1
>
