Re: [PATCHv9 02/14] mm: Add support for unaccepted memory

From: Vlastimil Babka <vbabka@suse.cz>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
	Sean Christopherson <seanjc@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Joerg Roedel <jroedel@suse.de>, Ard Biesheuvel <ardb@kernel.org>,
	Andi Kleen <ak@linux.intel.com>,
	Kuppuswamy Sathyanarayanan
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	David Rientjes <rientjes@google.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ingo Molnar <mingo@redhat.com>,
	Dario Faggioli <dfaggioli@suse.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Mike Rapoport <rppt@kernel.org>,
	David Hildenbrand <david@redhat.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	marcelo.cerri@canonical.com, tim.gardner@canonical.com,
	khalid.elmously@canonical.com, philip.cox@canonical.com,
	aarcange@redhat.com, peterx@redhat.com, x86@kernel.org,
	linux-mm@kvack.org, linux-coco@lists.linux.dev,
	linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
	Mike Rapoport <rppt@linux.ibm.com>
Subject: Re: [PATCHv9 02/14] mm: Add support for unaccepted memory
Date: Mon, 3 Apr 2023 15:07:33 +0200	[thread overview]
Message-ID: <2a8278cf-d282-ada6-0161-3b9c6adcb12e@suse.cz> (raw)
In-Reply-To: <20230403100244.4sre7ki4ccusiiul@box>

On 4/3/23 12:02, Kirill A. Shutemov wrote:
> On Mon, Apr 03, 2023 at 11:26:53AM +0200, Vlastimil Babka wrote:
>> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> > Acked-by: Mike Rapoport <rppt@linux.ibm.com>	# memblock
>> 
>> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> 
> Thanks!
> 
>> Just a small suggestion below:
>> 
>> > +
>> > +static bool try_to_accept_memory(struct zone *zone, unsigned int order)
>> > +{
>> > +	long to_accept;
>> > +	int ret = false;
>> > +
>> > +	if (!static_branch_unlikely(&zones_with_unaccepted_pages))
>> > +		return false;
>> 
>> 
>> This potentially (depends on what compiler decides) means we'll call this
>> function just to skip the static branch. OTOH forcing it as inline would be
>> wasteful too. So I'd split that away and make the callers do that static
>> branch check inline. Just as deferred_pages_enabled() is used.
> 
> Like this?

Yep.

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 07e16e9b49c4..80fe5e4b6cca 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -390,6 +390,7 @@ EXPORT_SYMBOL(nr_online_nodes);
>  static bool page_contains_unaccepted(struct page *page, unsigned int order);
>  static void accept_page(struct page *page, unsigned int order);
>  static bool try_to_accept_memory(struct zone *zone, unsigned int order);
> +static inline bool has_unaccepted_memory(void);
>  static bool __free_unaccepted(struct page *page);
>  
>  int page_group_by_mobility_disabled __read_mostly;
> @@ -3464,8 +3465,10 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  				       gfp_mask)) {
>  			int ret;
>  
> -			if (try_to_accept_memory(zone, order))
> -				goto try_this_zone;
> +			if (has_unaccepted_memory()) {
> +				if (try_to_accept_memory(zone, order))
> +					goto try_this_zone;
> +			}
>  
>  #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
>  			/*
> @@ -3519,8 +3522,10 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags,
>  
>  			return page;
>  		} else {
> -			if (try_to_accept_memory(zone, order))
> -				goto try_this_zone;
> +			if (has_unaccepted_memory()) {
> +				if (try_to_accept_memory(zone, order))
> +					goto try_this_zone;
> +			}
>  
>  #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
>  			/* Try again if zone has deferred pages */
> @@ -7302,9 +7307,6 @@ static bool try_to_accept_memory(struct zone *zone, unsigned int order)
>  	long to_accept;
>  	int ret = false;
>  
> -	if (!static_branch_unlikely(&zones_with_unaccepted_pages))
> -		return false;
> -
>  	/* How much to accept to get to high watermark? */
>  	to_accept = high_wmark_pages(zone) -
>  		    (zone_page_state(zone, NR_FREE_PAGES) -
> @@ -7321,6 +7323,11 @@ static bool try_to_accept_memory(struct zone *zone, unsigned int order)
>  	return ret;
>  }
>  
> +static inline bool has_unaccepted_memory(void)
> +{
> +	return static_branch_unlikely(&zones_with_unaccepted_pages);
> +}
> +
>  static bool __free_unaccepted(struct page *page)
>  {
>  	struct zone *zone = page_zone(page);
> @@ -7398,6 +7405,11 @@ static bool try_to_accept_memory(struct zone *zone, unsigned int order)
>  	return false;
>  }
>  
> +static inline bool has_unaccepted_memory(void)
> +{
> +	return false;
> +}
> +
>  static bool __free_unaccepted(struct page *page)
>  {
>  	BUILD_BUG();