linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Zi Yan <ziy@nvidia.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Mel Gorman <mgorman@techsingularity.net>,
	David Hildenbrand <david@redhat.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Oscar Salvador <osalvador@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux-MM <linux-mm@kvack.org>
Subject: Re: [BUG] Crash on x86_32 for: mm: page_alloc: avoid merging non-fallbackable pageblocks with others
Date: Wed, 30 Mar 2022 17:43:05 -0400	[thread overview]
Message-ID: <733F211D-9717-46A7-A0A2-40353E12F65A@nvidia.com> (raw)
In-Reply-To: <AF9D3EF9-B070-4ADE-A2D3-C466A4A2D125@nvidia.com>

[-- Attachment #1: Type: text/plain, Size: 2541 bytes --]

On 30 Mar 2022, at 17:25, Zi Yan wrote:

> On 30 Mar 2022, at 16:53, Steven Rostedt wrote:
>
>> On Wed, 30 Mar 2022 16:29:28 -0400
>> Zi Yan <ziy@nvidia.com> wrote:
>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index bdc8f60ae462..83a90e2973b7 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -1108,6 +1108,8 @@ static inline void __free_one_page(struct page *page,
>>>
>>>                 buddy_pfn = __find_buddy_pfn(pfn, order);
>>>                 buddy = page + (buddy_pfn - pfn);
>>> +               if (!page_is_buddy(page, buddy, order))
>>> +                       goto done_merging;
>>>                 buddy_mt = get_pageblock_migratetype(buddy);
>>>
>>>                 if (migratetype != buddy_mt
>>>
>>
>> The above did not apply to Linus's tree, nor even the problem commit
>> (before or after), but I found where the code is, and added it manually.
>>
>> It does appear to allow the machine to boot.
>>
> I just pulled Linus’s tree and grabbed the diff. Anyway, thanks.
>
> I would like to get more understanding of the issue before blindly sending
> this as a fix.
>
> Merge the other thread:
>>
>> Not sure if this matters or not, but my kernel command line has:
>>
>>   crashkernel=256M
>>
>> Could that have caused this to break?
>
> Unlikely, 256MB is MAX_ORDER_NR_PAGES aligned (MAX_ORDER is 11 here).
> __find_buddy_pfn() will not get any buddy_pfn from crashkernel memory
> region, since that would cross MAX_ORDER_NR_PAGES boundary.
>
> page_is_buddy() checks page_is_guard(buddy), PageBuddy(buddy),
> buddy_order(buddy), and page_zone_id(buddy), where page_is_guard(buddy)
> is always false since CONFIG_DEBUG_PAGEALLOC is not set in your config.
> So either PageBuddy(buddy) is false, buddy_order(buddy) != order,
> or page_zone_id(buddy) is not the same as page_zone_id(page).
>
> Do you mind adding the following code right before my fix code above
> and provide a complete boot log? I would like to understand what
> went wrong. Thanks.
>
> pr_info("buddy_pfn: %lx, PageBuddy: %d, buddy_order: %d (vs %d), page_zone_id: %d (vs %d)\n",
>         buddy_pfn, PageBuddy(buddy), buddy_order(buddy), order, page_zone_id(buddy),
>         page_zone_id(page));
>
>

This seems to be a bug in the original code too.
But "if (unlikely(has_isolate_pageblock(zone)))" is too rare to trigger it.
I do not see how having isolated pageblocks in a zone could get us away
from checking page_is_buddy().

--
Best Regards,
Yan, Zi

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]

  reply	other threads:[~2022-03-30 21:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-30 19:42 Steven Rostedt
2022-03-30 20:05 ` Linus Torvalds
2022-03-30 20:29   ` Zi Yan
2022-03-30 20:53     ` Steven Rostedt
2022-03-30 21:25       ` Zi Yan
2022-03-30 21:43         ` Zi Yan [this message]
2022-03-30 21:48           ` Vlastimil Babka
2022-03-30 21:57         ` Steven Rostedt
2022-03-30 20:57     ` Steven Rostedt
2022-03-30 21:43   ` Vlastimil Babka
2022-03-30 22:03     ` Zi Yan
2022-03-30 22:15       ` Vlastimil Babka
2022-03-30 22:24         ` Zi Yan

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=733F211D-9717-46A7-A0A2-40353E12F65A@nvidia.com \
    --to=ziy@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=osalvador@suse.de \
    --cc=rostedt@goodmis.org \
    --cc=rppt@linux.ibm.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@suse.cz \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox