Messages in this thread |  | | Date | Mon, 14 Apr 2025 08:53:02 +0200 | | From | Ingo Molnar <> | | Subject | Re: [PATCH v3 4/4] x86/folio_zero_user: multi-page clearing |
| |
* Ankur Arora <ankur.a.arora@oracle.com> wrote:
> clear_pages_rep(), clear_pages_erms() use string instructions to zero > memory. When operating on more than a single page, we can use these > more effectively by explicitly advertising the region-size to the > processor, which can use that as a hint to optimize the clearing > (ex. by eliding cacheline allocation.)
> +#ifndef CONFIG_HIGHMEM > +/* > + * folio_zero_user_preemptible(): multi-page clearing variant of folio_zero_user(). > + * > + * Taking inspiration from the common code variant, we split the zeroing in > + * three parts: left of the fault, right of the fault, and up to 5 pages > + * in the immediate neighbourhood of the target page. > + * > + * Cleared in that order to keep cache lines of the target region hot. > + * > + * For gigantic pages, there is no expectation of cache locality so just do a > + * straight zero. > + */ > +void folio_zero_user_preemptible(struct folio *folio, unsigned long addr_hint) > +{ > + unsigned long base_addr = ALIGN_DOWN(addr_hint, folio_size(folio)); > + const long fault_idx = (addr_hint - base_addr) / PAGE_SIZE; > + const struct range pg = DEFINE_RANGE(0, folio_nr_pages(folio) - 1); > + int width = 2; /* pages cleared last on either side */ > + struct range r[3]; > + int i; > + > + if (folio_nr_pages(folio) > MAX_ORDER_NR_PAGES) { > + clear_pages(page_address(folio_page(folio, 0)), folio_nr_pages(folio));
> + clear_pages(page_address(folio_page(folio, r[i].start)), len);
So the _user postfix naming is super confusing here and elsewhere in this series.
clear_page(), and by extension the clear_pages() interface you extended it to, fundamentally only works on kernel addresses:
/* * Zero a page. * %rdi - page */ SYM_TYPED_FUNC_START(clear_page_rep) movl $4096/8,%ecx xorl %eax,%eax rep stosq RET
Note the absolute lack of fault & exception handling.
But folio_zero_user*() uses the kernel-space variants of page clearing AFAICT (contrary to the naming):
void folio_zero_user(struct folio *folio, unsigned long addr_hint) { unsigned int nr_pages = folio_nr_pages(folio);
if (unlikely(nr_pages > MAX_ORDER_NR_PAGES)) clear_gigantic_page(folio, addr_hint, nr_pages); else process_huge_page(addr_hint, nr_pages, clear_subpage, folio); }
static void clear_gigantic_page(struct folio *folio, unsigned long addr_hint, unsigned int nr_pages) { unsigned long addr = ALIGN_DOWN(addr_hint, folio_size(folio)); int i; might_sleep(); for (i = 0; i < nr_pages; i++) { cond_resched(); clear_user_highpage(folio_page(folio, i), addr + i * PAGE_SIZE); } }
Which on x86 is simply mapped into a kernel-memory interface:
static inline void clear_user_page(void *page, unsigned long vaddr, struct page *pg) { clear_page(page); }
So at minimum this is a misnomer and a confusing mixture of user/kernel interface names on an epic scale that TBH should be cleaned up first before extended...
> +out: > + /* Explicitly invoke cond_resched() to handle any live patching necessary. */ > + cond_resched();
What again?
Thanks,
Ingo
|  |