Philipp Rudo <prudo(a)redhat.com> 于2022年4月27日周三 19:37写道:
Hi Kairui,
On Tue, 26 Apr 2022 22:30:03 +0800
Kairui Song <ryncsn(a)gmail.com> wrote:
> Philipp Rudo <prudo(a)redhat.com> 于2022年4月25日周一 21:18写道:
> >
> > Hi Kairui,
> >
> > great idea!
>
> Thanks!
>
> >
> > A few comments/questions below.
> >
> > On Sat, 23 Apr 2022 22:49:52 +0800
> > Kairui Song <ryncsn(a)gmail.com> wrote:
> >
> > > From: Kairui Song <kasong(a)tencent.com>
> > >
> > > These kdump.sysconfig.* files are almost identical with a bit different
> > > in several parameters, just use a simple script to generate them upon
> > > packaging. This should make it easier to maintain, updating a comment or
> > > param for certain arch can be done in one place.
> > >
> > > There are some minor differences with the generated version due to some
> > > arch's sysconfig is not up-to-dated, this actually fixed the issues,
> > > to check these differences:
> > >
> > > # for arch in aarch64 i386 ppc64 ppc64le s390x x86_64; do
> > > ./kdump-gen-sysconfig.sh kdump.sysconfig.template $arch >
kdump.sysconfig.$arch.new
> > > git checkout HEAD^ kdump.sysconfig.$arch &>/dev/null
> > > echo "Diff of arch $arch:"
> > > diff kdump.sysconfig.$arch kdump.sysconfig.$arch.new; echo
""
> > > done; git reset;
> > >
> > > Diff of arch aarch64:
> > > > #What is the images extension. Relocatable kernels don't have
one
> > > > KDUMP_IMG_EXT=""
> > >
> > > Diff of arch i386:
> > >
> > > Diff of arch ppc64:
> > > 42,43d41
> > > < #Specify the action after failure
> > > <
> > >
> > > Diff of arch ppc64le:
> > > 42,43d41
> > > < #Specify the action after failure
> > > <
> > >
> > > Diff of arch s390x:
> > > 26,28d25
> > > < # Any additional /sbin/mkdumprd arguments required.
> > > < MKDUMPRD_ARGS=""
> > > <
> > >
> > > Diff of arch x86_64:
> > >
> > > Signed-off-by: Kairui Song <kasong(a)tencent.com>
> > > ---
> > > kdump-gen-sysconfig.sh | 51 ++++++++++++++++++
> > > kdump.sysconfig.aarch64 | 53 ------------------
> > > kdump.sysconfig.i386 | 56 -------------------
> > > kdump.sysconfig.ppc64 | 58 --------------------
> > > kdump.sysconfig.ppc64le | 58 --------------------
> > > kdump.sysconfig.s390x | 59 ---------------------
> > > kdump.sysconfig => kdump.sysconfig.template | 3 ++
> > > kdump.sysconfig.x86_64 | 56 -------------------
> > > kexec-tools.spec | 18 +++----
> > > 9 files changed, 60 insertions(+), 352 deletions(-)
> > > create mode 100755 kdump-gen-sysconfig.sh
> > > delete mode 100644 kdump.sysconfig.aarch64
> > > delete mode 100644 kdump.sysconfig.i386
> > > delete mode 100644 kdump.sysconfig.ppc64
> > > delete mode 100644 kdump.sysconfig.ppc64le
> > > delete mode 100644 kdump.sysconfig.s390x
> > > rename kdump.sysconfig => kdump.sysconfig.template (96%)
> > > delete mode 100644 kdump.sysconfig.x86_64
> > >
> > > diff --git a/kdump-gen-sysconfig.sh b/kdump-gen-sysconfig.sh
> > > new file mode 100755
> > > index 0000000..1eac80d
> > > --- /dev/null
> > > +++ b/kdump-gen-sysconfig.sh
> > > @@ -0,0 +1,51 @@
> > > +#!/bin/bash
> > > +
> > > +TEMPLATE=$1
> > > +ARCH=$2
> > > +SED_EXP=""
> > > +
> > > +update_param()
> > > +{
> > > + SED_EXP="${SED_EXP}s/^$1=.*$/$1=\"$2\"/;"
> > > +}
> > > +
> > > +case "$ARCH" in
> > > +aarch64)
> > > + update_param KEXEC_ARGS "-s"
> > > + update_param KDUMP_COMMANDLINE_APPEND \
> > > + "irqpoll nr_cpus=1 reset_devices cgroup_disable=memory
udev.children-max=2 panic=10 swiotlb=noforce novmcoredd cma=0 hugetlb_cma=0"
> >
> > hmm... in the end KDUMP_COMMANDLINE_APPEND is set for every (supported)
> > arch. So having a "default" set in the template currently doesn't
make
> > much sense. Furthermore the command line has quite a few common entries
> > between the arches. So how about splitting update_param into two
> > functions param_add and param_remove that add remove the given params
> > from the default set in the template. With that I hope it will be easier
> > to see the differences between the arches.
>
> OK, I'll update it, I thought it's better to just keep the code simple
> enough, a few redundant config lines won't hurt. But after second
> thought, two helpers will definately make the script cleaner.
>
> >
> > BTW, is there a difference between nr_cpus=1 and maxcpus=1?
>
> Hmm, I remember there were some discussions about these two params
> some time ago but I can't recall the detail...
>
> Look at the kernel-doc:
> nr_cpus= [SMP]
> Maximum number of processors that an SMP kernel
> could support. nr_cpus=n : n >= 1 limits the kernel to
> support 'n' processors. It could be larger than the
> number of already plugged CPU during bootup, later in
> runtime you can physically add extra cpu until it reaches
> n. So during boot up some boot time memory for per-cpu
> variables need be pre-allocated for later physical cpu
> hot plugging.
>
> maxcpus= [SMP]
> Maximum number of processors that an SMP kernel
> will bring up during bootup. maxcpus=n : n >= 0 limits
> the kernel to bring up 'n' processors. Surely after
> bootup you can bring up the other plugged cpu by executing
> "echo 1 > /sys/devices/system/cpu/cpuX/online". So maxcpus
> only takes effect during system bootup.
> While n=0 is a special case, it is equivalent to "nosmp",
> which also disables the IO APIC.
>
> So nr_cpus is a hard limit and saves percpu memory, maxcpus is not.
Right. But then it's probably better to go with nr_cpus. At least then
we don't allocate memory for cpus that will never go online. As far as
I see only the default and ppc64(le) are using maxcpus. All other
architectures already use nr_cpus.
> The doc also mentioned this special value maxcpus=0, just curious has
> anyone tried that before with kdump?
I've never tried that...
> >
> > > + ;;
> > > +i386)
> > > + update_param KDUMP_COMMANDLINE_APPEND \
> > > + "irqpoll nr_cpus=1 reset_devices numa=off
udev.children-max=2 panic=10 transparent_hugepage=never novmcoredd cma=0
hugetlb_cma=0"
> > > + ;;
> > > +ppc64)
> > > + update_param KDUMP_COMMANDLINE_REMOVE \
> > > + "hugepages hugepagesz slub_debug quiet log_buf_len
swiotlb hugetlb_cma"
> > > + update_param KDUMP_COMMANDLINE_APPEND \
> > > + "irqpoll maxcpus=1 noirqdistrib reset_devices
cgroup_disable=memory numa=off udev.children-max=2 ehea.use_mcs=0 panic=10
kvm_cma_resv_ratio=0 transparent_hugepage=never novmcoredd hugetlb_cma=0"
> > > + update_param KEXEC_ARGS "--dt-no-old-root"
> > > + ;;
> > > +ppc64le)
> > > + update_param KDUMP_COMMANDLINE_REMOVE \
> > > + "hugepages hugepagesz slub_debug quiet log_buf_len
swiotlb hugetlb_cma"
> > > + update_param KDUMP_COMMANDLINE_APPEND \
> > > + "irqpoll maxcpus=1 noirqdistrib reset_devices
cgroup_disable=memory numa=off udev.children-max=2 ehea.use_mcs=0 panic=10
kvm_cma_resv_ratio=0 transparent_hugepage=never novmcoredd hugetlb_cma=0"
> > > + update_param KEXEC_ARGS "--dt-no-old-root -s"
> > > + ;;
> > > +s390x)
> > > + update_param KDUMP_COMMANDLINE_REMOVE \
> > > + "hugepages hugepagesz slub_debug quiet log_buf_len
swiotlb vmcp_cma cma hugetlb_cma prot_virt"
> > > + update_param KDUMP_COMMANDLINE_APPEND \
> > > + "nr_cpus=1 cgroup_disable=memory numa=off
udev.children-max=2 panic=10 transparent_hugepage=never novmcoredd vmcp_cma=0 cma=0
hugetlb_cma=0"
> > > + update_param KEXEC_ARGS "-s"
> > > + ;;
> > > +x86_64)
> > > + update_param KDUMP_COMMANDLINE_APPEND \
> > > + "irqpoll nr_cpus=1 reset_devices cgroup_disable=memory
mce=off numa=off udev.children-max=2 panic=10 acpi_no_memhotplug
transparent_hugepage=never nokaslr hest_disable novmcoredd cma=0 hugetlb_cma=0"
> > > + update_param KEXEC_ARGS \
> > > + "-s"
> > > + ;;
> >
> > Add some error handling? I.e.
> >
> > *)
> > echo "Unknown architecture $ARCH provided" >&2
> > exit 1
> > ;;
>
> Maybe just ignoring other arches is fine I guess? The original
> behavior is fall back to the default template, this script didn't
> change the behavior.
ok, how about printing a warning and continue with the default?
When I was playing around with this I had a typo in the arch and it
took me ten minutes to figure out why it wasn't working. So the warning
would have been very appreciated ;-)
Yes, that's a great idea, I'll update it this way. Thanks for the review :)
>
> Thanks
> Philipp
>