[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACPcB9fWsOHGhyF-b4emTuoTUO4twbG6of=bruBYop4Jf4rzig@mail.gmail.com>
Date: Thu, 23 May 2019 19:04:36 +0800
From: Kairui Song <kasong@...hat.com>
To: Bhupesh Sharma <bhsharma@...hat.com>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Rahul Lakkireddy <rahul.lakkireddy@...lsio.com>,
"David S . Miller" <davem@...emloft.net>,
Eric Biederman <ebiederm@...ssion.com>,
Alexey Dobriyan <adobriyan@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Dave Young <dyoung@...hat.com>,
"kexec@...ts.infradead.org" <kexec@...ts.infradead.org>
Subject: Re: [PATCH v2] vmcore: Add a kernel cmdline vmcore_device_dump
On Thu, May 23, 2019 at 2:44 AM Bhupesh Sharma <bhsharma@...hat.com> wrote:
>
> On 05/20/2019 11:48 AM, Kairui Song wrote:
> > Since commit 2724273e8fd0 ('vmcore: add API to collect hardware dump in
> > second kernel'), drivers is allowed to add device related dump data to
> > vmcore as they want by using the device dump API. This have a potential
> > issue, the data is stored in memory, drivers may append too much data
> > and use too much memory. The vmcore is typically used in a kdump kernel
> > which runs in a pre-reserved small chunk of memory. So as a result it
> > will make kdump unusable at all due to OOM issues.
> >
> > So introduce new vmcore_device_dump= kernel parameter, and disable
> > device dump by default. User can enable it only if device dump data is
> > required for debugging, and have the chance to increase the kdump
> > reserved memory accordingly before device dump fails kdump.
> >
> > Signed-off-by: Kairui Song <kasong@...hat.com>
> > ---
> > Update from V1:
> > - Use bool parameter to turn it on/off instead of letting user give
> > the size limit. Size of device dump is hard to determine.
> >
> > Documentation/admin-guide/kernel-parameters.txt | 15 +++++++++++++++
> > fs/proc/vmcore.c | 13 +++++++++++++
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 43176340c73d..2d48e39fd080 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -5062,6 +5062,21 @@
> > decrease the size and leave more room for directly
> > mapped kernel RAM.
> >
> > + vmcore_device_dump=
> > + [VMCORE]
> > + Format: {"off" | "on"}
> > + If CONFIG_PROC_VMCORE_DEVICE_DUMP is set,
> > + this parameter allows enable or disable device dump
> > + for vmcore.
>
> We can add a simpler description here, something like:
> Depends on CONFIG_PROC_VMCORE_DEVICE_DUMP
>
> > + Device dump allows drivers to append dump data to
> > + vmcore so you can collect driver specified debug info.
> > + Note that the drivers could append the data without
> > + any limit, and the data is stored in memory, this may
> > + bring a significant memory stress. If you want to turn
> > + on this option, make sure you have reserved enough memory
> > + with crashkernel= parameter.
> > + default: off
>
> ... and massage the rest of text accordingly.
>
> Better to also modify the help text for 'PROC_VMCORE_DEVICE_DUMP' config
> option defined in 'fs/proc/Kconfig'. Something like:
>
> config PROC_VMCORE_DEVICE_DUMP
> bool "Device Hardware/Firmware Log Collection"
> <..snip..>
> If you say Y here, the collected device dumps will be added
> as ELF notes to /proc/vmcore.
>
> If this option is selected, device dump collection can still be
> disabled by passing vmcore_device_dump=off to the kernel.
>
> See config INTEL_IOMMU_DEFAULT_ON in 'drivers/iommu/Kconfig' as an example.
>
Good suggestion! I'll update in V3.
--
Best Regards,
Kairui Song
Powered by blists - more mailing lists