| Message ID | 20190119132650.9978-7-anup.patel@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixmap support and MM cleanups | expand |
On Sat, Jan 19, 2019 at 01:28:59PM +0000, Anup Patel wrote: > This patch implements keepinitrd kernel parameter. By default, > keepinitrd=1 but users can pass "keepinitrd=0" to free-up > initrd memory at boot-time in free_initrd_mem() function. > > The keepinitrd kernel parameter is already implemented by > unicore32, arm, and arm64 architectures and it is documented > at: Documentation/admin-guide/kernel-parameters.txt But why do we need it? Is there any good reason every not to free the initrd / initramfs memory when it is not used?
On Tue, Feb 12, 2019 at 12:38 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Sat, Jan 19, 2019 at 01:28:59PM +0000, Anup Patel wrote: > > This patch implements keepinitrd kernel parameter. By default, > > keepinitrd=1 but users can pass "keepinitrd=0" to free-up > > initrd memory at boot-time in free_initrd_mem() function. > > > > The keepinitrd kernel parameter is already implemented by > > unicore32, arm, and arm64 architectures and it is documented > > at: Documentation/admin-guide/kernel-parameters.txt > > But why do we need it? Is there any good reason every not to free > the initrd / initramfs memory when it is not used? If it is initramfs (i.e. CPIO image) then contents of CPIO archive are extracted to create a ramfs instance. If it is initrd (i.e. some filesystem image) then RAM block device is created in-place at initrd location. (Please correct me if I am wrong about initrd here). So in case of initrd we might not want to free-up the RAM but we can certainly free-up in case of initramfs. Regards, Anup
On Feb 12 2019, Anup Patel <anup@brainfault.org> wrote: > So in case of initrd we might not want to free-up the RAM but > we can certainly free-up in case of initramfs. But the default should be keepinitrd=0, shoudn't it? Andreas.
On Tue, Feb 12, 2019 at 4:07 PM Andreas Schwab <schwab@suse.de> wrote: > > On Feb 12 2019, Anup Patel <anup@brainfault.org> wrote: > > > So in case of initrd we might not want to free-up the RAM but > > we can certainly free-up in case of initramfs. > > But the default should be keepinitrd=0, shoudn't it? Actually, it is keepinitrd=0 by default but the commit description is reverse. I will fix the commit description. Regards, Anup
On Tue, Feb 12, 2019 at 03:53:21PM +0530, Anup Patel wrote: > If it is initramfs (i.e. CPIO image) then contents of CPIO archive > are extracted to create a ramfs instance. > > If it is initrd (i.e. some filesystem image) then RAM block device > is created in-place at initrd location. (Please correct me if I am > wrong about initrd here). No. If it is an initrd image we still copy it into the rootfs first, and then load it into a ram disk. Take a look at init/initramfs.c:populate_rootfs() and init/do_mounts_initrd.c:initrd_load(). > So in case of initrd we might not want to free-up the RAM but > we can certainly free-up in case of initramfs. No, in either case we do not need the original initramfs/initrd memory. I suspect arm has this as a workaround for some weird legacy boot issue, but I can't see any reason why we would not want to free the memory on riscv.
On Sat, 19 Jan 2019 05:28:59 PST (-0800), Anup.Patel@wdc.com wrote: > This patch implements keepinitrd kernel parameter. By default, > keepinitrd=1 but users can pass "keepinitrd=0" to free-up > initrd memory at boot-time in free_initrd_mem() function. > > The keepinitrd kernel parameter is already implemented by > unicore32, arm, and arm64 architectures and it is documented > at: Documentation/admin-guide/kernel-parameters.txt All I see is " keepinitrd [HW,ARM] " which is pretty lacking for documentation and should be improved... I'm happy to take this as it stands for the next merge window, as if you hadn't mentioned documentation then I wouldn't have looked :) > Signed-off-by: Anup Patel <anup.patel@wdc.com> > --- > arch/riscv/kernel/setup.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c > index 9cd583b6d1cd..46e547dd8245 100644 > --- a/arch/riscv/kernel/setup.c > +++ b/arch/riscv/kernel/setup.c > @@ -97,8 +97,20 @@ static void __init setup_initrd(void) > initrd_end = 0; > } > > -void free_initrd_mem(unsigned long start, unsigned long end) > +static int keep_initrd __initdata; > + > +static int __init keepinitrd_setup(char *__unused) > +{ > + keep_initrd = 1; > + return 1; > +} > + > +__setup("keepinitrd", keepinitrd_setup); > + > +void __init free_initrd_mem(unsigned long start, unsigned long end) > { > + if (!keep_initrd) > + memblock_free(__pa(start), end - start); > } > #endif /* CONFIG_BLK_DEV_INITRD */ > > -- > 2.17.1
> -----Original Message----- > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- > owner@vger.kernel.org] On Behalf Of Christoph Hellwig > Sent: Wednesday, February 13, 2019 12:15 AM > To: Anup Patel <anup@brainfault.org> > Cc: Christoph Hellwig <hch@infradead.org>; Palmer Dabbelt > <palmer@sifive.com>; Anup Patel <Anup.Patel@wdc.com>; linux- > kernel@vger.kernel.org; Atish Patra <Atish.Patra@wdc.com>; Albert Ou > <aou@eecs.berkeley.edu>; Paul Walmsley <paul.walmsley@sifive.com>; > linux-riscv@lists.infradead.org > Subject: Re: [PATCH v2 6/6] RISC-V: Implement keepinitrd kernel parameter > > On Tue, Feb 12, 2019 at 03:53:21PM +0530, Anup Patel wrote: > > If it is initramfs (i.e. CPIO image) then contents of CPIO archive are > > extracted to create a ramfs instance. > > > > If it is initrd (i.e. some filesystem image) then RAM block device is > > created in-place at initrd location. (Please correct me if I am wrong > > about initrd here). > > No. If it is an initrd image we still copy it into the rootfs first, and then load it > into a ram disk. Take a look at > init/initramfs.c:populate_rootfs() and > init/do_mounts_initrd.c:initrd_load(). > > > So in case of initrd we might not want to free-up the RAM but we can > > certainly free-up in case of initramfs. > > No, in either case we do not need the original initramfs/initrd memory. I > suspect arm has this as a workaround for some weird legacy boot issue, but I > can't see any reason why we would not want to free the memory on riscv. Sure, the keepinitrd=0 by default so it will always free-up initrd by default. Please look at v3 patchset. Of course, we need separate patch to update documentation of keepinitrd. Regards, Anup
On Wed, Feb 13, 2019 at 03:43:06AM +0000, Anup Patel wrote: > Sure, the keepinitrd=0 by default so it will always free-up initrd by default. > Please look at v3 patchset. > > Of course, we need separate patch to update documentation of keepinitrd. No, we need to not just blindly copy what arm did for historic reasons unless we have a very good reason of our own. In addition to not having a real need for any of this in a new setup, it also is duplicated by the retain_initrd parameter which is implemented in generic code - and for that the commit message at least has a rationale related to kexec: https://lkml.org/lkml/2006/12/7/15
> -----Original Message----- > From: Christoph Hellwig [mailto:hch@infradead.org] > Sent: Wednesday, February 13, 2019 11:25 AM > To: Anup Patel <Anup.Patel@wdc.com> > Cc: Christoph Hellwig <hch@infradead.org>; Anup Patel > <anup@brainfault.org>; Palmer Dabbelt <palmer@sifive.com>; linux- > kernel@vger.kernel.org; Atish Patra <Atish.Patra@wdc.com>; Albert Ou > <aou@eecs.berkeley.edu>; Paul Walmsley <paul.walmsley@sifive.com>; > linux-riscv@lists.infradead.org > Subject: Re: [PATCH v2 6/6] RISC-V: Implement keepinitrd kernel parameter > > On Wed, Feb 13, 2019 at 03:43:06AM +0000, Anup Patel wrote: > > Sure, the keepinitrd=0 by default so it will always free-up initrd by default. > > Please look at v3 patchset. > > > > Of course, we need separate patch to update documentation of > keepinitrd. > > No, we need to not just blindly copy what arm did for historic reasons unless > we have a very good reason of our own. > > In addition to not having a real need for any of this in a new setup, it also is > duplicated by the retain_initrd parameter which is implemented in generic > code - and for that the commit message at least has a rationale related to > kexec: > > https://lkml.org/lkml/2006/12/7/15 Sure, I will re-work this patch to always free-up initrd. Regards, Anup
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 9cd583b6d1cd..46e547dd8245 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -97,8 +97,20 @@ static void __init setup_initrd(void) initrd_end = 0; } -void free_initrd_mem(unsigned long start, unsigned long end) +static int keep_initrd __initdata; + +static int __init keepinitrd_setup(char *__unused) +{ + keep_initrd = 1; + return 1; +} + +__setup("keepinitrd", keepinitrd_setup); + +void __init free_initrd_mem(unsigned long start, unsigned long end) { + if (!keep_initrd) + memblock_free(__pa(start), end - start); } #endif /* CONFIG_BLK_DEV_INITRD */
This patch implements keepinitrd kernel parameter. By default, keepinitrd=1 but users can pass "keepinitrd=0" to free-up initrd memory at boot-time in free_initrd_mem() function. The keepinitrd kernel parameter is already implemented by unicore32, arm, and arm64 architectures and it is documented at: Documentation/admin-guide/kernel-parameters.txt Signed-off-by: Anup Patel <anup.patel@wdc.com> --- arch/riscv/kernel/setup.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)