[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z_N5ps86xJmewe_P@kernel.org>
Date: Mon, 7 Apr 2025 10:07:18 +0300
From: Mike Rapoport <rppt@...nel.org>
To: David Woodhouse <dwmw2@...radead.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
"Sauerwein, David" <dssauerw@...zon.de>,
Anshuman Khandual <anshuman.khandual@....com>,
Ard Biesheuvel <ardb@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
David Hildenbrand <david@...hat.com>, Marc Zyngier <maz@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Mike Rapoport <rppt@...ux.ibm.com>, Will Deacon <will@...nel.org>,
kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [RFC PATCH v2 4/7] mm: Optimise SPARSEMEM implementation of
for_each_valid_pfn()
On Fri, Apr 04, 2025 at 04:59:56PM +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@...zon.co.uk>
>
> There's no point in checking the section and subsection bitmap for *every*
> PFN in the same section; they're either all valid or they aren't.
Don't you want to merge this with the previous commit?
> Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
> ---
> include/linux/mmzone.h | 39 +++++++++++++++++++++++++++++----------
> 1 file changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 67cdf675a4b9..0da1b0ba5d9f 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -2154,21 +2154,20 @@ static inline int pfn_valid(unsigned long pfn)
> return ret;
> }
>
> -static inline bool first_valid_pfn(unsigned long *p_pfn)
> +/* Returns -1 (an invalid PFN) if no valid PFN remaining */
> +static inline unsigned long first_valid_pfn(unsigned long pfn, unsigned long end_pfn)
> {
> - unsigned long pfn = *p_pfn;
> unsigned long nr = pfn_to_section_nr(pfn);
>
> rcu_read_lock_sched();
>
> - while (nr <= __highest_present_section_nr) {
> + while (nr <= __highest_present_section_nr && pfn < end_pfn) {
> struct mem_section *ms = __pfn_to_section(pfn);
>
> if (valid_section(ms) &&
> (early_section(ms) || pfn_section_first_valid(ms, &pfn))) {
> - *p_pfn = pfn;
> rcu_read_unlock_sched();
> - return true;
> + return pfn;
> }
>
> /* Nothing left in this section? Skip to next section */
> @@ -2177,14 +2176,34 @@ static inline bool first_valid_pfn(unsigned long *p_pfn)
> }
>
> rcu_read_unlock_sched();
> + return (unsigned long)-1;
> +}
>
> - return false;
> +static inline unsigned long next_valid_pfn(unsigned long pfn, unsigned long end_pfn)
> +{
> + pfn++;
> +
> + if (pfn >= end_pfn)
> + return (unsigned long)-1;
> +
> + /*
> + * Either every PFN within the section (or subsection for VMEMMAP) is
> + * valid, or none of them are. So there's no point repeating the check
> + * for every PFN; only call first_valid_pfn() the first time, and when
> + * crossing a (sub)section boundary (i.e. !(pfn & ~PFN_VALID_MASK)).
> + */
> + if (pfn & (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP) ?
> + PAGE_SUBSECTION_MASK : PAGE_SECTION_MASK))
> + return pfn;
> +
> + return first_valid_pfn(pfn, end_pfn);
> }
>
> -#define for_each_valid_pfn(_pfn, _start_pfn, _end_pfn) \
> - for ((_pfn) = (_start_pfn); \
> - first_valid_pfn(&(_pfn)) && (_pfn) < (_end_pfn); \
> - (_pfn)++)
> +
> +#define for_each_valid_pfn(_pfn, _start_pfn, _end_pfn) \
> + for ((_pfn) = first_valid_pfn((_start_pfn), (_end_pfn)); \
> + (_pfn) != (unsigned long)-1; \
> + (_pfn) = next_valid_pfn((_pfn), (_end_pfn)))
>
> #endif
>
> --
> 2.49.0
>
--
Sincerely yours,
Mike.
Powered by blists - more mailing lists