From: Jason Gunthorpe <jgg@nvidia.com>
To: Oak Zeng <oak.zeng@intel.com>
Cc: dri-devel@lists.freedesktop.org, intel-xe@lists.freedesktop.org,
matthew.brost@intel.com, Thomas.Hellstrom@linux.intel.com,
brian.welty@intel.com, himal.prasad.ghimiray@intel.com,
krishnaiah.bommu@intel.com, niranjana.vishwanathapura@intel.com,
Leon Romanovsky <leon@kernel.org>
Subject: Re: [PATCH 06/23] drm/xe/svm: Introduce a helper to build sg table from hmm range
Date: Thu, 4 Apr 2024 21:39:27 -0300 [thread overview]
Message-ID: <20240405003927.GA11940@nvidia.com> (raw)
In-Reply-To: <20240117221223.18540-7-oak.zeng@intel.com>
On Wed, Jan 17, 2024 at 05:12:06PM -0500, Oak Zeng wrote:
> +/**
> + * xe_svm_build_sg() - build a scatter gather table for all the physical pages/pfn
> + * in a hmm_range.
> + *
> + * @range: the hmm range that we build the sg table from. range->hmm_pfns[]
> + * has the pfn numbers of pages that back up this hmm address range.
> + * @st: pointer to the sg table.
> + *
> + * All the contiguous pfns will be collapsed into one entry in
> + * the scatter gather table. This is for the convenience of
> + * later on operations to bind address range to GPU page table.
> + *
> + * This function allocates the storage of the sg table. It is
> + * caller's responsibility to free it calling sg_free_table.
> + *
> + * Returns 0 if successful; -ENOMEM if fails to allocate memory
> + */
> +int xe_svm_build_sg(struct hmm_range *range,
> + struct sg_table *st)
> +{
> + struct scatterlist *sg;
> + u64 i, npages;
> +
> + sg = NULL;
> + st->nents = 0;
> + npages = ((range->end - 1) >> PAGE_SHIFT) - (range->start >> PAGE_SHIFT) + 1;
> +
> + if (unlikely(sg_alloc_table(st, npages, GFP_KERNEL)))
> + return -ENOMEM;
> +
> + for (i = 0; i < npages; i++) {
> + unsigned long addr = range->hmm_pfns[i];
> +
> + if (sg && (addr == (sg_dma_address(sg) + sg->length))) {
> + sg->length += PAGE_SIZE;
> + sg_dma_len(sg) += PAGE_SIZE;
> + continue;
> + }
> +
> + sg = sg ? sg_next(sg) : st->sgl;
> + sg_dma_address(sg) = addr;
> + sg_dma_len(sg) = PAGE_SIZE;
> + sg->length = PAGE_SIZE;
> + st->nents++;
> + }
> +
> + sg_mark_end(sg);
> + return 0;
> +}
I didn't look at this series a lot but I wanted to make a few
remarks.. This I don't like quite a lot. Yes, the DMA API interaction
with hmm_range_fault is pretty bad, but it should not be hacked
around like this. Leon is working on a series to improve it:
https://lore.kernel.org/linux-rdma/cover.1709635535.git.leon@kernel.org/
Please participate there too. In the mean time you should just call
dma_map_page for every single page like ODP does.
Also, I tried to follow the large discussion in the end but it was
quite hard to read the text in Lore for some reason.
I would just opine some general points on how I see hmm_range_fault
being used by drivers.
First of all, the device should have a private page table. At least
one, but ideally many. Obviously it should work, so I found it a bit
puzzling the talk about problems with virtualization. Either the
private page table works virtualized, including faults, or it should
not be available..
Second, I see hmm_range_fault as having two main design patterns
interactions. Either it is the entire exclusive owner of a single
device private page table and fully mirrors the mm page table into the
device table.
Or it is a selective mirror where it copies part of the mm page table
into a "vma" of a possibly shared device page table. The
hmm_range_fault bit would exclusively own it's bit of VMA.
So I find it a quite strange that this RFC is creating VMA's,
notifiers and ranges on the fly. That should happen when userspace
indicates it wants some/all of the MM to be bound to a specific device
private pagetable/address space.
I also agree with the general spirit of the remarks that there should
not be a process binding or any kind of "global" character
device. Device private page tables are by their very nature private to
the device and should be created through a device specific char
dev. If you have a VMA concept for these page tables then a HMM
mirroring one is simply a different type of VMA along with all the
others.
I was also looking at the mmu notifier register/unregister with some
suspicion, it seems wrong to have a patch talking about "process
exit". Notifiers should be destroyed when the device private page
table is destroyed, or the VMA is destroyed. Attempting to connect it
to a process beyond tying the lifetime of a page table to a FD is
nonsensical.
Jason
next prev parent reply other threads:[~2024-04-05 0:39 UTC|newest]
Thread overview: 123+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-17 22:12 [PATCH 00/23] XeKmd basic SVM support Oak Zeng
2024-01-17 22:12 ` [PATCH 01/23] drm/xe/svm: Add SVM document Oak Zeng
2024-01-17 22:12 ` [PATCH 02/23] drm/xe/svm: Add svm key data structures Oak Zeng
2024-01-17 22:12 ` [PATCH 03/23] drm/xe/svm: create xe svm during vm creation Oak Zeng
2024-01-17 22:12 ` [PATCH 04/23] drm/xe/svm: Trace svm creation Oak Zeng
2024-01-17 22:12 ` [PATCH 05/23] drm/xe/svm: add helper to retrieve svm range from address Oak Zeng
2024-01-17 22:12 ` [PATCH 06/23] drm/xe/svm: Introduce a helper to build sg table from hmm range Oak Zeng
2024-04-05 0:39 ` Jason Gunthorpe [this message]
2024-04-05 3:33 ` Zeng, Oak
2024-04-05 12:37 ` Jason Gunthorpe
2024-04-05 16:42 ` Zeng, Oak
2024-04-05 18:02 ` Jason Gunthorpe
2024-04-09 16:45 ` Zeng, Oak
2024-04-09 17:24 ` Jason Gunthorpe
2024-04-23 21:17 ` Zeng, Oak
2024-04-24 2:31 ` Matthew Brost
2024-04-24 13:57 ` Jason Gunthorpe
2024-04-24 16:35 ` Matthew Brost
2024-04-24 16:44 ` Jason Gunthorpe
2024-04-24 16:56 ` Matthew Brost
2024-04-24 17:48 ` Jason Gunthorpe
2024-04-24 13:48 ` Jason Gunthorpe
2024-04-24 23:59 ` Zeng, Oak
2024-04-25 1:05 ` Jason Gunthorpe
2024-04-26 9:55 ` Thomas Hellström
2024-04-26 12:00 ` Jason Gunthorpe
2024-04-26 14:49 ` Thomas Hellström
2024-04-26 16:35 ` Jason Gunthorpe
2024-04-29 8:25 ` Thomas Hellström
2024-04-30 17:30 ` Jason Gunthorpe
2024-04-30 18:57 ` Daniel Vetter
2024-05-01 0:09 ` Jason Gunthorpe
2024-05-02 8:04 ` Daniel Vetter
2024-05-02 9:11 ` Thomas Hellström
2024-05-02 12:46 ` Jason Gunthorpe
2024-05-02 15:01 ` Thomas Hellström
2024-05-02 19:25 ` Zeng, Oak
2024-05-03 13:37 ` Jason Gunthorpe
2024-05-03 14:43 ` Zeng, Oak
2024-05-03 16:28 ` Jason Gunthorpe
2024-05-03 20:29 ` Zeng, Oak
2024-05-04 1:03 ` Dave Airlie
2024-05-06 13:04 ` Daniel Vetter
2024-05-06 23:50 ` Matthew Brost
2024-05-07 11:56 ` Jason Gunthorpe
2024-05-06 13:33 ` Jason Gunthorpe
2024-04-09 17:33 ` Matthew Brost
2024-01-17 22:12 ` [PATCH 07/23] drm/xe/svm: Add helper for binding hmm range to gpu Oak Zeng
2024-01-17 22:12 ` [PATCH 08/23] drm/xe/svm: Add helper to invalidate svm range from GPU Oak Zeng
2024-01-17 22:12 ` [PATCH 09/23] drm/xe/svm: Remap and provide memmap backing for GPU vram Oak Zeng
2024-01-17 22:12 ` [PATCH 10/23] drm/xe/svm: Introduce svm migration function Oak Zeng
2024-01-17 22:12 ` [PATCH 11/23] drm/xe/svm: implement functions to allocate and free device memory Oak Zeng
2024-01-17 22:12 ` [PATCH 12/23] drm/xe/svm: Trace buddy block allocation and free Oak Zeng
2024-01-17 22:12 ` [PATCH 13/23] drm/xe/svm: Handle CPU page fault Oak Zeng
2024-01-17 22:12 ` [PATCH 14/23] drm/xe/svm: trace svm range migration Oak Zeng
2024-01-17 22:12 ` [PATCH 15/23] drm/xe/svm: Implement functions to register and unregister mmu notifier Oak Zeng
2024-01-17 22:12 ` [PATCH 16/23] drm/xe/svm: Implement the mmu notifier range invalidate callback Oak Zeng
2024-01-17 22:12 ` [PATCH 17/23] drm/xe/svm: clean up svm range during process exit Oak Zeng
2024-01-17 22:12 ` [PATCH 18/23] drm/xe/svm: Move a few structures to xe_gt.h Oak Zeng
2024-01-17 22:12 ` [PATCH 19/23] drm/xe/svm: migrate svm range to vram Oak Zeng
2024-01-17 22:12 ` [PATCH 20/23] drm/xe/svm: Populate svm range Oak Zeng
2024-01-17 22:12 ` [PATCH 21/23] drm/xe/svm: GPU page fault support Oak Zeng
2024-01-23 2:06 ` Welty, Brian
2024-01-23 3:09 ` Zeng, Oak
2024-01-23 3:21 ` Making drm_gpuvm work across gpu devices Zeng, Oak
2024-01-23 11:13 ` Christian König
2024-01-23 19:37 ` Zeng, Oak
2024-01-23 20:17 ` Felix Kuehling
2024-01-25 1:39 ` Zeng, Oak
2024-01-23 23:56 ` Danilo Krummrich
2024-01-24 3:57 ` Zeng, Oak
2024-01-24 4:14 ` Zeng, Oak
2024-01-24 6:48 ` Christian König
2024-01-25 22:13 ` Danilo Krummrich
2024-01-24 8:33 ` Christian König
2024-01-25 1:17 ` Zeng, Oak
2024-01-25 1:25 ` David Airlie
2024-01-25 5:25 ` Zeng, Oak
2024-01-26 10:09 ` Christian König
2024-01-26 20:13 ` Zeng, Oak
2024-01-29 10:10 ` Christian König
2024-01-29 20:09 ` Zeng, Oak
2024-01-25 11:00 ` 回复：Making " 周春明(日月)
2024-01-25 17:00 ` Zeng, Oak
2024-01-25 17:15 ` Making " Felix Kuehling
2024-01-25 18:37 ` Zeng, Oak
2024-01-26 13:23 ` Christian König
2024-01-25 16:42 ` Zeng, Oak
2024-01-25 18:32 ` Daniel Vetter
2024-01-25 21:02 ` Zeng, Oak
2024-01-26 8:21 ` Thomas Hellström
2024-01-26 12:52 ` Christian König
2024-01-27 2:21 ` Zeng, Oak
2024-01-29 10:19 ` Christian König
2024-01-30 0:21 ` Zeng, Oak
2024-01-30 8:39 ` Christian König
2024-01-30 22:29 ` Zeng, Oak
2024-01-30 23:12 ` David Airlie
2024-01-31 9:15 ` Daniel Vetter
2024-01-31 20:17 ` Zeng, Oak
2024-01-31 20:59 ` Zeng, Oak
2024-02-01 8:52 ` Christian König
2024-02-29 18:22 ` Zeng, Oak
2024-03-08 4:43 ` Zeng, Oak
2024-03-08 10:07 ` Christian König
2024-01-30 8:43 ` Thomas Hellström
2024-01-29 15:03 ` Felix Kuehling
2024-01-29 15:33 ` Christian König
2024-01-29 16:24 ` Felix Kuehling
2024-01-29 16:28 ` Christian König
2024-01-29 17:52 ` Felix Kuehling
2024-01-29 19:03 ` Christian König
2024-01-29 20:24 ` Felix Kuehling
2024-02-23 20:12 ` Zeng, Oak
2024-02-27 6:54 ` Christian König
2024-02-27 15:58 ` Zeng, Oak
2024-02-28 19:51 ` Zeng, Oak
2024-02-29 9:41 ` Christian König
2024-02-29 16:05 ` Zeng, Oak
2024-02-29 17:12 ` Thomas Hellström
2024-03-01 7:01 ` Christian König
2024-01-17 22:12 ` [PATCH 22/23] drm/xe/svm: Add DRM_XE_SVM kernel config entry Oak Zeng
2024-01-17 22:12 ` [PATCH 23/23] drm/xe/svm: Add svm memory hints interface Oak Zeng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240405003927.GA11940@nvidia.com \
--to=jgg@nvidia.com \
--cc=Thomas.Hellstrom@linux.intel.com \
--cc=brian.welty@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=himal.prasad.ghimiray@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=krishnaiah.bommu@intel.com \
--cc=leon@kernel.org \
--cc=matthew.brost@intel.com \
--cc=niranjana.vishwanathapura@intel.com \
--cc=oak.zeng@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).