From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Shivank Garg <shivankg@amd.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
David Hildenbrand <david@kernel.org>,
"Liam R. Howlett" <liam@infradead.org>,
Vlastimil Babka <vbabka@kernel.org>,
Mike Rapoport <rppt@kernel.org>,
Suren Baghdasaryan <surenb@google.com>,
Michal Hocko <mhocko@suse.com>,
Ankur Arora <ankur.a.arora@oracle.com>,
Pedro Falcato <pfalcato@suse.de>,
Peter Zijlstra <peterz@infradead.org>,
Borislav Petkov <bp@alien8.de>, Bharata B Rao <bharata@amd.com>,
Hrushikesh Salunke <hsalunke@amd.com>,
K Prateek Nayak <kprateek.nayak@amd.com>,
x86@kernel.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] mm/memory: drop PROCESS_PAGES_NON_PREEMPT_BATCH
Date: Tue, 10 Nov 2026 15:06:58 +0000 [thread overview]
Message-ID: <avMxawZfHRAqgKA4@gremlin> (raw)
In-Reply-To: <20261109-clear-pages-batch-v3-1-6693efe18dff@amd.com>
On Mon, Nov 09, 2026 at 07:15:51PM +0000, Shivank Garg wrote:
> clear_contig_highpages() clears the whole range at once under
> preemptible models. Otherwise it clears PROCESS_PAGES_NON_PREEMPT_BATCH
> pages at a time, with a cond_resched() in between. The batch is 32MB if
> the architecture defines clear_pages(), and one page otherwise.
>
> Since commit 7dadeaa6e851 ("sched: Further restrict the preemption
> modes"), architectures that support lazy preemption only offer the
> full, lazy and RT preemption models, also with PREEMPT_DYNAMIC. x86 is
> one of them, and it is the only architecture that defines
> clear_pages(). So the 32MB batch is never used: whenever
> clear_contig_highpages() runs under a cooperative model, the batch is
> one page.
>
> Make that explicit. Clear the whole range when CONFIG_PREEMPTION is
> set, and one page at a time with cond_resched() in between otherwise.
> Drop PROCESS_PAGES_NON_PREEMPT_BATCH, which has no other users.
>
> PREEMPT_DYNAMIC kernels also have CONFIG_PREEMPTION set, and
> cond_resched() does nothing on any of these kernels. So a build-time
> CONFIG_PREEMPTION check is enough, and preempt_model_preemptible() is
> not needed.
>
> No functional change intended.
>
> Suggested-by: David Hildenbrand <david@kernel.org>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
Nice cleanup! LGTM so:
Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> This came up in review of the copy_mc_highpages() series [1]. Pedro
> pointed out that most architecture run with full or lazy preempt, so mm
> does not need to check the preemption model at runtime or batch the copy
> for cond_resched(), and a simple per-page fallback is enough for
> non-preemptible models.
> David suggested doing the same for the zeroing path, and this patch
> does that.
>
> [1] https://lore.kernel.org/all/20261019-batpagecopy-v2-1-d75e7d0dfaae@amd.com
> ---
> Changes in v3:
> - Use clear_user_highpage() in the per-page loop (Pedro).
> - rebase on mm/core.git/for-next.
> - Link to v2: https://lore.kernel.org/r/20261109-clear-pages-batch-v2-1-2e62c4f84f94@amd.com
>
> Changes in v2:
> - Rebased on mm/linux.git/for-next, and drop RFC tag.
> - Link to v1: https://lore.kernel.org/r/20261022-clear-pages-batch-v1-1-13d3bd0b876e@amd.com
> ---
> include/linux/mm.h | 28 +---------------------------
> mm/memory.c | 29 ++++++++++++++++-------------
> 2 files changed, 17 insertions(+), 40 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index e6da76fbe8e8..8d14e45fcf24 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -5250,7 +5250,7 @@ static inline void clear_page_guard(struct zone *zone, struct page *page,
> *
> * When running under preemptible models this is not a problem. Under
> * cooperatively scheduled models, however, the caller is expected to
> - * limit @npages to no more than PROCESS_PAGES_NON_PREEMPT_BATCH.
> + * clear one page at a time and call cond_resched() in between.
> */
> static inline void clear_pages(void *addr, unsigned int npages)
> {
> @@ -5261,32 +5261,6 @@ static inline void clear_pages(void *addr, unsigned int npages)
> }
> #endif
>
> -#ifndef PROCESS_PAGES_NON_PREEMPT_BATCH
> -#ifdef clear_pages
> -/*
> - * The architecture defines clear_pages(), and we assume that it is
> - * generally "fast". So choose a batch size large enough to allow the processor
> - * headroom for optimizing the operation and yet small enough that we see
> - * reasonable preemption latency for when this optimization is not possible
> - * (ex. slow microarchitectures, memory bandwidth saturation.)
> - *
> - * With a value of 32MB and assuming a memory bandwidth of ~10GBps, this should
> - * result in worst case preemption latency of around 3ms when clearing pages.
> - *
> - * (See comment above clear_pages() for why preemption latency is a concern
> - * here.)
> - */
> -#define PROCESS_PAGES_NON_PREEMPT_BATCH (SZ_32M >> PAGE_SHIFT)
> -#else /* !clear_pages */
> -/*
> - * The architecture does not provide a clear_pages() implementation. Assume
> - * that clear_page() -- which clear_pages() will fallback to -- is relatively
> - * slow and choose a small value for PROCESS_PAGES_NON_PREEMPT_BATCH.
> - */
> -#define PROCESS_PAGES_NON_PREEMPT_BATCH 1
> -#endif
> -#endif
> -
Nice to remove this :)
> #ifdef __HAVE_ARCH_GATE_AREA
> extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
> extern int in_gate_area_no_mm(unsigned long addr);
> diff --git a/mm/memory.c b/mm/memory.c
> index c9d64170e47a..fcc7ea2a00d1 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -7650,27 +7650,30 @@ static inline int process_huge_page(
> }
>
> static void clear_contig_highpages(struct page *page, unsigned long addr,
> - unsigned int nr_pages)
> + unsigned int nr_pages)
> {
> - unsigned int i, count;
> + unsigned int i;
> +
> + might_sleep();
> +
> /*
> * When clearing we want to operate on the largest extent possible to
> * allow for architecture specific extent based optimizations.
> *
> - * However, since clear_user_highpages() (and primitives clear_user_pages(),
> - * clear_pages()), do not call cond_resched(), limit the unit size when
> - * running under non-preemptible scheduling models.
> + * clear_user_highpages() (and primitives clear_user_pages(),
> + * clear_pages()) do not call cond_resched(). That is fine on
> + * preemptible kernels, where the scheduler can preempt the clearing
> + * at any point, so clear the whole range at once. Otherwise, clear
> + * one page at a time and reschedule in between.
> */
> - const unsigned int unit = preempt_model_preemptible() ?
> - nr_pages : PROCESS_PAGES_NON_PREEMPT_BATCH;
> -
> - might_sleep();
> + if (IS_ENABLED(CONFIG_PREEMPTION)) {
> + clear_user_highpages(page, addr, nr_pages);
> + return;
> + }
>
> - for (i = 0; i < nr_pages; i += count) {
> + for (i = 0; i < nr_pages; i++) {
> cond_resched();
> -
> - count = min(unit, nr_pages - i);
> - clear_user_highpages(page + i, addr + i * PAGE_SIZE, count);
> + clear_user_highpage(page + i, addr + i * PAGE_SIZE);
> }
Very lovely :)
> }
>
>
> ---
> base-commit: 2683df3b7eac1d91380194955ad0c9751454610c
> change-id: 20261021-clear-pages-batch-77f72ef808e4
>
> Best regards,
> --
> Shivank Garg <shivankg@amd.com>
>
--
Cheers, Lorenzo
next prev parent reply other threads:[~2026-11-10 15:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-09 19:15 [PATCH v3] mm/memory: drop PROCESS_PAGES_NON_PREEMPT_BATCH Shivank Garg
2026-11-09 22:00 ` David Hildenbrand (Arm)
2026-11-10 4:16 ` Garg, Shivank
2026-11-10 15:06 ` Lorenzo Stoakes (ARM) [this message]
2026-11-11 9:30 ` David Hildenbrand
2026-11-11 12:40 ` Peter Zijlstra
2026-11-11 13:12 ` Lorenzo Stoakes (ARM)
2026-11-11 13:21 ` Peter Zijlstra
2026-11-11 19:07 ` Garg, Shivank
2026-11-11 20:58 ` David Hildenbrand (Arm)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=avMxawZfHRAqgKA4@gremlin \
--to=ljs@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=ankur.a.arora@oracle.com \
--cc=bharata@amd.com \
--cc=bp@alien8.de \
--cc=david@kernel.org \
--cc=hsalunke@amd.com \
--cc=kprateek.nayak@amd.com \
--cc=liam@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=peterz@infradead.org \
--cc=pfalcato@suse.de \
--cc=rppt@kernel.org \
--cc=shivankg@amd.com \
--cc=surenb@google.com \
--cc=vbabka@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.