From: Oreoluwa Babatunde <quic_obabatun@quicinc.com>
To: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
<ysato@users.sourceforge.jp>, <dalias@libc.org>
Cc: <akpm@linux-foundation.org>, <linux-sh@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <robh+dt@kernel.org>,
<kernel@quicinc.com>, Rob Herring <robh@kernel.org>,
Rob Landley <rob@landley.net>
Subject: Re: [PATCH v2] sh: Call paging_init() earlier in the init sequence
Date: Tue, 7 May 2024 14:42:05 -0700 [thread overview]
Message-ID: <ec5f3194-7e9e-4cc9-86b9-02a204649246@quicinc.com> (raw)
In-Reply-To: <b00e0adc72815e465cf32fc5505445cfceeeca84.camel@physik.fu-berlin.de>
On 5/2/2024 3:29 AM, John Paul Adrian Glaubitz wrote:
> Hi Oreoluwa,
>
> On Wed, 2024-05-01 at 19:18 +0200, John Paul Adrian Glaubitz wrote:
>> Hi Oreoluwa,
>>
>> On Tue, 2024-04-23 at 16:31 -0700, Oreoluwa Babatunde wrote:
>>> The unflatten_device_tree() function contains a call to
>>> memblock_alloc(). This is a problem because this allocation is done
>>> before any of the reserved memory is set aside in paging_init().
>>> This means that there is a possibility for memblock to allocate from
>>> any of the memory regions that are supposed to be set aside as reserved.
>>>
>>> Hence, move the call to paging_init() to be earlier in the init
>>> sequence so that the reserved memory regions are set aside before any
>>> allocations are done using memblock.
>> I was just about to merge your patch when I ran a git blame on the code in
>> arch/sh/kernel/setup.c and noticed the following commit by Rich Felker:
>>
>> commit eb6b6930a70faefe04479a71088cc10366782d9a
>> Author: Rich Felker <dalias@libc.org>
>> Date: Mon Jul 31 01:27:50 2017 -0400
>>
>> sh: fix memory corruption of unflattened device tree
>>
>> unflatten_device_tree() makes use of memblock allocation, and
>> therefore must be called before paging_init() migrates the memblock
>> allocation data to the bootmem framework. Otherwise the record of the
>> allocation for the expanded device tree will be lost, and will
>> eventually be clobbered when allocated for another use.
>>
>> Signed-off-by: Rich Felker <dalias@libc.org>
>>
>> It looks like that the call to unflatten_device_tree() before paging_init()
>> is intentional and needed for the device tree to be preserved in memory
>> after running paging_init().
Hi John,
Thank you for pointing this out.
memblock_alloc() marks all its allocations as reserved by calling
memblock_reserve().
https://elixir.bootlin.com/linux/latest/source/mm/memblock.c#L1463
This should normally stop other users from allocating from within that
region of memory.
But in this case, since all the free memory regions have already been
transferred over to the bootmem framework by paging_init(), I am not
sure if that logic will still hold for the unflatten_deivcetree allocated memory.
The main goal of this patch is to make sure that the reserved memory
regions defined in the DT are set aside before any memblock allocations
are done (which includes the allocation done by unflatten_devicetree).
Hence, I can restructure the patch to only remove the portion of code that is
is responsible for setting aside the DT defined reserved memory regions from
within paging_init(), and move it above the unflatten_devicetree() call.
https://elixir.bootlin.com/linux/latest/source/arch/sh/mm/init.c#L292
I will explore further and possibly restructure this patch based on my findings.
Thank you!
Oreoluwa
>>
>> @Geert: Do you have any comments on this patch?
>> @Rob: Could you test this patch on your J2 board and report back?
next prev parent reply other threads:[~2024-05-07 21:42 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-23 23:31 [PATCH v2] sh: Call paging_init() earlier in the init sequence Oreoluwa Babatunde
2024-04-24 4:47 ` John Paul Adrian Glaubitz
2024-04-24 8:45 ` Markus Elfring
2024-04-24 10:24 ` John Paul Adrian Glaubitz
2024-04-24 11:06 ` [v2] " Markus Elfring
2024-04-29 9:03 ` [PATCH v2] " John Paul Adrian Glaubitz
2024-04-29 16:28 ` Oreoluwa Babatunde
2024-04-29 17:26 ` John Paul Adrian Glaubitz
2024-04-29 17:54 ` Oreoluwa Babatunde
2024-05-01 17:18 ` John Paul Adrian Glaubitz
2024-05-02 10:29 ` John Paul Adrian Glaubitz
2024-05-07 21:42 ` Oreoluwa Babatunde [this message]
2024-05-07 22:41 ` John Paul Adrian Glaubitz
2024-05-20 18:03 ` Oreoluwa Babatunde
2024-05-20 18:24 ` John Paul Adrian Glaubitz
2024-05-20 18:49 ` Oreoluwa Babatunde
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=ec5f3194-7e9e-4cc9-86b9-02a204649246@quicinc.com \
--to=quic_obabatun@quicinc.com \
--cc=akpm@linux-foundation.org \
--cc=dalias@libc.org \
--cc=glaubitz@physik.fu-berlin.de \
--cc=kernel@quicinc.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=rob@landley.net \
--cc=robh+dt@kernel.org \
--cc=robh@kernel.org \
--cc=ysato@users.sourceforge.jp \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).