* [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio()
@ 2024-02-16 21:13 Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 2/6] mm/migrate_device: further convert migrate_device_unmap() to folios Sidhartha Kumar
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Sidhartha Kumar @ 2024-02-16 21:13 UTC (permalink / raw
To: linux-kernel, linux-mm; +Cc: akpm, willy, apopple, Sidhartha Kumar
Add a folio compatible wrapper for migrate_pfn_to_page() so we can
return a folio directly.
Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
Suggested-by: Alistair Popple <apopple@nvidia.com>
---
include/linux/migrate.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 2ce13e8a309bd..21a1a5e415338 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -171,6 +171,11 @@ static inline struct page *migrate_pfn_to_page(unsigned long mpfn)
return pfn_to_page(mpfn >> MIGRATE_PFN_SHIFT);
}
+static inline struct folio *migrate_pfn_to_folio(unsigned long mpfn)
+{
+ return page_folio(migrate_pfn_to_page(mpfn));
+}
+
static inline unsigned long migrate_pfn(unsigned long pfn)
{
return (pfn << MIGRATE_PFN_SHIFT) | MIGRATE_PFN_VALID;
--
2.42.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/6] mm/migrate_device: further convert migrate_device_unmap() to folios
2024-02-16 21:13 [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Sidhartha Kumar
@ 2024-02-16 21:13 ` Sidhartha Kumar
2024-02-16 21:51 ` Matthew Wilcox
2024-02-16 21:13 ` [PATCH v2 3/6] mm/migrate_device: further convert migrate_device_finalize() " Sidhartha Kumar
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Sidhartha Kumar @ 2024-02-16 21:13 UTC (permalink / raw
To: linux-kernel, linux-mm; +Cc: akpm, willy, apopple, Sidhartha Kumar
migrate_device_unmap() already has a folio, we can use the folio
versions of is_zone_device_page() and putback_lru_page.
Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
---
v1 -> v2:
use migrate_pfn_to_folio() to directly work with a folio
per Alistair Popple.
mm/migrate_device.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index b6c27c76e1a0b..3d3c2593b5b64 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -368,42 +368,40 @@ static unsigned long migrate_device_unmap(unsigned long *src_pfns,
lru_add_drain();
for (i = 0; i < npages; i++) {
- struct page *page = migrate_pfn_to_page(src_pfns[i]);
- struct folio *folio;
+ struct folio *folio = migrate_pfn_to_folio(src_pfns[i]);
- if (!page) {
+ if (!folio) {
if (src_pfns[i] & MIGRATE_PFN_MIGRATE)
unmapped++;
continue;
}
/* ZONE_DEVICE pages are not on LRU */
- if (!is_zone_device_page(page)) {
- if (!PageLRU(page) && allow_drain) {
+ if (!folio_is_zone_device(folio)) {
+ if (!folio_test_lru(folio) && allow_drain) {
/* Drain CPU's lru cache */
lru_add_drain_all();
allow_drain = false;
}
- if (!isolate_lru_page(page)) {
+ if (!folio_isolate_lru(folio)) {
src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
restore++;
continue;
}
/* Drop the reference we took in collect */
- put_page(page);
+ folio_put(folio);
}
- folio = page_folio(page);
if (folio_mapped(folio))
try_to_migrate(folio, 0);
- if (page_mapped(page) ||
- !migrate_vma_check_page(page, fault_page)) {
- if (!is_zone_device_page(page)) {
- get_page(page);
- putback_lru_page(page);
+ if (folio_mapped(folio) ||
+ !migrate_vma_check_page(&folio->page, fault_page)) {
+ if (!folio_is_zone_device(folio)) {
+ folio_get(folio);
+ folio_putback_lru(folio);
}
src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
@@ -415,13 +413,11 @@ static unsigned long migrate_device_unmap(unsigned long *src_pfns,
}
for (i = 0; i < npages && restore; i++) {
- struct page *page = migrate_pfn_to_page(src_pfns[i]);
- struct folio *folio;
+ struct folio *folio = migrate_pfn_to_folio(src_pfns[i]);
- if (!page || (src_pfns[i] & MIGRATE_PFN_MIGRATE))
+ if (!folio || (src_pfns[i] & MIGRATE_PFN_MIGRATE))
continue;
- folio = page_folio(page);
remove_migration_ptes(folio, folio, false);
src_pfns[i] = 0;
--
2.42.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/6] mm/migrate_device: further convert migrate_device_finalize() to folios
2024-02-16 21:13 [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 2/6] mm/migrate_device: further convert migrate_device_unmap() to folios Sidhartha Kumar
@ 2024-02-16 21:13 ` Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() " Sidhartha Kumar
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Sidhartha Kumar @ 2024-02-16 21:13 UTC (permalink / raw
To: linux-kernel, linux-mm; +Cc: akpm, willy, apopple, Sidhartha Kumar
Use folio api functions from the already defined src and dst folio
variables.
Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
---
v1 -> v2:
use migrate_pfn_to_folio() to directly work with a folio
per Alistair Popple.
mm/migrate_device.c | 41 +++++++++++++++++++----------------------
1 file changed, 19 insertions(+), 22 deletions(-)
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 3d3c2593b5b64..81623f2d72c2b 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -814,42 +814,39 @@ void migrate_device_finalize(unsigned long *src_pfns,
unsigned long i;
for (i = 0; i < npages; i++) {
- struct folio *dst, *src;
- struct page *newpage = migrate_pfn_to_page(dst_pfns[i]);
- struct page *page = migrate_pfn_to_page(src_pfns[i]);
+ struct folio *dst = migrate_pfn_to_folio(dst_pfns[i]);
+ struct folio *src = migrate_pfn_to_folio(src_pfns[i]);
- if (!page) {
- if (newpage) {
- unlock_page(newpage);
- put_page(newpage);
+ if (!src) {
+ if (dst) {
+ folio_unlock(dst);
+ folio_put(dst);
}
continue;
}
- if (!(src_pfns[i] & MIGRATE_PFN_MIGRATE) || !newpage) {
- if (newpage) {
- unlock_page(newpage);
- put_page(newpage);
+ if (!(src_pfns[i] & MIGRATE_PFN_MIGRATE) || !dst) {
+ if (dst) {
+ folio_unlock(dst);
+ folio_put(dst);
}
- newpage = page;
+ dst = src;
}
- src = page_folio(page);
- dst = page_folio(newpage);
remove_migration_ptes(src, dst, false);
folio_unlock(src);
- if (is_zone_device_page(page))
- put_page(page);
+ if (folio_is_zone_device(src))
+ folio_put(src);
else
- putback_lru_page(page);
+ folio_putback_lru(src);
- if (newpage != page) {
- unlock_page(newpage);
- if (is_zone_device_page(newpage))
- put_page(newpage);
+ if (dst != src) {
+ folio_unlock(dst);
+ if (folio_is_zone_device(dst))
+ folio_put(dst);
else
- putback_lru_page(newpage);
+ folio_putback_lru(dst);
}
}
}
--
2.42.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() to folios
2024-02-16 21:13 [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 2/6] mm/migrate_device: further convert migrate_device_unmap() to folios Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 3/6] mm/migrate_device: further convert migrate_device_finalize() " Sidhartha Kumar
@ 2024-02-16 21:13 ` Sidhartha Kumar
2024-02-16 21:55 ` Matthew Wilcox
2024-02-16 21:13 ` [PATCH v2 5/6] mm/migrate_device: convert migrate_device_coherent_page() to migrate_device_coherent_folio() Sidhartha Kumar
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Sidhartha Kumar @ 2024-02-16 21:13 UTC (permalink / raw
To: linux-kernel, linux-mm; +Cc: akpm, willy, apopple, Sidhartha Kumar
Use migrate_pfn_to_folio() so we can work with folios directly in
__migrate_device_pages().
Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
---
mm/migrate_device.c | 36 +++++++++++++++---------------------
1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 81623f2d72c2b..d9633c7b1d21b 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -687,17 +687,17 @@ static void __migrate_device_pages(unsigned long *src_pfns,
bool notified = false;
for (i = 0; i < npages; i++) {
- struct page *newpage = migrate_pfn_to_page(dst_pfns[i]);
- struct page *page = migrate_pfn_to_page(src_pfns[i]);
+ struct folio *dst = migrate_pfn_to_folio(dst_pfns[i]);
+ struct folio *src = migrate_pfn_to_folio(src_pfns[i]);
struct address_space *mapping;
int r;
- if (!newpage) {
+ if (!dst) {
src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
continue;
}
- if (!page) {
+ if (!src) {
unsigned long addr;
if (!(src_pfns[i] & MIGRATE_PFN_MIGRATE))
@@ -719,33 +719,29 @@ static void __migrate_device_pages(unsigned long *src_pfns,
migrate->pgmap_owner);
mmu_notifier_invalidate_range_start(&range);
}
- migrate_vma_insert_page(migrate, addr, newpage,
+ migrate_vma_insert_page(migrate, addr, &dst->page,
&src_pfns[i]);
continue;
}
- mapping = page_mapping(page);
+ mapping = folio_mapping(src);
- if (is_device_private_page(newpage) ||
- is_device_coherent_page(newpage)) {
+ if (folio_is_device_private(dst) ||
+ folio_is_device_coherent(dst)) {
if (mapping) {
- struct folio *folio;
-
- folio = page_folio(page);
-
/*
* For now only support anonymous memory migrating to
* device private or coherent memory.
*
* Try to get rid of swap cache if possible.
*/
- if (!folio_test_anon(folio) ||
- !folio_free_swap(folio)) {
+ if (!folio_test_anon(src) ||
+ !folio_free_swap(src)) {
src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
continue;
}
}
- } else if (is_zone_device_page(newpage)) {
+ } else if (folio_is_zone_device(dst)) {
/*
* Other types of ZONE_DEVICE page are not supported.
*/
@@ -753,13 +749,11 @@ static void __migrate_device_pages(unsigned long *src_pfns,
continue;
}
- if (migrate && migrate->fault_page == page)
- r = migrate_folio_extra(mapping, page_folio(newpage),
- page_folio(page),
- MIGRATE_SYNC_NO_COPY, 1);
+ if (migrate && migrate->fault_page == &src->page)
+ r = migrate_folio_extra(mapping, dst, src,
+ MIGRATE_SYNC_NO_COPY, 1);
else
- r = migrate_folio(mapping, page_folio(newpage),
- page_folio(page), MIGRATE_SYNC_NO_COPY);
+ r = migrate_folio(mapping, dst, src, MIGRATE_SYNC_NO_COPY);
if (r != MIGRATEPAGE_SUCCESS)
src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/6] mm/migrate_device: convert migrate_device_coherent_page() to migrate_device_coherent_folio()
2024-02-16 21:13 [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Sidhartha Kumar
` (2 preceding siblings ...)
2024-02-16 21:13 ` [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() " Sidhartha Kumar
@ 2024-02-16 21:13 ` Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 6/6] mm/migrate_device: convert migrate_device_range() to folios Sidhartha Kumar
2024-02-16 21:50 ` [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Matthew Wilcox
5 siblings, 0 replies; 13+ messages in thread
From: Sidhartha Kumar @ 2024-02-16 21:13 UTC (permalink / raw
To: linux-kernel, linux-mm; +Cc: akpm, willy, apopple, Sidhartha Kumar
Rename migrate_device_coherent_page() to migrate_device_coherent_folio()
as it now takes in a folio.
Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
---
mm/gup.c | 2 +-
mm/internal.h | 2 +-
mm/migrate_device.c | 12 ++++++------
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index df83182ec72d5..2c8b183b94485 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2109,7 +2109,7 @@ static int migrate_longterm_unpinnable_pages(
folio_get(folio);
gup_put_folio(folio, 1, FOLL_PIN);
- if (migrate_device_coherent_page(&folio->page)) {
+ if (migrate_device_coherent_folio(folio)) {
ret = -EBUSY;
goto err;
}
diff --git a/mm/internal.h b/mm/internal.h
index c6ea449c5353c..32304bec79f3f 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1003,7 +1003,7 @@ int numa_migrate_prep(struct folio *folio, struct vm_area_struct *vma,
unsigned long addr, int page_nid, int *flags);
void free_zone_device_page(struct page *page);
-int migrate_device_coherent_page(struct page *page);
+int migrate_device_coherent_folio(struct folio *folio);
/*
* mm/gup.c
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index d9633c7b1d21b..5c239de0f08b2 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -705,7 +705,7 @@ static void __migrate_device_pages(unsigned long *src_pfns,
/*
* The only time there is no vma is when called from
- * migrate_device_coherent_page(). However this isn't
+ * migrate_device_coherent_folio(). However this isn't
* called if the page could not be unmapped.
*/
VM_BUG_ON(!migrate);
@@ -915,15 +915,15 @@ EXPORT_SYMBOL(migrate_device_range);
* a reference on page which will be copied to the new page if migration is
* successful or dropped on failure.
*/
-int migrate_device_coherent_page(struct page *page)
+int migrate_device_coherent_folio(struct folio *folio)
{
unsigned long src_pfn, dst_pfn = 0;
struct page *dpage;
- WARN_ON_ONCE(PageCompound(page));
+ WARN_ON_ONCE(folio_test_large(folio));
- lock_page(page);
- src_pfn = migrate_pfn(page_to_pfn(page)) | MIGRATE_PFN_MIGRATE;
+ folio_lock(folio);
+ src_pfn = migrate_pfn(folio_pfn(folio)) | MIGRATE_PFN_MIGRATE;
/*
* We don't have a VMA and don't need to walk the page tables to find
@@ -942,7 +942,7 @@ int migrate_device_coherent_page(struct page *page)
migrate_device_pages(&src_pfn, &dst_pfn, 1);
if (src_pfn & MIGRATE_PFN_MIGRATE)
- copy_highpage(dpage, page);
+ copy_highpage(dpage, &folio->page);
migrate_device_finalize(&src_pfn, &dst_pfn, 1);
if (src_pfn & MIGRATE_PFN_MIGRATE)
--
2.42.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 6/6] mm/migrate_device: convert migrate_device_range() to folios
2024-02-16 21:13 [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Sidhartha Kumar
` (3 preceding siblings ...)
2024-02-16 21:13 ` [PATCH v2 5/6] mm/migrate_device: convert migrate_device_coherent_page() to migrate_device_coherent_folio() Sidhartha Kumar
@ 2024-02-16 21:13 ` Sidhartha Kumar
2024-02-16 21:50 ` [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Matthew Wilcox
5 siblings, 0 replies; 13+ messages in thread
From: Sidhartha Kumar @ 2024-02-16 21:13 UTC (permalink / raw
To: linux-kernel, linux-mm; +Cc: akpm, willy, apopple, Sidhartha Kumar
Use pfn_folio() to get a folio and use the folio equivalent page
functions throughout migrate_device_range().
Signed-off-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
---
mm/migrate_device.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/migrate_device.c b/mm/migrate_device.c
index 5c239de0f08b2..0c8d86851e631 100644
--- a/mm/migrate_device.c
+++ b/mm/migrate_device.c
@@ -888,16 +888,16 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
unsigned long i, pfn;
for (pfn = start, i = 0; i < npages; pfn++, i++) {
- struct page *page = pfn_to_page(pfn);
+ struct folio *folio = pfn_folio(pfn);
- if (!get_page_unless_zero(page)) {
+ if (!folio_try_get(folio)) {
src_pfns[i] = 0;
continue;
}
- if (!trylock_page(page)) {
+ if (!folio_trylock(folio)) {
src_pfns[i] = 0;
- put_page(page);
+ folio_put(folio);
continue;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio()
2024-02-16 21:13 [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Sidhartha Kumar
` (4 preceding siblings ...)
2024-02-16 21:13 ` [PATCH v2 6/6] mm/migrate_device: convert migrate_device_range() to folios Sidhartha Kumar
@ 2024-02-16 21:50 ` Matthew Wilcox
2024-02-16 22:03 ` Sidhartha Kumar
5 siblings, 1 reply; 13+ messages in thread
From: Matthew Wilcox @ 2024-02-16 21:50 UTC (permalink / raw
To: Sidhartha Kumar; +Cc: linux-kernel, linux-mm, akpm, apopple
On Fri, Feb 16, 2024 at 01:13:15PM -0800, Sidhartha Kumar wrote:
> +static inline struct folio *migrate_pfn_to_folio(unsigned long mpfn)
> +{
> + return page_folio(migrate_pfn_to_page(mpfn));
umm, no.
struct page *page = migrate_pfn_to_page(mpfn);
if (page)
return page_folio(page);
return NULL;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/6] mm/migrate_device: further convert migrate_device_unmap() to folios
2024-02-16 21:13 ` [PATCH v2 2/6] mm/migrate_device: further convert migrate_device_unmap() to folios Sidhartha Kumar
@ 2024-02-16 21:51 ` Matthew Wilcox
0 siblings, 0 replies; 13+ messages in thread
From: Matthew Wilcox @ 2024-02-16 21:51 UTC (permalink / raw
To: Sidhartha Kumar; +Cc: linux-kernel, linux-mm, akpm, apopple
On Fri, Feb 16, 2024 at 01:13:16PM -0800, Sidhartha Kumar wrote:
> migrate_device_unmap() already has a folio, we can use the folio
> versions of is_zone_device_page() and putback_lru_page.
> - if (page_mapped(page) ||
> - !migrate_vma_check_page(page, fault_page)) {
> - if (!is_zone_device_page(page)) {
> - get_page(page);
> - putback_lru_page(page);
> + if (folio_mapped(folio) ||
> + !migrate_vma_check_page(&folio->page, fault_page)) {
> + if (!folio_is_zone_device(folio)) {
> + folio_get(folio);
> + folio_putback_lru(folio);
page_mapped() is not the same thing as folio_mapped(page_folio(page)).
please include some argumentation about why this is a correct
transformation to make.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() to folios
2024-02-16 21:13 ` [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() " Sidhartha Kumar
@ 2024-02-16 21:55 ` Matthew Wilcox
2024-02-16 22:00 ` Sidhartha Kumar
0 siblings, 1 reply; 13+ messages in thread
From: Matthew Wilcox @ 2024-02-16 21:55 UTC (permalink / raw
To: Sidhartha Kumar; +Cc: linux-kernel, linux-mm, akpm, apopple
On Fri, Feb 16, 2024 at 01:13:18PM -0800, Sidhartha Kumar wrote:
> Use migrate_pfn_to_folio() so we can work with folios directly in
> __migrate_device_pages().
i don't understand why this would be correct if we have multipage
folios.
> @@ -719,33 +719,29 @@ static void __migrate_device_pages(unsigned long *src_pfns,
> migrate->pgmap_owner);
> mmu_notifier_invalidate_range_start(&range);
> }
> - migrate_vma_insert_page(migrate, addr, newpage,
> + migrate_vma_insert_page(migrate, addr, &dst->page,
seems to me that a migration pfn is going to refer to a precise page.
now you're telling it to insert the head page of the folio. isn't this
wrong?
> @@ -753,13 +749,11 @@ static void __migrate_device_pages(unsigned long *src_pfns,
> continue;
> }
>
> - if (migrate && migrate->fault_page == page)
> - r = migrate_folio_extra(mapping, page_folio(newpage),
> - page_folio(page),
> - MIGRATE_SYNC_NO_COPY, 1);
> + if (migrate && migrate->fault_page == &src->page)
shouldn't this rather be "page_folio(migrate->fault_page) == src"?
ie we're looking for two pages from the same folio, rather than the page
being the same as the head page of the folio?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() to folios
2024-02-16 21:55 ` Matthew Wilcox
@ 2024-02-16 22:00 ` Sidhartha Kumar
2024-02-16 23:19 ` Matthew Wilcox
0 siblings, 1 reply; 13+ messages in thread
From: Sidhartha Kumar @ 2024-02-16 22:00 UTC (permalink / raw
To: Matthew Wilcox; +Cc: linux-kernel, linux-mm, akpm, apopple
On 2/16/24 1:55 PM, Matthew Wilcox wrote:
> On Fri, Feb 16, 2024 at 01:13:18PM -0800, Sidhartha Kumar wrote:
>> Use migrate_pfn_to_folio() so we can work with folios directly in
>> __migrate_device_pages().
>
> i don't understand why this would be correct if we have multipage
> folios.
>
Alistair mentioned that he is working on order > 0 device page support so I was
under the impression that currently device pages are only order 0.
Thanks,
Sid
>> @@ -719,33 +719,29 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>> migrate->pgmap_owner);
>> mmu_notifier_invalidate_range_start(&range);
>> }
>> - migrate_vma_insert_page(migrate, addr, newpage,
>> + migrate_vma_insert_page(migrate, addr, &dst->page,
>
> seems to me that a migration pfn is going to refer to a precise page.
> now you're telling it to insert the head page of the folio. isn't this
> wrong?
>
>> @@ -753,13 +749,11 @@ static void __migrate_device_pages(unsigned long *src_pfns,
>> continue;
>> }
>>
>> - if (migrate && migrate->fault_page == page)
>> - r = migrate_folio_extra(mapping, page_folio(newpage),
>> - page_folio(page),
>> - MIGRATE_SYNC_NO_COPY, 1);
>> + if (migrate && migrate->fault_page == &src->page)
>
> shouldn't this rather be "page_folio(migrate->fault_page) == src"?
> ie we're looking for two pages from the same folio, rather than the page
> being the same as the head page of the folio?
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio()
2024-02-16 21:50 ` [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Matthew Wilcox
@ 2024-02-16 22:03 ` Sidhartha Kumar
0 siblings, 0 replies; 13+ messages in thread
From: Sidhartha Kumar @ 2024-02-16 22:03 UTC (permalink / raw
To: Matthew Wilcox; +Cc: linux-kernel, linux-mm, akpm, apopple
On 2/16/24 1:50 PM, Matthew Wilcox wrote:
> On Fri, Feb 16, 2024 at 01:13:15PM -0800, Sidhartha Kumar wrote:
>> +static inline struct folio *migrate_pfn_to_folio(unsigned long mpfn)
>> +{
>> + return page_folio(migrate_pfn_to_page(mpfn));
>
> umm, no.
>
> struct page *page = migrate_pfn_to_page(mpfn);
> if (page)
> return page_folio(page);
> return NULL;
Thanks, I overlooked checking for NULL before converting the page to a folio.
I'll make this change for v3.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() to folios
2024-02-16 22:00 ` Sidhartha Kumar
@ 2024-02-16 23:19 ` Matthew Wilcox
2024-02-19 9:49 ` Alistair Popple
0 siblings, 1 reply; 13+ messages in thread
From: Matthew Wilcox @ 2024-02-16 23:19 UTC (permalink / raw
To: Sidhartha Kumar; +Cc: linux-kernel, linux-mm, akpm, apopple
On Fri, Feb 16, 2024 at 02:00:31PM -0800, Sidhartha Kumar wrote:
> On 2/16/24 1:55 PM, Matthew Wilcox wrote:
> > On Fri, Feb 16, 2024 at 01:13:18PM -0800, Sidhartha Kumar wrote:
> > > Use migrate_pfn_to_folio() so we can work with folios directly in
> > > __migrate_device_pages().
> >
> > i don't understand why this would be correct if we have multipage
> > folios.
> >
>
> Alistair mentioned that he is working on order > 0 device page support so I
> was under the impression that currently device pages are only order 0.
That might well be true, but I'm *very* uncomfortable with doing a folio
conversion in core MM that won't work with large folios. We need to
consider what will happen with large device folios.
(for filesystems, I am less bothered. Individual filesystems control
whether they see large folios or not, and for lesser filesystems it may
never be worth converting them to support large folios)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() to folios
2024-02-16 23:19 ` Matthew Wilcox
@ 2024-02-19 9:49 ` Alistair Popple
0 siblings, 0 replies; 13+ messages in thread
From: Alistair Popple @ 2024-02-19 9:49 UTC (permalink / raw
To: Matthew Wilcox; +Cc: Sidhartha Kumar, linux-kernel, linux-mm, akpm
Matthew Wilcox <willy@infradead.org> writes:
> On Fri, Feb 16, 2024 at 02:00:31PM -0800, Sidhartha Kumar wrote:
>> On 2/16/24 1:55 PM, Matthew Wilcox wrote:
>> > On Fri, Feb 16, 2024 at 01:13:18PM -0800, Sidhartha Kumar wrote:
>> > > Use migrate_pfn_to_folio() so we can work with folios directly in
>> > > __migrate_device_pages().
>> >
>> > i don't understand why this would be correct if we have multipage
>> > folios.
>> >
>>
>> Alistair mentioned that he is working on order > 0 device page support so I
>> was under the impression that currently device pages are only order 0.
Right, at the moment we only create order 0 device private pages.
> That might well be true, but I'm *very* uncomfortable with doing a folio
> conversion in core MM that won't work with large folios. We need to
> consider what will happen with large device folios.
Yes. Perhaps it would be better to post these fixes as part of a series
introducing multi-page folio support for deivce private pages after
all. I don't think we can address your other comments here without first
describing how large folios for device private pages would work, and the
best way to describe that is with the patches.
I doubt I will get those posted this week, but will aim to do so in the
next 2-3 weeks.
> (for filesystems, I am less bothered. Individual filesystems control
> whether they see large folios or not, and for lesser filesystems it may
> never be worth converting them to support large folios)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-02-19 9:50 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-16 21:13 [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 2/6] mm/migrate_device: further convert migrate_device_unmap() to folios Sidhartha Kumar
2024-02-16 21:51 ` Matthew Wilcox
2024-02-16 21:13 ` [PATCH v2 3/6] mm/migrate_device: further convert migrate_device_finalize() " Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 4/6] mm/migrate_device: convert __migrate_device_pages() " Sidhartha Kumar
2024-02-16 21:55 ` Matthew Wilcox
2024-02-16 22:00 ` Sidhartha Kumar
2024-02-16 23:19 ` Matthew Wilcox
2024-02-19 9:49 ` Alistair Popple
2024-02-16 21:13 ` [PATCH v2 5/6] mm/migrate_device: convert migrate_device_coherent_page() to migrate_device_coherent_folio() Sidhartha Kumar
2024-02-16 21:13 ` [PATCH v2 6/6] mm/migrate_device: convert migrate_device_range() to folios Sidhartha Kumar
2024-02-16 21:50 ` [PATCH v2 1/6] mm/migrate: introduce migrate_pfn_to_folio() Matthew Wilcox
2024-02-16 22:03 ` Sidhartha Kumar
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).