* [PATCH v5 0/6] userfaultfd: convert userfaultfd functions to use folios
@ 2023-03-31 9:39 Peng Zhang
2023-03-31 9:39 ` [PATCH v5 1/6] userfaultfd: convert mfill_atomic_pte_copy() to use a folio Peng Zhang
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Peng Zhang @ 2023-03-31 9:39 UTC (permalink / raw)
To: linux-mm, linux-kernel, akpm, willy, mike.kravetz, sidhartha.kumar
Cc: vishal.moola, muchun.song, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
This patch series converts several userfaultfd functions to use folios.
Change log:
v4->v5:
- Update commit description and change page_kaddr to kaddr, suggested by
Matthew Wilcox. (patch 1,6)
- Remove pages_per_huge_page from copy_user_folio(), suggested by
Matthew Wilcox. (patch 5)
- Add RB from Sidhartha Kumar. (patch 1,3,4)
v3->v4:
- Rebase onto mm-unstable per Andrew Morton. Update commit description
because some function names are changed. (patch 1,4,6)
v2->v3:
- Split patch 2 into three patches, suggested by Mike Kravetz. (patch
2-4)
- Add a new patch to convert copy_user_huge_page to copy_user_folio,
suggested by Mike Kravetz. (patch 5)
- Fix two uninitialized bugs, thanks to Dan Carpenter. (patch 6)
- Do some indenting cleanups.
v1->v2:
Modified patch 2, suggested by Matthew Wilcox:
- Rename copy_large_folio_from_user() to copy_folio_from_user().
- Delete the inner_folio.
- kmap() and kmap_atomic() are converted to kmap_local_page(). Use
pagefault_disable() to ensure that a deadlock will not occur.
- flush_dcache_folio() is placed outside the loop.
ZhangPeng (6):
userfaultfd: convert mfill_atomic_pte_copy() to use a folio
userfaultfd: use kmap_local_page() in copy_huge_page_from_user()
userfaultfd: convert copy_huge_page_from_user() to
copy_folio_from_user()
userfaultfd: convert mfill_atomic_hugetlb() to use a folio
mm: convert copy_user_huge_page() to copy_user_folio()
userfaultfd: convert mfill_atomic() to use a folio
include/linux/hugetlb.h | 4 +-
include/linux/mm.h | 14 +++----
include/linux/shmem_fs.h | 4 +-
mm/hugetlb.c | 39 ++++++++----------
mm/memory.c | 64 ++++++++++++++---------------
mm/shmem.c | 16 ++++----
mm/userfaultfd.c | 89 ++++++++++++++++++++--------------------
7 files changed, 110 insertions(+), 120 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 1/6] userfaultfd: convert mfill_atomic_pte_copy() to use a folio
2023-03-31 9:39 [PATCH v5 0/6] userfaultfd: convert userfaultfd functions to use folios Peng Zhang
@ 2023-03-31 9:39 ` Peng Zhang
2023-04-06 21:31 ` Mike Kravetz
2023-03-31 9:39 ` [PATCH v5 2/6] userfaultfd: use kmap_local_page() in copy_huge_page_from_user() Peng Zhang
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Peng Zhang @ 2023-03-31 9:39 UTC (permalink / raw)
To: linux-mm, linux-kernel, akpm, willy, mike.kravetz, sidhartha.kumar
Cc: vishal.moola, muchun.song, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
Call vma_alloc_folio() directly instead of alloc_page_vma() and convert
page_kaddr to kaddr in mfill_atomic_pte_copy(). Removes several calls to
compound_head().
Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
---
mm/userfaultfd.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 7f1b5f8b712c..24d6ed7ff302 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -135,17 +135,18 @@ static int mfill_atomic_pte_copy(pmd_t *dst_pmd,
uffd_flags_t flags,
struct page **pagep)
{
- void *page_kaddr;
+ void *kaddr;
int ret;
- struct page *page;
+ struct folio *folio;
if (!*pagep) {
ret = -ENOMEM;
- page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, dst_vma, dst_addr);
- if (!page)
+ folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, dst_vma,
+ dst_addr, false);
+ if (!folio)
goto out;
- page_kaddr = kmap_local_page(page);
+ kaddr = kmap_local_folio(folio, 0);
/*
* The read mmap_lock is held here. Despite the
* mmap_lock being read recursive a deadlock is still
@@ -162,45 +163,45 @@ static int mfill_atomic_pte_copy(pmd_t *dst_pmd,
* and retry the copy outside the mmap_lock.
*/
pagefault_disable();
- ret = copy_from_user(page_kaddr,
- (const void __user *) src_addr,
+ ret = copy_from_user(kaddr, (const void __user *) src_addr,
PAGE_SIZE);
pagefault_enable();
- kunmap_local(page_kaddr);
+ kunmap_local(kaddr);
/* fallback to copy_from_user outside mmap_lock */
if (unlikely(ret)) {
ret = -ENOENT;
- *pagep = page;
+ *pagep = &folio->page;
/* don't free the page */
goto out;
}
- flush_dcache_page(page);
+ flush_dcache_folio(folio);
} else {
- page = *pagep;
+ folio = page_folio(*pagep);
+ VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
*pagep = NULL;
}
/*
- * The memory barrier inside __SetPageUptodate makes sure that
+ * The memory barrier inside __folio_mark_uptodate makes sure that
* preceding stores to the page contents become visible before
* the set_pte_at() write.
*/
- __SetPageUptodate(page);
+ __folio_mark_uptodate(folio);
ret = -ENOMEM;
- if (mem_cgroup_charge(page_folio(page), dst_vma->vm_mm, GFP_KERNEL))
+ if (mem_cgroup_charge(folio, dst_vma->vm_mm, GFP_KERNEL))
goto out_release;
ret = mfill_atomic_install_pte(dst_pmd, dst_vma, dst_addr,
- page, true, flags);
+ &folio->page, true, flags);
if (ret)
goto out_release;
out:
return ret;
out_release:
- put_page(page);
+ folio_put(folio);
goto out;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 2/6] userfaultfd: use kmap_local_page() in copy_huge_page_from_user()
2023-03-31 9:39 [PATCH v5 0/6] userfaultfd: convert userfaultfd functions to use folios Peng Zhang
2023-03-31 9:39 ` [PATCH v5 1/6] userfaultfd: convert mfill_atomic_pte_copy() to use a folio Peng Zhang
@ 2023-03-31 9:39 ` Peng Zhang
2023-04-06 21:32 ` Mike Kravetz
2023-03-31 9:39 ` [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user() Peng Zhang
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Peng Zhang @ 2023-03-31 9:39 UTC (permalink / raw)
To: linux-mm, linux-kernel, akpm, willy, mike.kravetz, sidhartha.kumar
Cc: vishal.moola, muchun.song, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
kmap() and kmap_atomic() are being deprecated in favor of
kmap_local_page() which is appropriate for any thread local context.[1]
Let's replace the kmap() and kmap_atomic() with kmap_local_page() in
copy_huge_page_from_user(). When allow_pagefault is false, disable page
faults to prevent potential deadlock.[2]
[1] https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com/
[2] https://lkml.kernel.org/r/20221025220136.2366143-1-ira.weiny@intel.com
Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
---
mm/memory.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 387226d6094d..808f354bce65 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5880,16 +5880,14 @@ long copy_huge_page_from_user(struct page *dst_page,
for (i = 0; i < pages_per_huge_page; i++) {
subpage = nth_page(dst_page, i);
- if (allow_pagefault)
- page_kaddr = kmap(subpage);
- else
- page_kaddr = kmap_atomic(subpage);
+ page_kaddr = kmap_local_page(subpage);
+ if (!allow_pagefault)
+ pagefault_disable();
rc = copy_from_user(page_kaddr,
usr_src + i * PAGE_SIZE, PAGE_SIZE);
- if (allow_pagefault)
- kunmap(subpage);
- else
- kunmap_atomic(page_kaddr);
+ if (!allow_pagefault)
+ pagefault_enable();
+ kunmap_local(page_kaddr);
ret_val -= (PAGE_SIZE - rc);
if (rc)
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user()
2023-03-31 9:39 [PATCH v5 0/6] userfaultfd: convert userfaultfd functions to use folios Peng Zhang
2023-03-31 9:39 ` [PATCH v5 1/6] userfaultfd: convert mfill_atomic_pte_copy() to use a folio Peng Zhang
2023-03-31 9:39 ` [PATCH v5 2/6] userfaultfd: use kmap_local_page() in copy_huge_page_from_user() Peng Zhang
@ 2023-03-31 9:39 ` Peng Zhang
2023-04-06 22:22 ` Mike Kravetz
2023-04-07 2:28 ` Vishal Moola
2023-03-31 9:39 ` [PATCH v5 4/6] userfaultfd: convert mfill_atomic_hugetlb() to use a folio Peng Zhang
` (2 subsequent siblings)
5 siblings, 2 replies; 21+ messages in thread
From: Peng Zhang @ 2023-03-31 9:39 UTC (permalink / raw)
To: linux-mm, linux-kernel, akpm, willy, mike.kravetz, sidhartha.kumar
Cc: vishal.moola, muchun.song, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
Replace copy_huge_page_from_user() with copy_folio_from_user().
copy_folio_from_user() does the same as copy_huge_page_from_user(), but
takes in a folio instead of a page. Convert page_kaddr to kaddr in
copy_folio_from_user() to do indenting cleanup.
Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
---
include/linux/mm.h | 7 +++----
mm/hugetlb.c | 5 ++---
mm/memory.c | 26 ++++++++++++--------------
mm/userfaultfd.c | 6 ++----
4 files changed, 19 insertions(+), 25 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index e249208f8fbe..cf4d773ca506 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3682,10 +3682,9 @@ extern void copy_user_huge_page(struct page *dst, struct page *src,
unsigned long addr_hint,
struct vm_area_struct *vma,
unsigned int pages_per_huge_page);
-extern long copy_huge_page_from_user(struct page *dst_page,
- const void __user *usr_src,
- unsigned int pages_per_huge_page,
- bool allow_pagefault);
+long copy_folio_from_user(struct folio *dst_folio,
+ const void __user *usr_src,
+ bool allow_pagefault);
/**
* vma_is_special_huge - Are transhuge page-table entries considered special?
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 7e4a80769c9e..aade1b513474 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6217,9 +6217,8 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
goto out;
}
- ret = copy_huge_page_from_user(&folio->page,
- (const void __user *) src_addr,
- pages_per_huge_page(h), false);
+ ret = copy_folio_from_user(folio, (const void __user *) src_addr,
+ false);
/* fallback to copy_from_user outside mmap_lock */
if (unlikely(ret)) {
diff --git a/mm/memory.c b/mm/memory.c
index 808f354bce65..4976422b6979 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5868,35 +5868,33 @@ void copy_user_huge_page(struct page *dst, struct page *src,
process_huge_page(addr_hint, pages_per_huge_page, copy_subpage, &arg);
}
-long copy_huge_page_from_user(struct page *dst_page,
- const void __user *usr_src,
- unsigned int pages_per_huge_page,
- bool allow_pagefault)
+long copy_folio_from_user(struct folio *dst_folio,
+ const void __user *usr_src,
+ bool allow_pagefault)
{
- void *page_kaddr;
+ void *kaddr;
unsigned long i, rc = 0;
- unsigned long ret_val = pages_per_huge_page * PAGE_SIZE;
+ unsigned int nr_pages = folio_nr_pages(dst_folio);
+ unsigned long ret_val = nr_pages * PAGE_SIZE;
struct page *subpage;
- for (i = 0; i < pages_per_huge_page; i++) {
- subpage = nth_page(dst_page, i);
- page_kaddr = kmap_local_page(subpage);
+ for (i = 0; i < nr_pages; i++) {
+ subpage = folio_page(dst_folio, i);
+ kaddr = kmap_local_page(subpage);
if (!allow_pagefault)
pagefault_disable();
- rc = copy_from_user(page_kaddr,
- usr_src + i * PAGE_SIZE, PAGE_SIZE);
+ rc = copy_from_user(kaddr, usr_src + i * PAGE_SIZE, PAGE_SIZE);
if (!allow_pagefault)
pagefault_enable();
- kunmap_local(page_kaddr);
+ kunmap_local(kaddr);
ret_val -= (PAGE_SIZE - rc);
if (rc)
break;
- flush_dcache_page(subpage);
-
cond_resched();
}
+ flush_dcache_folio(dst_folio);
return ret_val;
}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE || CONFIG_HUGETLBFS */
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 24d6ed7ff302..78fed9003cf7 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -422,10 +422,8 @@ static __always_inline ssize_t mfill_atomic_hugetlb(
mmap_read_unlock(dst_mm);
BUG_ON(!page);
- err = copy_huge_page_from_user(page,
- (const void __user *)src_addr,
- vma_hpagesize / PAGE_SIZE,
- true);
+ err = copy_folio_from_user(page_folio(page),
+ (const void __user *)src_addr, true);
if (unlikely(err)) {
err = -EFAULT;
goto out;
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 4/6] userfaultfd: convert mfill_atomic_hugetlb() to use a folio
2023-03-31 9:39 [PATCH v5 0/6] userfaultfd: convert userfaultfd functions to use folios Peng Zhang
` (2 preceding siblings ...)
2023-03-31 9:39 ` [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user() Peng Zhang
@ 2023-03-31 9:39 ` Peng Zhang
2023-04-06 22:48 ` Mike Kravetz
2023-03-31 9:39 ` [PATCH v5 5/6] mm: convert copy_user_huge_page() to copy_user_folio() Peng Zhang
2023-03-31 9:39 ` [PATCH v5 6/6] userfaultfd: convert mfill_atomic() to use a folio Peng Zhang
5 siblings, 1 reply; 21+ messages in thread
From: Peng Zhang @ 2023-03-31 9:39 UTC (permalink / raw)
To: linux-mm, linux-kernel, akpm, willy, mike.kravetz, sidhartha.kumar
Cc: vishal.moola, muchun.song, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
Convert hugetlb_mfill_atomic_pte() to take in a folio pointer instead of
a page pointer. Convert mfill_atomic_hugetlb() to use a folio.
Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
---
include/linux/hugetlb.h | 4 ++--
mm/hugetlb.c | 26 +++++++++++++-------------
mm/userfaultfd.c | 16 ++++++++--------
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 2a758bcd6719..28703fe22386 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -163,7 +163,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
unsigned long dst_addr,
unsigned long src_addr,
uffd_flags_t flags,
- struct page **pagep);
+ struct folio **foliop);
#endif /* CONFIG_USERFAULTFD */
bool hugetlb_reserve_pages(struct inode *inode, long from, long to,
struct vm_area_struct *vma,
@@ -397,7 +397,7 @@ static inline int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
unsigned long dst_addr,
unsigned long src_addr,
uffd_flags_t flags,
- struct page **pagep)
+ struct folio **foliop)
{
BUG();
return 0;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index aade1b513474..c88f856ec2e2 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6178,7 +6178,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
unsigned long dst_addr,
unsigned long src_addr,
uffd_flags_t flags,
- struct page **pagep)
+ struct folio **foliop)
{
struct mm_struct *dst_mm = dst_vma->vm_mm;
bool is_continue = uffd_flags_mode_is(flags, MFILL_ATOMIC_CONTINUE);
@@ -6201,8 +6201,8 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
if (IS_ERR(folio))
goto out;
folio_in_pagecache = true;
- } else if (!*pagep) {
- /* If a page already exists, then it's UFFDIO_COPY for
+ } else if (!*foliop) {
+ /* If a folio already exists, then it's UFFDIO_COPY for
* a non-missing case. Return -EEXIST.
*/
if (vm_shared &&
@@ -6237,33 +6237,33 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
ret = -ENOMEM;
goto out;
}
- *pagep = &folio->page;
- /* Set the outparam pagep and return to the caller to
+ *foliop = folio;
+ /* Set the outparam foliop and return to the caller to
* copy the contents outside the lock. Don't free the
- * page.
+ * folio.
*/
goto out;
}
} else {
if (vm_shared &&
hugetlbfs_pagecache_present(h, dst_vma, dst_addr)) {
- put_page(*pagep);
+ folio_put(*foliop);
ret = -EEXIST;
- *pagep = NULL;
+ *foliop = NULL;
goto out;
}
folio = alloc_hugetlb_folio(dst_vma, dst_addr, 0);
if (IS_ERR(folio)) {
- put_page(*pagep);
+ folio_put(*foliop);
ret = -ENOMEM;
- *pagep = NULL;
+ *foliop = NULL;
goto out;
}
- copy_user_huge_page(&folio->page, *pagep, dst_addr, dst_vma,
+ copy_user_huge_page(&folio->page, &(*foliop)->page, dst_addr, dst_vma,
pages_per_huge_page(h));
- put_page(*pagep);
- *pagep = NULL;
+ folio_put(*foliop);
+ *foliop = NULL;
}
/*
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 78fed9003cf7..f9ebaa079a6a 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -322,7 +322,7 @@ static __always_inline ssize_t mfill_atomic_hugetlb(
pte_t *dst_pte;
unsigned long src_addr, dst_addr;
long copied;
- struct page *page;
+ struct folio *folio;
unsigned long vma_hpagesize;
pgoff_t idx;
u32 hash;
@@ -342,7 +342,7 @@ static __always_inline ssize_t mfill_atomic_hugetlb(
src_addr = src_start;
dst_addr = dst_start;
copied = 0;
- page = NULL;
+ folio = NULL;
vma_hpagesize = vma_kernel_pagesize(dst_vma);
/*
@@ -411,7 +411,7 @@ static __always_inline ssize_t mfill_atomic_hugetlb(
}
err = hugetlb_mfill_atomic_pte(dst_pte, dst_vma, dst_addr,
- src_addr, flags, &page);
+ src_addr, flags, &folio);
hugetlb_vma_unlock_read(dst_vma);
mutex_unlock(&hugetlb_fault_mutex_table[hash]);
@@ -420,9 +420,9 @@ static __always_inline ssize_t mfill_atomic_hugetlb(
if (unlikely(err == -ENOENT)) {
mmap_read_unlock(dst_mm);
- BUG_ON(!page);
+ BUG_ON(!folio);
- err = copy_folio_from_user(page_folio(page),
+ err = copy_folio_from_user(folio,
(const void __user *)src_addr, true);
if (unlikely(err)) {
err = -EFAULT;
@@ -433,7 +433,7 @@ static __always_inline ssize_t mfill_atomic_hugetlb(
dst_vma = NULL;
goto retry;
} else
- BUG_ON(page);
+ BUG_ON(folio);
if (!err) {
dst_addr += vma_hpagesize;
@@ -450,8 +450,8 @@ static __always_inline ssize_t mfill_atomic_hugetlb(
out_unlock:
mmap_read_unlock(dst_mm);
out:
- if (page)
- put_page(page);
+ if (folio)
+ folio_put(folio);
BUG_ON(copied < 0);
BUG_ON(err > 0);
BUG_ON(!copied && !err);
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 5/6] mm: convert copy_user_huge_page() to copy_user_folio()
2023-03-31 9:39 [PATCH v5 0/6] userfaultfd: convert userfaultfd functions to use folios Peng Zhang
` (3 preceding siblings ...)
2023-03-31 9:39 ` [PATCH v5 4/6] userfaultfd: convert mfill_atomic_hugetlb() to use a folio Peng Zhang
@ 2023-03-31 9:39 ` Peng Zhang
2023-04-06 23:55 ` Mike Kravetz
2023-03-31 9:39 ` [PATCH v5 6/6] userfaultfd: convert mfill_atomic() to use a folio Peng Zhang
5 siblings, 1 reply; 21+ messages in thread
From: Peng Zhang @ 2023-03-31 9:39 UTC (permalink / raw)
To: linux-mm, linux-kernel, akpm, willy, mike.kravetz, sidhartha.kumar
Cc: vishal.moola, muchun.song, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
Replace copy_user_huge_page() with copy_user_folio(). copy_user_folio()
does the same as copy_user_huge_page(), but takes in folios instead of
pages. Convert copy_user_gigantic_page() to take in folios.
Remove pages_per_huge_page from copy_user_folio(), because we can get
that from folio_nr_pages(dst).
Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
---
include/linux/mm.h | 7 +++----
mm/hugetlb.c | 10 ++++------
mm/memory.c | 28 ++++++++++++++--------------
3 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index cf4d773ca506..898ece0a3802 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -3678,10 +3678,9 @@ extern const struct attribute_group memory_failure_attr_group;
extern void clear_huge_page(struct page *page,
unsigned long addr_hint,
unsigned int pages_per_huge_page);
-extern void copy_user_huge_page(struct page *dst, struct page *src,
- unsigned long addr_hint,
- struct vm_area_struct *vma,
- unsigned int pages_per_huge_page);
+void copy_user_folio(struct folio *dst, struct folio *src,
+ unsigned long addr_hint,
+ struct vm_area_struct *vma);
long copy_folio_from_user(struct folio *dst_folio,
const void __user *usr_src,
bool allow_pagefault);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c88f856ec2e2..a7ed17cbc84e 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5097,8 +5097,8 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src,
ret = PTR_ERR(new_folio);
break;
}
- copy_user_huge_page(&new_folio->page, ptepage, addr, dst_vma,
- npages);
+ copy_user_folio(new_folio, page_folio(ptepage),
+ addr, dst_vma);
put_page(ptepage);
/* Install the new hugetlb folio if src pte stable */
@@ -5616,8 +5616,7 @@ static vm_fault_t hugetlb_wp(struct mm_struct *mm, struct vm_area_struct *vma,
goto out_release_all;
}
- copy_user_huge_page(&new_folio->page, old_page, address, vma,
- pages_per_huge_page(h));
+ copy_user_folio(new_folio, page_folio(old_page), address, vma);
__folio_mark_uptodate(new_folio);
mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm, haddr,
@@ -6260,8 +6259,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
*foliop = NULL;
goto out;
}
- copy_user_huge_page(&folio->page, &(*foliop)->page, dst_addr, dst_vma,
- pages_per_huge_page(h));
+ copy_user_folio(folio, *foliop, dst_addr, dst_vma);
folio_put(*foliop);
*foliop = NULL;
}
diff --git a/mm/memory.c b/mm/memory.c
index 4976422b6979..0a5cefea9774 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5815,21 +5815,21 @@ void clear_huge_page(struct page *page,
process_huge_page(addr_hint, pages_per_huge_page, clear_subpage, page);
}
-static void copy_user_gigantic_page(struct page *dst, struct page *src,
- unsigned long addr,
- struct vm_area_struct *vma,
- unsigned int pages_per_huge_page)
+static void copy_user_gigantic_page(struct folio *dst, struct folio *src,
+ unsigned long addr,
+ struct vm_area_struct *vma,
+ unsigned int pages_per_huge_page)
{
int i;
- struct page *dst_base = dst;
- struct page *src_base = src;
+ struct page *dst_page;
+ struct page *src_page;
for (i = 0; i < pages_per_huge_page; i++) {
- dst = nth_page(dst_base, i);
- src = nth_page(src_base, i);
+ dst_page = folio_page(dst, i);
+ src_page = folio_page(src, i);
cond_resched();
- copy_user_highpage(dst, src, addr + i*PAGE_SIZE, vma);
+ copy_user_highpage(dst_page, src_page, addr + i*PAGE_SIZE, vma);
}
}
@@ -5847,15 +5847,15 @@ static void copy_subpage(unsigned long addr, int idx, void *arg)
addr, copy_arg->vma);
}
-void copy_user_huge_page(struct page *dst, struct page *src,
- unsigned long addr_hint, struct vm_area_struct *vma,
- unsigned int pages_per_huge_page)
+void copy_user_folio(struct folio *dst, struct folio *src,
+ unsigned long addr_hint, struct vm_area_struct *vma)
{
+ unsigned int pages_per_huge_page = folio_nr_pages(dst);
unsigned long addr = addr_hint &
~(((unsigned long)pages_per_huge_page << PAGE_SHIFT) - 1);
struct copy_subpage_arg arg = {
- .dst = dst,
- .src = src,
+ .dst = &dst->page,
+ .src = &src->page,
.vma = vma,
};
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v5 6/6] userfaultfd: convert mfill_atomic() to use a folio
2023-03-31 9:39 [PATCH v5 0/6] userfaultfd: convert userfaultfd functions to use folios Peng Zhang
` (4 preceding siblings ...)
2023-03-31 9:39 ` [PATCH v5 5/6] mm: convert copy_user_huge_page() to copy_user_folio() Peng Zhang
@ 2023-03-31 9:39 ` Peng Zhang
2023-04-07 0:07 ` Mike Kravetz
5 siblings, 1 reply; 21+ messages in thread
From: Peng Zhang @ 2023-03-31 9:39 UTC (permalink / raw)
To: linux-mm, linux-kernel, akpm, willy, mike.kravetz, sidhartha.kumar
Cc: vishal.moola, muchun.song, wangkefeng.wang, sunnanyong, ZhangPeng
From: ZhangPeng <zhangpeng362@huawei.com>
Convert mfill_atomic_pte_copy(), shmem_mfill_atomic_pte() and
mfill_atomic_pte() to take in a folio pointer.
Convert mfill_atomic() to use a folio. Convert page_kaddr to kaddr in
mfill_atomic().
Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
---
include/linux/shmem_fs.h | 4 ++--
mm/shmem.c | 16 ++++++++--------
mm/userfaultfd.c | 40 ++++++++++++++++++++--------------------
3 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index 3bb8d21edbb3..9e151ba45068 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -158,10 +158,10 @@ extern int shmem_mfill_atomic_pte(pmd_t *dst_pmd,
unsigned long dst_addr,
unsigned long src_addr,
uffd_flags_t flags,
- struct page **pagep);
+ struct folio **foliop);
#else /* !CONFIG_SHMEM */
#define shmem_mfill_atomic_pte(dst_pmd, dst_vma, dst_addr, \
- src_addr, flags, pagep) ({ BUG(); 0; })
+ src_addr, flags, foliop) ({ BUG(); 0; })
#endif /* CONFIG_SHMEM */
#endif /* CONFIG_USERFAULTFD */
diff --git a/mm/shmem.c b/mm/shmem.c
index 6c08f5a75d3a..9218c955f482 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2548,7 +2548,7 @@ int shmem_mfill_atomic_pte(pmd_t *dst_pmd,
unsigned long dst_addr,
unsigned long src_addr,
uffd_flags_t flags,
- struct page **pagep)
+ struct folio **foliop)
{
struct inode *inode = file_inode(dst_vma->vm_file);
struct shmem_inode_info *info = SHMEM_I(inode);
@@ -2566,14 +2566,14 @@ int shmem_mfill_atomic_pte(pmd_t *dst_pmd,
* and now we find ourselves with -ENOMEM. Release the page, to
* avoid a BUG_ON in our caller.
*/
- if (unlikely(*pagep)) {
- put_page(*pagep);
- *pagep = NULL;
+ if (unlikely(*foliop)) {
+ folio_put(*foliop);
+ *foliop = NULL;
}
return -ENOMEM;
}
- if (!*pagep) {
+ if (!*foliop) {
ret = -ENOMEM;
folio = shmem_alloc_folio(gfp, info, pgoff);
if (!folio)
@@ -2605,7 +2605,7 @@ int shmem_mfill_atomic_pte(pmd_t *dst_pmd,
/* fallback to copy_from_user outside mmap_lock */
if (unlikely(ret)) {
- *pagep = &folio->page;
+ *foliop = folio;
ret = -ENOENT;
/* don't free the page */
goto out_unacct_blocks;
@@ -2616,9 +2616,9 @@ int shmem_mfill_atomic_pte(pmd_t *dst_pmd,
clear_user_highpage(&folio->page, dst_addr);
}
} else {
- folio = page_folio(*pagep);
+ folio = *foliop;
VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
- *pagep = NULL;
+ *foliop = NULL;
}
VM_BUG_ON(folio_test_locked(folio));
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index f9ebaa079a6a..be423189120d 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -133,13 +133,13 @@ static int mfill_atomic_pte_copy(pmd_t *dst_pmd,
unsigned long dst_addr,
unsigned long src_addr,
uffd_flags_t flags,
- struct page **pagep)
+ struct folio **foliop)
{
void *kaddr;
int ret;
struct folio *folio;
- if (!*pagep) {
+ if (!*foliop) {
ret = -ENOMEM;
folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, dst_vma,
dst_addr, false);
@@ -171,16 +171,16 @@ static int mfill_atomic_pte_copy(pmd_t *dst_pmd,
/* fallback to copy_from_user outside mmap_lock */
if (unlikely(ret)) {
ret = -ENOENT;
- *pagep = &folio->page;
+ *foliop = folio;
/* don't free the page */
goto out;
}
flush_dcache_folio(folio);
} else {
- folio = page_folio(*pagep);
+ folio = *foliop;
VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
- *pagep = NULL;
+ *foliop = NULL;
}
/*
@@ -471,7 +471,7 @@ static __always_inline ssize_t mfill_atomic_pte(pmd_t *dst_pmd,
unsigned long dst_addr,
unsigned long src_addr,
uffd_flags_t flags,
- struct page **pagep)
+ struct folio **foliop)
{
ssize_t err;
@@ -494,14 +494,14 @@ static __always_inline ssize_t mfill_atomic_pte(pmd_t *dst_pmd,
if (uffd_flags_mode_is(flags, MFILL_ATOMIC_COPY))
err = mfill_atomic_pte_copy(dst_pmd, dst_vma,
dst_addr, src_addr,
- flags, pagep);
+ flags, foliop);
else
err = mfill_atomic_pte_zeropage(dst_pmd,
dst_vma, dst_addr);
} else {
err = shmem_mfill_atomic_pte(dst_pmd, dst_vma,
dst_addr, src_addr,
- flags, pagep);
+ flags, foliop);
}
return err;
@@ -519,7 +519,7 @@ static __always_inline ssize_t mfill_atomic(struct mm_struct *dst_mm,
pmd_t *dst_pmd;
unsigned long src_addr, dst_addr;
long copied;
- struct page *page;
+ struct folio *folio;
/*
* Sanitize the command parameters:
@@ -534,7 +534,7 @@ static __always_inline ssize_t mfill_atomic(struct mm_struct *dst_mm,
src_addr = src_start;
dst_addr = dst_start;
copied = 0;
- page = NULL;
+ folio = NULL;
retry:
mmap_read_lock(dst_mm);
@@ -630,28 +630,28 @@ static __always_inline ssize_t mfill_atomic(struct mm_struct *dst_mm,
BUG_ON(pmd_trans_huge(*dst_pmd));
err = mfill_atomic_pte(dst_pmd, dst_vma, dst_addr,
- src_addr, flags, &page);
+ src_addr, flags, &folio);
cond_resched();
if (unlikely(err == -ENOENT)) {
- void *page_kaddr;
+ void *kaddr;
mmap_read_unlock(dst_mm);
- BUG_ON(!page);
+ BUG_ON(!folio);
- page_kaddr = kmap_local_page(page);
- err = copy_from_user(page_kaddr,
+ kaddr = kmap_local_folio(folio, 0);
+ err = copy_from_user(kaddr,
(const void __user *) src_addr,
PAGE_SIZE);
- kunmap_local(page_kaddr);
+ kunmap_local(kaddr);
if (unlikely(err)) {
err = -EFAULT;
goto out;
}
- flush_dcache_page(page);
+ flush_dcache_folio(folio);
goto retry;
} else
- BUG_ON(page);
+ BUG_ON(folio);
if (!err) {
dst_addr += PAGE_SIZE;
@@ -668,8 +668,8 @@ static __always_inline ssize_t mfill_atomic(struct mm_struct *dst_mm,
out_unlock:
mmap_read_unlock(dst_mm);
out:
- if (page)
- put_page(page);
+ if (folio)
+ folio_put(folio);
BUG_ON(copied < 0);
BUG_ON(err > 0);
BUG_ON(!copied && !err);
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] userfaultfd: convert mfill_atomic_pte_copy() to use a folio
2023-03-31 9:39 ` [PATCH v5 1/6] userfaultfd: convert mfill_atomic_pte_copy() to use a folio Peng Zhang
@ 2023-04-06 21:31 ` Mike Kravetz
2023-04-08 4:42 ` zhangpeng (AS)
0 siblings, 1 reply; 21+ messages in thread
From: Mike Kravetz @ 2023-04-06 21:31 UTC (permalink / raw)
To: Peng Zhang
Cc: linux-mm, linux-kernel, akpm, willy, sidhartha.kumar,
vishal.moola, muchun.song, wangkefeng.wang, sunnanyong
On 03/31/23 17:39, Peng Zhang wrote:
> From: ZhangPeng <zhangpeng362@huawei.com>
>
> Call vma_alloc_folio() directly instead of alloc_page_vma() and convert
> page_kaddr to kaddr in mfill_atomic_pte_copy(). Removes several calls to
> compound_head().
>
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> ---
> mm/userfaultfd.c | 33 +++++++++++++++++----------------
> 1 file changed, 17 insertions(+), 16 deletions(-)
Looks good,
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> } else {
> - page = *pagep;
> + folio = page_folio(*pagep);
> + VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
> *pagep = NULL;
> }
However, I am still unsure about the reason for adding the VM_BUG_ON_FOLIO
here.
--
Mike Kravetz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 2/6] userfaultfd: use kmap_local_page() in copy_huge_page_from_user()
2023-03-31 9:39 ` [PATCH v5 2/6] userfaultfd: use kmap_local_page() in copy_huge_page_from_user() Peng Zhang
@ 2023-04-06 21:32 ` Mike Kravetz
0 siblings, 0 replies; 21+ messages in thread
From: Mike Kravetz @ 2023-04-06 21:32 UTC (permalink / raw)
To: Peng Zhang
Cc: linux-mm, linux-kernel, akpm, willy, sidhartha.kumar,
vishal.moola, muchun.song, wangkefeng.wang, sunnanyong
On 03/31/23 17:39, Peng Zhang wrote:
> From: ZhangPeng <zhangpeng362@huawei.com>
>
> kmap() and kmap_atomic() are being deprecated in favor of
> kmap_local_page() which is appropriate for any thread local context.[1]
>
> Let's replace the kmap() and kmap_atomic() with kmap_local_page() in
> copy_huge_page_from_user(). When allow_pagefault is false, disable page
> faults to prevent potential deadlock.[2]
>
> [1] https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com/
> [2] https://lkml.kernel.org/r/20221025220136.2366143-1-ira.weiny@intel.com
>
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> ---
> mm/memory.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
Thanks,
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user()
2023-03-31 9:39 ` [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user() Peng Zhang
@ 2023-04-06 22:22 ` Mike Kravetz
2023-04-07 2:28 ` Vishal Moola
1 sibling, 0 replies; 21+ messages in thread
From: Mike Kravetz @ 2023-04-06 22:22 UTC (permalink / raw)
To: Peng Zhang
Cc: linux-mm, linux-kernel, akpm, willy, sidhartha.kumar,
vishal.moola, muchun.song, wangkefeng.wang, sunnanyong
On 03/31/23 17:39, Peng Zhang wrote:
> From: ZhangPeng <zhangpeng362@huawei.com>
>
> Replace copy_huge_page_from_user() with copy_folio_from_user().
> copy_folio_from_user() does the same as copy_huge_page_from_user(), but
> takes in a folio instead of a page. Convert page_kaddr to kaddr in
> copy_folio_from_user() to do indenting cleanup.
>
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> ---
> include/linux/mm.h | 7 +++----
> mm/hugetlb.c | 5 ++---
> mm/memory.c | 26 ++++++++++++--------------
> mm/userfaultfd.c | 6 ++----
> 4 files changed, 19 insertions(+), 25 deletions(-)
Thanks,
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 4/6] userfaultfd: convert mfill_atomic_hugetlb() to use a folio
2023-03-31 9:39 ` [PATCH v5 4/6] userfaultfd: convert mfill_atomic_hugetlb() to use a folio Peng Zhang
@ 2023-04-06 22:48 ` Mike Kravetz
0 siblings, 0 replies; 21+ messages in thread
From: Mike Kravetz @ 2023-04-06 22:48 UTC (permalink / raw)
To: Peng Zhang
Cc: linux-mm, linux-kernel, akpm, willy, sidhartha.kumar,
vishal.moola, muchun.song, wangkefeng.wang, sunnanyong
On 03/31/23 17:39, Peng Zhang wrote:
> From: ZhangPeng <zhangpeng362@huawei.com>
>
> Convert hugetlb_mfill_atomic_pte() to take in a folio pointer instead of
> a page pointer. Convert mfill_atomic_hugetlb() to use a folio.
>
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> ---
> include/linux/hugetlb.h | 4 ++--
> mm/hugetlb.c | 26 +++++++++++++-------------
> mm/userfaultfd.c | 16 ++++++++--------
> 3 files changed, 23 insertions(+), 23 deletions(-)
Thanks,
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 5/6] mm: convert copy_user_huge_page() to copy_user_folio()
2023-03-31 9:39 ` [PATCH v5 5/6] mm: convert copy_user_huge_page() to copy_user_folio() Peng Zhang
@ 2023-04-06 23:55 ` Mike Kravetz
2023-04-08 4:42 ` zhangpeng (AS)
0 siblings, 1 reply; 21+ messages in thread
From: Mike Kravetz @ 2023-04-06 23:55 UTC (permalink / raw)
To: Peng Zhang
Cc: linux-mm, linux-kernel, akpm, willy, sidhartha.kumar,
vishal.moola, muchun.song, wangkefeng.wang, sunnanyong
On 03/31/23 17:39, Peng Zhang wrote:
> From: ZhangPeng <zhangpeng362@huawei.com>
>
> Replace copy_user_huge_page() with copy_user_folio(). copy_user_folio()
> does the same as copy_user_huge_page(), but takes in folios instead of
> pages. Convert copy_user_gigantic_page() to take in folios.
> Remove pages_per_huge_page from copy_user_folio(), because we can get
> that from folio_nr_pages(dst).
>
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> ---
> include/linux/mm.h | 7 +++----
> mm/hugetlb.c | 10 ++++------
> mm/memory.c | 28 ++++++++++++++--------------
> 3 files changed, 21 insertions(+), 24 deletions(-)
No technical problems with the patch, but ...
>
> @@ -5847,15 +5847,15 @@ static void copy_subpage(unsigned long addr, int idx, void *arg)
> addr, copy_arg->vma);
> }
>
> -void copy_user_huge_page(struct page *dst, struct page *src,
> - unsigned long addr_hint, struct vm_area_struct *vma,
> - unsigned int pages_per_huge_page)
> +void copy_user_folio(struct folio *dst, struct folio *src,
> + unsigned long addr_hint, struct vm_area_struct *vma)
> {
> + unsigned int pages_per_huge_page = folio_nr_pages(dst);
> unsigned long addr = addr_hint &
> ~(((unsigned long)pages_per_huge_page << PAGE_SHIFT) - 1);
> struct copy_subpage_arg arg = {
> - .dst = dst,
> - .src = src,
> + .dst = &dst->page,
> + .src = &src->page,
> .vma = vma,
> };
>
I seem to recall that Matthew suggested changing the function name to
copy_user_folio. My only concern is that the name now sounds like a
general purpose routine for copying folios. It certainly would work
for a single page folio, but there is a bunch of unnecessary overhead
in that case.
That makes me think there should perhaps be an optimized path for single
page folios that just does copy_user_highpage(). But, the argument addr_hint
does not make much sense in the single page folio case. So, I am not
sure if I agree with leaving large/huge out of the function name.
Just wondering if Matthew has any additional thoughts?
--
Mike Kravetz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 6/6] userfaultfd: convert mfill_atomic() to use a folio
2023-03-31 9:39 ` [PATCH v5 6/6] userfaultfd: convert mfill_atomic() to use a folio Peng Zhang
@ 2023-04-07 0:07 ` Mike Kravetz
0 siblings, 0 replies; 21+ messages in thread
From: Mike Kravetz @ 2023-04-07 0:07 UTC (permalink / raw)
To: Peng Zhang
Cc: linux-mm, linux-kernel, akpm, willy, sidhartha.kumar,
vishal.moola, muchun.song, wangkefeng.wang, sunnanyong
On 03/31/23 17:39, Peng Zhang wrote:
> From: ZhangPeng <zhangpeng362@huawei.com>
>
> Convert mfill_atomic_pte_copy(), shmem_mfill_atomic_pte() and
> mfill_atomic_pte() to take in a folio pointer.
> Convert mfill_atomic() to use a folio. Convert page_kaddr to kaddr in
> mfill_atomic().
>
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> ---
> include/linux/shmem_fs.h | 4 ++--
> mm/shmem.c | 16 ++++++++--------
> mm/userfaultfd.c | 40 ++++++++++++++++++++--------------------
> 3 files changed, 30 insertions(+), 30 deletions(-)
Looks good to me,
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user()
2023-03-31 9:39 ` [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user() Peng Zhang
2023-04-06 22:22 ` Mike Kravetz
@ 2023-04-07 2:28 ` Vishal Moola
2023-04-08 4:43 ` zhangpeng (AS)
2023-04-11 3:40 ` Matthew Wilcox
1 sibling, 2 replies; 21+ messages in thread
From: Vishal Moola @ 2023-04-07 2:28 UTC (permalink / raw)
To: Peng Zhang
Cc: linux-mm, linux-kernel, akpm, willy, mike.kravetz,
sidhartha.kumar, muchun.song, wangkefeng.wang, sunnanyong
On Fri, Mar 31, 2023 at 2:41 AM Peng Zhang <zhangpeng362@huawei.com> wrote:
>
> From: ZhangPeng <zhangpeng362@huawei.com>
>
> Replace copy_huge_page_from_user() with copy_folio_from_user().
> copy_folio_from_user() does the same as copy_huge_page_from_user(), but
> takes in a folio instead of a page. Convert page_kaddr to kaddr in
> copy_folio_from_user() to do indenting cleanup.
>
> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> ---
> include/linux/mm.h | 7 +++----
> mm/hugetlb.c | 5 ++---
> mm/memory.c | 26 ++++++++++++--------------
> mm/userfaultfd.c | 6 ++----
> 4 files changed, 19 insertions(+), 25 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index e249208f8fbe..cf4d773ca506 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3682,10 +3682,9 @@ extern void copy_user_huge_page(struct page *dst, struct page *src,
> unsigned long addr_hint,
> struct vm_area_struct *vma,
> unsigned int pages_per_huge_page);
> -extern long copy_huge_page_from_user(struct page *dst_page,
> - const void __user *usr_src,
> - unsigned int pages_per_huge_page,
> - bool allow_pagefault);
> +long copy_folio_from_user(struct folio *dst_folio,
> + const void __user *usr_src,
> + bool allow_pagefault);
>
> /**
> * vma_is_special_huge - Are transhuge page-table entries considered special?
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 7e4a80769c9e..aade1b513474 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6217,9 +6217,8 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
> goto out;
> }
>
> - ret = copy_huge_page_from_user(&folio->page,
> - (const void __user *) src_addr,
> - pages_per_huge_page(h), false);
> + ret = copy_folio_from_user(folio, (const void __user *) src_addr,
> + false);
>
> /* fallback to copy_from_user outside mmap_lock */
> if (unlikely(ret)) {
> diff --git a/mm/memory.c b/mm/memory.c
> index 808f354bce65..4976422b6979 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5868,35 +5868,33 @@ void copy_user_huge_page(struct page *dst, struct page *src,
> process_huge_page(addr_hint, pages_per_huge_page, copy_subpage, &arg);
> }
>
> -long copy_huge_page_from_user(struct page *dst_page,
> - const void __user *usr_src,
> - unsigned int pages_per_huge_page,
> - bool allow_pagefault)
> +long copy_folio_from_user(struct folio *dst_folio,
> + const void __user *usr_src,
> + bool allow_pagefault)
> {
> - void *page_kaddr;
> + void *kaddr;
> unsigned long i, rc = 0;
> - unsigned long ret_val = pages_per_huge_page * PAGE_SIZE;
> + unsigned int nr_pages = folio_nr_pages(dst_folio);
> + unsigned long ret_val = nr_pages * PAGE_SIZE;
> struct page *subpage;
>
> - for (i = 0; i < pages_per_huge_page; i++) {
> - subpage = nth_page(dst_page, i);
> - page_kaddr = kmap_local_page(subpage);
> + for (i = 0; i < nr_pages; i++) {
> + subpage = folio_page(dst_folio, i);
> + kaddr = kmap_local_page(subpage);
> if (!allow_pagefault)
> pagefault_disable();
> - rc = copy_from_user(page_kaddr,
> - usr_src + i * PAGE_SIZE, PAGE_SIZE);
> + rc = copy_from_user(kaddr, usr_src + i * PAGE_SIZE, PAGE_SIZE);
> if (!allow_pagefault)
> pagefault_enable();
> - kunmap_local(page_kaddr);
> + kunmap_local(kaddr);
>
> ret_val -= (PAGE_SIZE - rc);
> if (rc)
> break;
>
> - flush_dcache_page(subpage);
> -
> cond_resched();
> }
> + flush_dcache_folio(dst_folio);
> return ret_val;
> }
Moving the flush_dcache_page() outside the loop to be
flush_dcache_folio() changes the behavior of the function.
Initially, if it fails to copy the entire page, the function breaks out
of the loop and returns the number of unwritten bytes without
flushing the page from the cache. Now if it fails, it will still flush
out the page it failed on, as well as any later pages it may not
have gotten to yet.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] userfaultfd: convert mfill_atomic_pte_copy() to use a folio
2023-04-06 21:31 ` Mike Kravetz
@ 2023-04-08 4:42 ` zhangpeng (AS)
0 siblings, 0 replies; 21+ messages in thread
From: zhangpeng (AS) @ 2023-04-08 4:42 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm, linux-kernel, akpm, willy, sidhartha.kumar,
vishal.moola, muchun.song, wangkefeng.wang, sunnanyong
On 2023/4/7 5:31, Mike Kravetz wrote:
> On 03/31/23 17:39, Peng Zhang wrote:
>> From: ZhangPeng <zhangpeng362@huawei.com>
>>
>> Call vma_alloc_folio() directly instead of alloc_page_vma() and convert
>> page_kaddr to kaddr in mfill_atomic_pte_copy(). Removes several calls to
>> compound_head().
>>
>> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
>> Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
>> ---
>> mm/userfaultfd.c | 33 +++++++++++++++++----------------
>> 1 file changed, 17 insertions(+), 16 deletions(-)
> Looks good,
>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
>
>> } else {
>> - page = *pagep;
>> + folio = page_folio(*pagep);
>> + VM_BUG_ON_FOLIO(folio_test_large(folio), folio);
>> *pagep = NULL;
>> }
> However, I am still unsure about the reason for adding the VM_BUG_ON_FOLIO
> here.
VM_BUG_ON_FOLIO was added to ensure that folio is a single-page folio.
However, the folio corresponding to the foliop is always a single-page
folio. We just don't need this check. I'll drop VM_BUG_ON_FOLIO.
Thanks for your review.
Best Regards,
Peng
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 5/6] mm: convert copy_user_huge_page() to copy_user_folio()
2023-04-06 23:55 ` Mike Kravetz
@ 2023-04-08 4:42 ` zhangpeng (AS)
0 siblings, 0 replies; 21+ messages in thread
From: zhangpeng (AS) @ 2023-04-08 4:42 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-mm, linux-kernel, akpm, willy, sidhartha.kumar,
vishal.moola, muchun.song, wangkefeng.wang, sunnanyong
On 2023/4/7 7:55, Mike Kravetz wrote:
> On 03/31/23 17:39, Peng Zhang wrote:
>> From: ZhangPeng <zhangpeng362@huawei.com>
>>
>> Replace copy_user_huge_page() with copy_user_folio(). copy_user_folio()
>> does the same as copy_user_huge_page(), but takes in folios instead of
>> pages. Convert copy_user_gigantic_page() to take in folios.
>> Remove pages_per_huge_page from copy_user_folio(), because we can get
>> that from folio_nr_pages(dst).
>>
>> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
>> ---
>> include/linux/mm.h | 7 +++----
>> mm/hugetlb.c | 10 ++++------
>> mm/memory.c | 28 ++++++++++++++--------------
>> 3 files changed, 21 insertions(+), 24 deletions(-)
> No technical problems with the patch, but ...
>>
>> @@ -5847,15 +5847,15 @@ static void copy_subpage(unsigned long addr, int idx, void *arg)
>> addr, copy_arg->vma);
>> }
>>
>> -void copy_user_huge_page(struct page *dst, struct page *src,
>> - unsigned long addr_hint, struct vm_area_struct *vma,
>> - unsigned int pages_per_huge_page)
>> +void copy_user_folio(struct folio *dst, struct folio *src,
>> + unsigned long addr_hint, struct vm_area_struct *vma)
>> {
>> + unsigned int pages_per_huge_page = folio_nr_pages(dst);
>> unsigned long addr = addr_hint &
>> ~(((unsigned long)pages_per_huge_page << PAGE_SHIFT) - 1);
>> struct copy_subpage_arg arg = {
>> - .dst = dst,
>> - .src = src,
>> + .dst = &dst->page,
>> + .src = &src->page,
>> .vma = vma,
>> };
>>
> I seem to recall that Matthew suggested changing the function name to
> copy_user_folio. My only concern is that the name now sounds like a
> general purpose routine for copying folios. It certainly would work
> for a single page folio, but there is a bunch of unnecessary overhead
> in that case.
>
> That makes me think there should perhaps be an optimized path for single
> page folios that just does copy_user_highpage(). But, the argument addr_hint
> does not make much sense in the single page folio case. So, I am not
> sure if I agree with leaving large/huge out of the function name.
>
> Just wondering if Matthew has any additional thoughts?
Agreed. In my opinion, it's better to leave large/huge out of the
function name.
Also wondering if Matthew has any additional considerations?
Best Regards,
Peng
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user()
2023-04-07 2:28 ` Vishal Moola
@ 2023-04-08 4:43 ` zhangpeng (AS)
2023-04-10 21:26 ` Mike Kravetz
2023-04-11 3:40 ` Matthew Wilcox
1 sibling, 1 reply; 21+ messages in thread
From: zhangpeng (AS) @ 2023-04-08 4:43 UTC (permalink / raw)
To: Vishal Moola
Cc: linux-mm, linux-kernel, akpm, willy, mike.kravetz,
sidhartha.kumar, muchun.song, wangkefeng.wang, sunnanyong
On 2023/4/7 10:28, Vishal Moola wrote:
> On Fri, Mar 31, 2023 at 2:41 AM Peng Zhang <zhangpeng362@huawei.com> wrote:
>> From: ZhangPeng <zhangpeng362@huawei.com>
>>
>> Replace copy_huge_page_from_user() with copy_folio_from_user().
>> copy_folio_from_user() does the same as copy_huge_page_from_user(), but
>> takes in a folio instead of a page. Convert page_kaddr to kaddr in
>> copy_folio_from_user() to do indenting cleanup.
>>
>> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
>> Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
>> ---
>> include/linux/mm.h | 7 +++----
>> mm/hugetlb.c | 5 ++---
>> mm/memory.c | 26 ++++++++++++--------------
>> mm/userfaultfd.c | 6 ++----
>> 4 files changed, 19 insertions(+), 25 deletions(-)
>>
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index e249208f8fbe..cf4d773ca506 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -3682,10 +3682,9 @@ extern void copy_user_huge_page(struct page *dst, struct page *src,
>> unsigned long addr_hint,
>> struct vm_area_struct *vma,
>> unsigned int pages_per_huge_page);
>> -extern long copy_huge_page_from_user(struct page *dst_page,
>> - const void __user *usr_src,
>> - unsigned int pages_per_huge_page,
>> - bool allow_pagefault);
>> +long copy_folio_from_user(struct folio *dst_folio,
>> + const void __user *usr_src,
>> + bool allow_pagefault);
>>
>> /**
>> * vma_is_special_huge - Are transhuge page-table entries considered special?
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 7e4a80769c9e..aade1b513474 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -6217,9 +6217,8 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
>> goto out;
>> }
>>
>> - ret = copy_huge_page_from_user(&folio->page,
>> - (const void __user *) src_addr,
>> - pages_per_huge_page(h), false);
>> + ret = copy_folio_from_user(folio, (const void __user *) src_addr,
>> + false);
>>
>> /* fallback to copy_from_user outside mmap_lock */
>> if (unlikely(ret)) {
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 808f354bce65..4976422b6979 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -5868,35 +5868,33 @@ void copy_user_huge_page(struct page *dst, struct page *src,
>> process_huge_page(addr_hint, pages_per_huge_page, copy_subpage, &arg);
>> }
>>
>> -long copy_huge_page_from_user(struct page *dst_page,
>> - const void __user *usr_src,
>> - unsigned int pages_per_huge_page,
>> - bool allow_pagefault)
>> +long copy_folio_from_user(struct folio *dst_folio,
>> + const void __user *usr_src,
>> + bool allow_pagefault)
>> {
>> - void *page_kaddr;
>> + void *kaddr;
>> unsigned long i, rc = 0;
>> - unsigned long ret_val = pages_per_huge_page * PAGE_SIZE;
>> + unsigned int nr_pages = folio_nr_pages(dst_folio);
>> + unsigned long ret_val = nr_pages * PAGE_SIZE;
>> struct page *subpage;
>>
>> - for (i = 0; i < pages_per_huge_page; i++) {
>> - subpage = nth_page(dst_page, i);
>> - page_kaddr = kmap_local_page(subpage);
>> + for (i = 0; i < nr_pages; i++) {
>> + subpage = folio_page(dst_folio, i);
>> + kaddr = kmap_local_page(subpage);
>> if (!allow_pagefault)
>> pagefault_disable();
>> - rc = copy_from_user(page_kaddr,
>> - usr_src + i * PAGE_SIZE, PAGE_SIZE);
>> + rc = copy_from_user(kaddr, usr_src + i * PAGE_SIZE, PAGE_SIZE);
>> if (!allow_pagefault)
>> pagefault_enable();
>> - kunmap_local(page_kaddr);
>> + kunmap_local(kaddr);
>>
>> ret_val -= (PAGE_SIZE - rc);
>> if (rc)
>> break;
>>
>> - flush_dcache_page(subpage);
>> -
>> cond_resched();
>> }
>> + flush_dcache_folio(dst_folio);
>> return ret_val;
>> }
> Moving the flush_dcache_page() outside the loop to be
> flush_dcache_folio() changes the behavior of the function.
>
> Initially, if it fails to copy the entire page, the function breaks out
> of the loop and returns the number of unwritten bytes without
> flushing the page from the cache. Now if it fails, it will still flush
> out the page it failed on, as well as any later pages it may not
> have gotten to yet.
Agreed. If it fails, could we just not flush the folio?
Like this:
long copy_folio_from_user(...)
{
...
for (i = 0; i < nr_pages; i++) {
...
rc = copy_from_user(kaddr, usr_src + i * PAGE_SIZE, PAGE_SIZE);
...
ret_val -= (PAGE_SIZE - rc);
if (rc)
- break;
+ return ret_val;
cond_resched();
}
flush_dcache_folio(dst_folio);
return ret_val;
}
Thanks for your review.
Best Regards,
Peng
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user()
2023-04-08 4:43 ` zhangpeng (AS)
@ 2023-04-10 21:26 ` Mike Kravetz
2023-04-11 1:30 ` Yin, Fengwei
0 siblings, 1 reply; 21+ messages in thread
From: Mike Kravetz @ 2023-04-10 21:26 UTC (permalink / raw)
To: zhangpeng (AS)
Cc: Vishal Moola, linux-mm, linux-kernel, akpm, willy,
sidhartha.kumar, muchun.song, wangkefeng.wang, sunnanyong
On 04/08/23 12:43, zhangpeng (AS) wrote:
> On 2023/4/7 10:28, Vishal Moola wrote:
>
> > On Fri, Mar 31, 2023 at 2:41 AM Peng Zhang <zhangpeng362@huawei.com> wrote:
> > > From: ZhangPeng <zhangpeng362@huawei.com>
> > >
> > > Replace copy_huge_page_from_user() with copy_folio_from_user().
> > > copy_folio_from_user() does the same as copy_huge_page_from_user(), but
> > > takes in a folio instead of a page. Convert page_kaddr to kaddr in
> > > copy_folio_from_user() to do indenting cleanup.
> > >
> > > Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
> > > Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
> > > ---
> > > - bool allow_pagefault)
> > > +long copy_folio_from_user(struct folio *dst_folio,
> > > + const void __user *usr_src,
> > > + bool allow_pagefault)
> > > {
> > > - void *page_kaddr;
> > > + void *kaddr;
> > > unsigned long i, rc = 0;
> > > - unsigned long ret_val = pages_per_huge_page * PAGE_SIZE;
> > > + unsigned int nr_pages = folio_nr_pages(dst_folio);
> > > + unsigned long ret_val = nr_pages * PAGE_SIZE;
> > > struct page *subpage;
> > >
> > > - for (i = 0; i < pages_per_huge_page; i++) {
> > > - subpage = nth_page(dst_page, i);
> > > - page_kaddr = kmap_local_page(subpage);
> > > + for (i = 0; i < nr_pages; i++) {
> > > + subpage = folio_page(dst_folio, i);
> > > + kaddr = kmap_local_page(subpage);
> > > if (!allow_pagefault)
> > > pagefault_disable();
> > > - rc = copy_from_user(page_kaddr,
> > > - usr_src + i * PAGE_SIZE, PAGE_SIZE);
> > > + rc = copy_from_user(kaddr, usr_src + i * PAGE_SIZE, PAGE_SIZE);
> > > if (!allow_pagefault)
> > > pagefault_enable();
> > > - kunmap_local(page_kaddr);
> > > + kunmap_local(kaddr);
> > >
> > > ret_val -= (PAGE_SIZE - rc);
> > > if (rc)
> > > break;
> > >
> > > - flush_dcache_page(subpage);
> > > -
> > > cond_resched();
> > > }
> > > + flush_dcache_folio(dst_folio);
> > > return ret_val;
> > > }
> > Moving the flush_dcache_page() outside the loop to be
> > flush_dcache_folio() changes the behavior of the function.
> >
> > Initially, if it fails to copy the entire page, the function breaks out
> > of the loop and returns the number of unwritten bytes without
> > flushing the page from the cache. Now if it fails, it will still flush
> > out the page it failed on, as well as any later pages it may not
> > have gotten to yet.
>
> Agreed. If it fails, could we just not flush the folio?
I believe that should be OK. If returning an error, nobody should be
depending on any part of the page being present or not in the cache.
--
Mike Kravetz
> Like this:
> long copy_folio_from_user(...)
> {
> ...
> for (i = 0; i < nr_pages; i++) {
> ...
> rc = copy_from_user(kaddr, usr_src + i * PAGE_SIZE, PAGE_SIZE);
> ...
> ret_val -= (PAGE_SIZE - rc);
> if (rc)
> - break;
> + return ret_val;
> cond_resched();
> }
> flush_dcache_folio(dst_folio);
> return ret_val;
> }
>
> Thanks for your review.
>
> Best Regards,
> Peng
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user()
2023-04-10 21:26 ` Mike Kravetz
@ 2023-04-11 1:30 ` Yin, Fengwei
0 siblings, 0 replies; 21+ messages in thread
From: Yin, Fengwei @ 2023-04-11 1:30 UTC (permalink / raw)
To: Mike Kravetz, zhangpeng (AS)
Cc: Vishal Moola, linux-mm, linux-kernel, akpm, willy,
sidhartha.kumar, muchun.song, wangkefeng.wang, sunnanyong
On 4/11/2023 5:26 AM, Mike Kravetz wrote:
> On 04/08/23 12:43, zhangpeng (AS) wrote:
>> On 2023/4/7 10:28, Vishal Moola wrote:
>>
>>> On Fri, Mar 31, 2023 at 2:41 AM Peng Zhang <zhangpeng362@huawei.com> wrote:
>>>> From: ZhangPeng <zhangpeng362@huawei.com>
>>>>
>>>> Replace copy_huge_page_from_user() with copy_folio_from_user().
>>>> copy_folio_from_user() does the same as copy_huge_page_from_user(), but
>>>> takes in a folio instead of a page. Convert page_kaddr to kaddr in
>>>> copy_folio_from_user() to do indenting cleanup.
>>>>
>>>> Signed-off-by: ZhangPeng <zhangpeng362@huawei.com>
>>>> Reviewed-by: Sidhartha Kumar <sidhartha.kumar@oracle.com>
>>>> ---
>>>> - bool allow_pagefault)
>>>> +long copy_folio_from_user(struct folio *dst_folio,
>>>> + const void __user *usr_src,
>>>> + bool allow_pagefault)
>>>> {
>>>> - void *page_kaddr;
>>>> + void *kaddr;
>>>> unsigned long i, rc = 0;
>>>> - unsigned long ret_val = pages_per_huge_page * PAGE_SIZE;
>>>> + unsigned int nr_pages = folio_nr_pages(dst_folio);
>>>> + unsigned long ret_val = nr_pages * PAGE_SIZE;
>>>> struct page *subpage;
>>>>
>>>> - for (i = 0; i < pages_per_huge_page; i++) {
>>>> - subpage = nth_page(dst_page, i);
>>>> - page_kaddr = kmap_local_page(subpage);
>>>> + for (i = 0; i < nr_pages; i++) {
>>>> + subpage = folio_page(dst_folio, i);
>>>> + kaddr = kmap_local_page(subpage);
>>>> if (!allow_pagefault)
>>>> pagefault_disable();
>>>> - rc = copy_from_user(page_kaddr,
>>>> - usr_src + i * PAGE_SIZE, PAGE_SIZE);
>>>> + rc = copy_from_user(kaddr, usr_src + i * PAGE_SIZE, PAGE_SIZE);
>>>> if (!allow_pagefault)
>>>> pagefault_enable();
>>>> - kunmap_local(page_kaddr);
>>>> + kunmap_local(kaddr);
>>>>
>>>> ret_val -= (PAGE_SIZE - rc);
>>>> if (rc)
>>>> break;
>>>>
>>>> - flush_dcache_page(subpage);
>>>> -
>>>> cond_resched();
>>>> }
>>>> + flush_dcache_folio(dst_folio);
>>>> return ret_val;
>>>> }
>>> Moving the flush_dcache_page() outside the loop to be
>>> flush_dcache_folio() changes the behavior of the function.
>>>
>>> Initially, if it fails to copy the entire page, the function breaks out
>>> of the loop and returns the number of unwritten bytes without
>>> flushing the page from the cache. Now if it fails, it will still flush
>>> out the page it failed on, as well as any later pages it may not
>>> have gotten to yet.
>>
>> Agreed. If it fails, could we just not flush the folio?
>
> I believe that should be OK. If returning an error, nobody should be
> depending on any part of the page being present or not in the cache.
Maybe we should flush_dcache because this function returns the
bytes copied successfully? flushing cache to make sure the copied
pieces to RAM for sure.
For the range not copied yet, flushing cache or not doesn't make
difference. Thanks.
Regards
Yin, Fengwei
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user()
2023-04-07 2:28 ` Vishal Moola
2023-04-08 4:43 ` zhangpeng (AS)
@ 2023-04-11 3:40 ` Matthew Wilcox
2023-04-18 22:21 ` Andrew Morton
1 sibling, 1 reply; 21+ messages in thread
From: Matthew Wilcox @ 2023-04-11 3:40 UTC (permalink / raw)
To: Vishal Moola
Cc: Peng Zhang, linux-mm, linux-kernel, akpm, mike.kravetz,
sidhartha.kumar, muchun.song, wangkefeng.wang, sunnanyong
On Thu, Apr 06, 2023 at 07:28:44PM -0700, Vishal Moola wrote:
> > - flush_dcache_page(subpage);
> > -
> > cond_resched();
> > }
> > + flush_dcache_folio(dst_folio);
> > return ret_val;
> > }
>
> Moving the flush_dcache_page() outside the loop to be
> flush_dcache_folio() changes the behavior of the function.
>
> Initially, if it fails to copy the entire page, the function breaks out
> of the loop and returns the number of unwritten bytes without
> flushing the page from the cache. Now if it fails, it will still flush
> out the page it failed on, as well as any later pages it may not
> have gotten to yet.
I'm not sure this is worth worrying about. Failing to copy the entire
folio is unlikely, and if we do, flushing the entire folio instead of just
a few pages in it is harmless. Plus I have patches which significantly
optiise flush_dcache_folio() over flush_dcache_page() (for the majority
of architectures) and so I think this change is actually beneficial in
the long term.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user()
2023-04-11 3:40 ` Matthew Wilcox
@ 2023-04-18 22:21 ` Andrew Morton
0 siblings, 0 replies; 21+ messages in thread
From: Andrew Morton @ 2023-04-18 22:21 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Vishal Moola, Peng Zhang, linux-mm, linux-kernel, mike.kravetz,
sidhartha.kumar, muchun.song, wangkefeng.wang, sunnanyong
On Tue, 11 Apr 2023 04:40:17 +0100 Matthew Wilcox <willy@infradead.org> wrote:
> On Thu, Apr 06, 2023 at 07:28:44PM -0700, Vishal Moola wrote:
> > > - flush_dcache_page(subpage);
> > > -
> > > cond_resched();
> > > }
> > > + flush_dcache_folio(dst_folio);
> > > return ret_val;
> > > }
> >
> > Moving the flush_dcache_page() outside the loop to be
> > flush_dcache_folio() changes the behavior of the function.
> >
> > Initially, if it fails to copy the entire page, the function breaks out
> > of the loop and returns the number of unwritten bytes without
> > flushing the page from the cache. Now if it fails, it will still flush
> > out the page it failed on, as well as any later pages it may not
> > have gotten to yet.
>
> I'm not sure this is worth worrying about. Failing to copy the entire
> folio is unlikely, and if we do, flushing the entire folio instead of just
> a few pages in it is harmless. Plus I have patches which significantly
> optiise flush_dcache_folio() over flush_dcache_page() (for the majority
> of architectures) and so I think this change is actually beneficial in
> the long term.
Thanks, I'll send the series in for the next merge window as-is. If
others remain unhappy with the flushing issue, please propose something
during the next -rc cycle.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-04-18 22:21 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-31 9:39 [PATCH v5 0/6] userfaultfd: convert userfaultfd functions to use folios Peng Zhang
2023-03-31 9:39 ` [PATCH v5 1/6] userfaultfd: convert mfill_atomic_pte_copy() to use a folio Peng Zhang
2023-04-06 21:31 ` Mike Kravetz
2023-04-08 4:42 ` zhangpeng (AS)
2023-03-31 9:39 ` [PATCH v5 2/6] userfaultfd: use kmap_local_page() in copy_huge_page_from_user() Peng Zhang
2023-04-06 21:32 ` Mike Kravetz
2023-03-31 9:39 ` [PATCH v5 3/6] userfaultfd: convert copy_huge_page_from_user() to copy_folio_from_user() Peng Zhang
2023-04-06 22:22 ` Mike Kravetz
2023-04-07 2:28 ` Vishal Moola
2023-04-08 4:43 ` zhangpeng (AS)
2023-04-10 21:26 ` Mike Kravetz
2023-04-11 1:30 ` Yin, Fengwei
2023-04-11 3:40 ` Matthew Wilcox
2023-04-18 22:21 ` Andrew Morton
2023-03-31 9:39 ` [PATCH v5 4/6] userfaultfd: convert mfill_atomic_hugetlb() to use a folio Peng Zhang
2023-04-06 22:48 ` Mike Kravetz
2023-03-31 9:39 ` [PATCH v5 5/6] mm: convert copy_user_huge_page() to copy_user_folio() Peng Zhang
2023-04-06 23:55 ` Mike Kravetz
2023-04-08 4:42 ` zhangpeng (AS)
2023-03-31 9:39 ` [PATCH v5 6/6] userfaultfd: convert mfill_atomic() to use a folio Peng Zhang
2023-04-07 0:07 ` Mike Kravetz
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 NNTP newsgroup(s).