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@xxxxxxxxxxxxxxx>
>> > Acked-by: Mike Rapoport <rppt@xxxxxxxxxxxxx> # memblock
>>
>> Reviewed-by: Vlastimil Babka <vbabka@xxxxxxx>
>
> 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();
