Re: [Nouveau] [PATCH v2 8/8] hmm-tests: Add test for migrate_device_range()
Andrew Morton writes:
> On Wed, 28 Sep 2022 22:01:22 +1000 Alistair Popple wrote:
>
>> @@ -1401,22 +1494,7 @@ static int dmirror_device_init(struct dmirror_device
>> *mdevice, int id)
>>
>> static void dmirror_device_remove(struct dmirror_device *mdevice)
>> {
>> -unsigned int i;
>> -
>> -if (mdevice->devmem_chunks) {
>> -for (i = 0; i < mdevice->devmem_count; i++) {
>> -struct dmirror_chunk *devmem =
>> -mdevice->devmem_chunks[i];
>> -
>> -memunmap_pages(>pagemap);
>> -if (devmem->pagemap.type == MEMORY_DEVICE_PRIVATE)
>> -release_mem_region(devmem->pagemap.range.start,
>> -
>> range_len(>pagemap.range));
>> -kfree(devmem);
>> -}
>> -kfree(mdevice->devmem_chunks);
>> -}
>> -
>> +dmirror_device_remove_chunks(mdevice);
>> cdev_del(>cdevice);
>> }
>
> Needed a bit or rework due to
> https://lkml.kernel.org/r/20220826050631.25771-1-mpent...@redhat.com.
> Please check my resolution.
Thanks. Rework looks good to me.
> --- a/lib/test_hmm.c~hmm-tests-add-test-for-migrate_device_range
> +++ a/lib/test_hmm.c
> @@ -100,6 +100,7 @@ struct dmirror {
> struct dmirror_chunk {
> struct dev_pagemap pagemap;
> struct dmirror_device *mdevice;
> + bool remove;
> };
>
> /*
> @@ -192,11 +193,15 @@ static int dmirror_fops_release(struct i
> return 0;
> }
>
> +static struct dmirror_chunk *dmirror_page_to_chunk(struct page *page)
> +{
> + return container_of(page->pgmap, struct dmirror_chunk, pagemap);
> +}
> +
> static struct dmirror_device *dmirror_page_to_device(struct page *page)
>
> {
> - return container_of(page->pgmap, struct dmirror_chunk,
> - pagemap)->mdevice;
> + return dmirror_page_to_chunk(page)->mdevice;
> }
>
> static int dmirror_do_fault(struct dmirror *dmirror, struct hmm_range *range)
> @@ -1218,6 +1223,85 @@ static int dmirror_snapshot(struct dmirr
> return ret;
> }
>
> +static void dmirror_device_evict_chunk(struct dmirror_chunk *chunk)
> +{
> + unsigned long start_pfn = chunk->pagemap.range.start >> PAGE_SHIFT;
> + unsigned long end_pfn = chunk->pagemap.range.end >> PAGE_SHIFT;
> + unsigned long npages = end_pfn - start_pfn + 1;
> + unsigned long i;
> + unsigned long *src_pfns;
> + unsigned long *dst_pfns;
> +
> + src_pfns = kcalloc(npages, sizeof(*src_pfns), GFP_KERNEL);
> + dst_pfns = kcalloc(npages, sizeof(*dst_pfns), GFP_KERNEL);
> +
> + migrate_device_range(src_pfns, start_pfn, npages);
> + for (i = 0; i < npages; i++) {
> + struct page *dpage, *spage;
> +
> + spage = migrate_pfn_to_page(src_pfns[i]);
> + if (!spage || !(src_pfns[i] & MIGRATE_PFN_MIGRATE))
> + continue;
> +
> + if (WARN_ON(!is_device_private_page(spage) &&
> + !is_device_coherent_page(spage)))
> + continue;
> + spage = BACKING_PAGE(spage);
> + dpage = alloc_page(GFP_HIGHUSER_MOVABLE | __GFP_NOFAIL);
> + lock_page(dpage);
> + copy_highpage(dpage, spage);
> + dst_pfns[i] = migrate_pfn(page_to_pfn(dpage));
> + if (src_pfns[i] & MIGRATE_PFN_WRITE)
> + dst_pfns[i] |= MIGRATE_PFN_WRITE;
> + }
> + migrate_device_pages(src_pfns, dst_pfns, npages);
> + migrate_device_finalize(src_pfns, dst_pfns, npages);
> + kfree(src_pfns);
> + kfree(dst_pfns);
> +}
> +
> +/* Removes free pages from the free list so they can't be re-allocated */
> +static void dmirror_remove_free_pages(struct dmirror_chunk *devmem)
> +{
> + struct dmirror_device *mdevice = devmem->mdevice;
> + struct page *page;
> +
> + for (page = mdevice->free_pages; page; page = page->zone_device_data)
> + if (dmirror_page_to_chunk(page) == devmem)
> + mdevice->free_pages = page->zone_device_data;
> +}
> +
> +static void dmirror_device_remove_chunks(struct dmirror_device *mdevice)
> +{
> + unsigned int i;
> +
> + mutex_lock(>devmem_lock);
> + if (mdevice->devmem_chunks) {
> + for (i = 0; i < mdevice->devmem_count; i++) {
> + struct dmirror_chunk *devmem =
> + mdevice->devmem_chunks[i];
> +
> + spin_lock(>lock);
> + devmem->remove = true;
> + dmirror_remove_free_pages(devmem);
> + spin_unlock(>lock);
> +
> + dmirror_device_evict_chunk(devmem);
> + memunmap_pages(>pagemap);
> + if (devmem->pagemap.type == MEMORY_DEVICE_PRIVATE)
> + release_mem_region(devmem->pagemap.range.start,
> +
>
Re: [Nouveau] [PATCH v2 8/8] hmm-tests: Add test for migrate_device_range()
On Wed, 28 Sep 2022 22:01:22 +1000 Alistair Popple wrote:
> @@ -1401,22 +1494,7 @@ static int dmirror_device_init(struct dmirror_device
> *mdevice, int id)
>
> static void dmirror_device_remove(struct dmirror_device *mdevice)
> {
> - unsigned int i;
> -
> - if (mdevice->devmem_chunks) {
> - for (i = 0; i < mdevice->devmem_count; i++) {
> - struct dmirror_chunk *devmem =
> - mdevice->devmem_chunks[i];
> -
> - memunmap_pages(>pagemap);
> - if (devmem->pagemap.type == MEMORY_DEVICE_PRIVATE)
> - release_mem_region(devmem->pagemap.range.start,
> -
> range_len(>pagemap.range));
> - kfree(devmem);
> - }
> - kfree(mdevice->devmem_chunks);
> - }
> -
> + dmirror_device_remove_chunks(mdevice);
> cdev_del(>cdevice);
> }
Needed a bit or rework due to
https://lkml.kernel.org/r/20220826050631.25771-1-mpent...@redhat.com.
Please check my resolution.
--- a/lib/test_hmm.c~hmm-tests-add-test-for-migrate_device_range
+++ a/lib/test_hmm.c
@@ -100,6 +100,7 @@ struct dmirror {
struct dmirror_chunk {
struct dev_pagemap pagemap;
struct dmirror_device *mdevice;
+ bool remove;
};
/*
@@ -192,11 +193,15 @@ static int dmirror_fops_release(struct i
return 0;
}
+static struct dmirror_chunk *dmirror_page_to_chunk(struct page *page)
+{
+ return container_of(page->pgmap, struct dmirror_chunk, pagemap);
+}
+
static struct dmirror_device *dmirror_page_to_device(struct page *page)
{
- return container_of(page->pgmap, struct dmirror_chunk,
- pagemap)->mdevice;
+ return dmirror_page_to_chunk(page)->mdevice;
}
static int dmirror_do_fault(struct dmirror *dmirror, struct hmm_range *range)
@@ -1218,6 +1223,85 @@ static int dmirror_snapshot(struct dmirr
return ret;
}
+static void dmirror_device_evict_chunk(struct dmirror_chunk *chunk)
+{
+ unsigned long start_pfn = chunk->pagemap.range.start >> PAGE_SHIFT;
+ unsigned long end_pfn = chunk->pagemap.range.end >> PAGE_SHIFT;
+ unsigned long npages = end_pfn - start_pfn + 1;
+ unsigned long i;
+ unsigned long *src_pfns;
+ unsigned long *dst_pfns;
+
+ src_pfns = kcalloc(npages, sizeof(*src_pfns), GFP_KERNEL);
+ dst_pfns = kcalloc(npages, sizeof(*dst_pfns), GFP_KERNEL);
+
+ migrate_device_range(src_pfns, start_pfn, npages);
+ for (i = 0; i < npages; i++) {
+ struct page *dpage, *spage;
+
+ spage = migrate_pfn_to_page(src_pfns[i]);
+ if (!spage || !(src_pfns[i] & MIGRATE_PFN_MIGRATE))
+ continue;
+
+ if (WARN_ON(!is_device_private_page(spage) &&
+ !is_device_coherent_page(spage)))
+ continue;
+ spage = BACKING_PAGE(spage);
+ dpage = alloc_page(GFP_HIGHUSER_MOVABLE | __GFP_NOFAIL);
+ lock_page(dpage);
+ copy_highpage(dpage, spage);
+ dst_pfns[i] = migrate_pfn(page_to_pfn(dpage));
+ if (src_pfns[i] & MIGRATE_PFN_WRITE)
+ dst_pfns[i] |= MIGRATE_PFN_WRITE;
+ }
+ migrate_device_pages(src_pfns, dst_pfns, npages);
+ migrate_device_finalize(src_pfns, dst_pfns, npages);
+ kfree(src_pfns);
+ kfree(dst_pfns);
+}
+
+/* Removes free pages from the free list so they can't be re-allocated */
+static void dmirror_remove_free_pages(struct dmirror_chunk *devmem)
+{
+ struct dmirror_device *mdevice = devmem->mdevice;
+ struct page *page;
+
+ for (page = mdevice->free_pages; page; page = page->zone_device_data)
+ if (dmirror_page_to_chunk(page) == devmem)
+ mdevice->free_pages = page->zone_device_data;
+}
+
+static void dmirror_device_remove_chunks(struct dmirror_device *mdevice)
+{
+ unsigned int i;
+
+ mutex_lock(>devmem_lock);
+ if (mdevice->devmem_chunks) {
+ for (i = 0; i < mdevice->devmem_count; i++) {
+ struct dmirror_chunk *devmem =
+ mdevice->devmem_chunks[i];
+
+ spin_lock(>lock);
+ devmem->remove = true;
+ dmirror_remove_free_pages(devmem);
+ spin_unlock(>lock);
+
+ dmirror_device_evict_chunk(devmem);
+ memunmap_pages(>pagemap);
+ if (devmem->pagemap.type == MEMORY_DEVICE_PRIVATE)
+ release_mem_region(devmem->pagemap.range.start,
+
range_len(>pagemap.range));
+ kfree(devmem);
+ }
+ mdevice->devmem_count = 0;
+
[Nouveau] [PATCH v2 8/8] hmm-tests: Add test for migrate_device_range()
Signed-off-by: Alistair Popple
Cc: Jason Gunthorpe
Cc: Ralph Campbell
Cc: John Hubbard
Cc: Alex Sierra
Cc: Felix Kuehling
---
lib/test_hmm.c | 120 +-
lib/test_hmm_uapi.h| 1 +-
tools/testing/selftests/vm/hmm-tests.c | 49 +++-
3 files changed, 149 insertions(+), 21 deletions(-)
diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index 688c15d..6c2fc85 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -100,6 +100,7 @@ struct dmirror {
struct dmirror_chunk {
struct dev_pagemap pagemap;
struct dmirror_device *mdevice;
+ bool remove;
};
/*
@@ -192,11 +193,15 @@ static int dmirror_fops_release(struct inode *inode,
struct file *filp)
return 0;
}
+static struct dmirror_chunk *dmirror_page_to_chunk(struct page *page)
+{
+ return container_of(page->pgmap, struct dmirror_chunk, pagemap);
+}
+
static struct dmirror_device *dmirror_page_to_device(struct page *page)
{
- return container_of(page->pgmap, struct dmirror_chunk,
- pagemap)->mdevice;
+ return dmirror_page_to_chunk(page)->mdevice;
}
static int dmirror_do_fault(struct dmirror *dmirror, struct hmm_range *range)
@@ -1218,6 +1223,85 @@ static int dmirror_snapshot(struct dmirror *dmirror,
return ret;
}
+static void dmirror_device_evict_chunk(struct dmirror_chunk *chunk)
+{
+ unsigned long start_pfn = chunk->pagemap.range.start >> PAGE_SHIFT;
+ unsigned long end_pfn = chunk->pagemap.range.end >> PAGE_SHIFT;
+ unsigned long npages = end_pfn - start_pfn + 1;
+ unsigned long i;
+ unsigned long *src_pfns;
+ unsigned long *dst_pfns;
+
+ src_pfns = kcalloc(npages, sizeof(*src_pfns), GFP_KERNEL);
+ dst_pfns = kcalloc(npages, sizeof(*dst_pfns), GFP_KERNEL);
+
+ migrate_device_range(src_pfns, start_pfn, npages);
+ for (i = 0; i < npages; i++) {
+ struct page *dpage, *spage;
+
+ spage = migrate_pfn_to_page(src_pfns[i]);
+ if (!spage || !(src_pfns[i] & MIGRATE_PFN_MIGRATE))
+ continue;
+
+ if (WARN_ON(!is_device_private_page(spage) &&
+ !is_device_coherent_page(spage)))
+ continue;
+ spage = BACKING_PAGE(spage);
+ dpage = alloc_page(GFP_HIGHUSER_MOVABLE | __GFP_NOFAIL);
+ lock_page(dpage);
+ copy_highpage(dpage, spage);
+ dst_pfns[i] = migrate_pfn(page_to_pfn(dpage));
+ if (src_pfns[i] & MIGRATE_PFN_WRITE)
+ dst_pfns[i] |= MIGRATE_PFN_WRITE;
+ }
+ migrate_device_pages(src_pfns, dst_pfns, npages);
+ migrate_device_finalize(src_pfns, dst_pfns, npages);
+ kfree(src_pfns);
+ kfree(dst_pfns);
+}
+
+/* Removes free pages from the free list so they can't be re-allocated */
+static void dmirror_remove_free_pages(struct dmirror_chunk *devmem)
+{
+ struct dmirror_device *mdevice = devmem->mdevice;
+ struct page *page;
+
+ for (page = mdevice->free_pages; page; page = page->zone_device_data)
+ if (dmirror_page_to_chunk(page) == devmem)
+ mdevice->free_pages = page->zone_device_data;
+}
+
+static void dmirror_device_remove_chunks(struct dmirror_device *mdevice)
+{
+ unsigned int i;
+
+ mutex_lock(>devmem_lock);
+ if (mdevice->devmem_chunks) {
+ for (i = 0; i < mdevice->devmem_count; i++) {
+ struct dmirror_chunk *devmem =
+ mdevice->devmem_chunks[i];
+
+ spin_lock(>lock);
+ devmem->remove = true;
+ dmirror_remove_free_pages(devmem);
+ spin_unlock(>lock);
+
+ dmirror_device_evict_chunk(devmem);
+ memunmap_pages(>pagemap);
+ if (devmem->pagemap.type == MEMORY_DEVICE_PRIVATE)
+ release_mem_region(devmem->pagemap.range.start,
+
range_len(>pagemap.range));
+ kfree(devmem);
+ }
+ mdevice->devmem_count = 0;
+ mdevice->devmem_capacity = 0;
+ mdevice->free_pages = NULL;
+ kfree(mdevice->devmem_chunks);
+ mdevice->devmem_chunks = NULL;
+ }
+ mutex_unlock(>devmem_lock);
+}
+
static long dmirror_fops_unlocked_ioctl(struct file *filp,
unsigned int command,
unsigned long arg)
@@ -1272,6 +1356,11 @@ static long dmirror_fops_unlocked_ioctl(struct file
*filp,
ret = dmirror_snapshot(dmirror, );
break;
+ case HMM_DMIRROR_RELEASE:
+ dmirror_device_remove_chunks(dmirror->mdevice);
+
