| Message ID | 20241025004456.3435808-1-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [resend,1/2] mm: always use base address when clear gigantic page | expand |
On Fri, 25 Oct 2024 08:44:55 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > When clear gigantic page, it zeros page from the first subpage to > the last subpage, that is, aligned base address is needed in it, > and we don't need to aligned down the address in the caller as the > real address will be passed to process_huge_page(). Matthew just told us that folios con't have subpages (https://lkml.kernel.org/r/ZxsRCyBSO-C27Uzn@casper.infradead.org). Please carefully describe the impact of this change. I think it's "small cleanup and optimization?" Also, I find the changelog rather hard to follow. I think we're adding the alignment operation to the callee and hence removing it from the caller?
On 2024/10/26 6:56, Andrew Morton wrote: > On Fri, 25 Oct 2024 08:44:55 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote: > >> When clear gigantic page, it zeros page from the first subpage to >> the last subpage, that is, aligned base address is needed in it, >> and we don't need to aligned down the address in the caller as the >> real address will be passed to process_huge_page(). > > Matthew just told us that folios con't have subpages > (https://lkml.kernel.org/r/ZxsRCyBSO-C27Uzn@casper.infradead.org). > OK, will change subpage to page. > Please carefully describe the impact of this change. I think it's > "small cleanup and optimization?" > > Also, I find the changelog rather hard to follow. I think we're adding > the alignment operation to the callee and hence removing it from the > caller? > Sorry for the confuse, there is some different between gigantic page(nr_pages > MAX_ORDER_NR_PAGE) and non-gigantic page, 1) for gigantic page, it always clear/copy page from the fist page to the last page, see copy_user_gigantic_page/clear_gigantic_page, but if directly pass addr_hint which maybe not the address of the first page, then if arch's code use this addr_hint to flush cache, it may flush the wrong cache. 2) for non-gigantic page, it calculate the base address inside, see process_huge_page, if we passed the wrong addr_hint, it only has performance impact(not sure, but at least no different on arm64), no function impact. Will update the change and resend.
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c index a4441fb77f7c..a5ea006f403e 100644 --- a/fs/hugetlbfs/inode.c +++ b/fs/hugetlbfs/inode.c @@ -825,7 +825,7 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset, error = PTR_ERR(folio); goto out; } - folio_zero_user(folio, ALIGN_DOWN(addr, hpage_size)); + folio_zero_user(folio, addr); __folio_mark_uptodate(folio); error = hugetlb_add_to_page_cache(folio, mapping, index); if (unlikely(error)) { diff --git a/mm/memory.c b/mm/memory.c index 48e534aa939c..934ab5fff537 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -6802,6 +6802,7 @@ static void clear_gigantic_page(struct folio *folio, unsigned long addr, int i; might_sleep(); + addr = ALIGN_DOWN(addr, folio_size(folio)); for (i = 0; i < nr_pages; i++) { cond_resched(); clear_user_highpage(folio_page(folio, i), addr + i * PAGE_SIZE);
When clear gigantic page, it zeros page from the first subpage to the last subpage, that is, aligned base address is needed in it, and we don't need to aligned down the address in the caller as the real address will be passed to process_huge_page(). Fixes: 78fefd04c123 ("mm: memory: convert clear_huge_page() to folio_zero_user()") Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- fs/hugetlbfs/inode.c | 2 +- mm/memory.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)