| Message ID | 20201002063414.275161-2-andrew@aj.id.au |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Enable pstore for Rainier | expand |
On Fri, 2 Oct 2020 at 06:35, Andrew Jeffery <andrew@aj.id.au> wrote: > > Reserve a 1MiB region of memory to record kmsg dumps and console state > into 16kiB ring-buffer slots. The sizing allows for up to 32 dumps to be > captured and read out. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > index e6f422edf454..46a0e95049fd 100644 > --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > @@ -47,6 +47,14 @@ reserved-memory { > #size-cells = <1>; > ranges; > > + ramoops@b7f00000 { > + compatible = "ramoops"; > + reg = <0xb7f00000 0x100000>; > + record-size = <0x4000>; > + console-size = <0x4000>; This is conserative. We've got plenty of space, how about we make it bigger? $ git grep console-size *.dts* | grep -Po "0x([0-9]+)" | xargs printf "%x\n" | sort -n 8000 8000 10000 10000 20000 20000 20000 20000 20000 60000 100000 The median is 128KB, which sounds reasonable. $ git grep record-size *.dts* | grep -Po "0x([0-9]+)" | xargs printf "%x\n" 20000 400 400 20000 20000 20000 10000 10000 10000 10000 20000 64KB is the median record size. > + pmsg-size = <0x4000>; Do we want to add ftrace too? Should we also add max-reason = KMSG_DUMP_EMERG? Logging reboots and shutdowns is informative (you know if a reboot was intentional or due to a crash that wasn't recorded) and allows for testing. Cheers, Joel > + }; > + > flash_memory: region@B8000000 { > no-map; > reg = <0xB8000000 0x04000000>; /* 64M */ > -- > 2.25.1 >
On October 5, 2020 about 10:23 in some timezone, Joel Stanley wrote: >Subject: [EXTERNAL] Re: [PATCH 1/3] ARM: dts: rainier: Add reserved >memory for ramoops > >On Fri, 2 Oct 2020 at 06:35, Andrew Jeffery <andrew@aj.id.au> wrote: >> >> Reserve a 1MiB region of memory to record kmsg dumps and console >state >> into 16kiB ring-buffer slots. The sizing allows for up to 32 dumps >to be >> captured and read out. >> >> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> >> --- >> arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts >b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts >> index e6f422edf454..46a0e95049fd 100644 >> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts >> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts >> @@ -47,6 +47,14 @@ reserved-memory { >> #size-cells = <1>; >> ranges; >> >> + ramoops@b7f00000 { >> + compatible = "ramoops"; >> + reg = <0xb7f00000 0x100000>; >> + record-size = <0x4000>; >> + console-size = <0x4000>; > >This is conserative. We've got plenty of space, how about we make it >bigger? > >$ git grep console-size *.dts* | grep -Po "0x([0-9]+)" | xargs printf >"%x\n" | sort -n >8000 >8000 >10000 >10000 >20000 >20000 >20000 >20000 >20000 >60000 >100000 > >The median is 128KB, which sounds reasonable. How big is the dmesg after we are booted? How big is the default kernel buffer for 2 cpus (the kernel config has a base size, but also a dynamic size with a base + n * cpu min at boot). Having the console space larger than printk buffer will not help. We could config the buffer up if we are not capturing enough of a boot and some runtime. > >$ git grep record-size *.dts* | grep -Po "0x([0-9]+)" | xargs printf >"%x\n" >20000 >400 >400 >20000 >20000 >20000 >10000 >10000 >10000 >10000 >20000 > >64KB is the median record size. > This probably affects the effective compression, assuming each block is a seperate compression input. >> + pmsg-size = <0x4000>; > >Do we want to add ftrace too? > >Should we also add max-reason = KMSG_DUMP_EMERG? > The admin guide lists KMSG_DUMP_OOPS and KMSG_DUMP_PANIC ? https://www.kernel.org/doc/html/latest/admin-guide/ramoops.html We could have something monitoring for OOPS , copying to a log and then unlinking the pstore after committed. >Logging reboots and shutdowns is informative (you know if a reboot >was >intentional or due to a crash that wasn't recorded) and allows for >testing. > That should be exposed from audit logging? but yes. milton >Cheers, > >Joel > >> + }; >> + >> flash_memory: region@B8000000 { >> no-map; >> reg = <0xB8000000 0x04000000>; /* 64M */ >> -- >> 2.25.1 >> > >
On Tue, 6 Oct 2020, at 14:55, Milton Miller II wrote: > On October 5, 2020 about 10:23 in some timezone, Joel Stanley wrote: > >Subject: [EXTERNAL] Re: [PATCH 1/3] ARM: dts: rainier: Add reserved > >memory for ramoops > > > >On Fri, 2 Oct 2020 at 06:35, Andrew Jeffery <andrew@aj.id.au> wrote: > >> > >> Reserve a 1MiB region of memory to record kmsg dumps and console > >state > >> into 16kiB ring-buffer slots. The sizing allows for up to 32 dumps > >to be > >> captured and read out. > >> > >> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > >> --- > >> arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 8 ++++++++ > >> 1 file changed, 8 insertions(+) > >> > >> diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > >b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > >> index e6f422edf454..46a0e95049fd 100644 > >> --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > >> +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > >> @@ -47,6 +47,14 @@ reserved-memory { > >> #size-cells = <1>; > >> ranges; > >> > >> + ramoops@b7f00000 { > >> + compatible = "ramoops"; > >> + reg = <0xb7f00000 0x100000>; > >> + record-size = <0x4000>; > >> + console-size = <0x4000>; > > > >This is conserative. We've got plenty of space, how about we make it > >bigger? > > > >$ git grep console-size *.dts* | grep -Po "0x([0-9]+)" | xargs printf > >"%x\n" | sort -n > >8000 > >8000 > >10000 > >10000 > >20000 > >20000 > >20000 > >20000 > >20000 > >60000 > >100000 > > > >The median is 128KB, which sounds reasonable. > > How big is the dmesg after we are booted? How big is the default > kernel buffer for 2 cpus (the kernel config has a base size, but > also a dynamic size with a base + n * cpu min at boot). > > Having the console space larger than printk buffer will not help. > > We could config the buffer up if we are not capturing enough of a > boot and some runtime. > > > > >$ git grep record-size *.dts* | grep -Po "0x([0-9]+)" | xargs printf > >"%x\n" > >20000 > >400 > >400 > >20000 > >20000 > >20000 > >10000 > >10000 > >10000 > >10000 > >20000 > > > >64KB is the median record size. > > > This probably affects the effective compression, assuming > each block is a seperate compression input. > > >> + pmsg-size = <0x4000>; > > > >Do we want to add ftrace too? > > > >Should we also add max-reason = KMSG_DUMP_EMERG? > > > > The admin guide lists KMSG_DUMP_OOPS and KMSG_DUMP_PANIC ? > https://www.kernel.org/doc/html/latest/admin-guide/ramoops.html > > We could have something monitoring for OOPS , copying to a log and > then unlinking the pstore after committed. systemd-pstore already does this for us, no further configuration required.
On October 5, 2020 at about 11:30PM in some timezone, Andrew Jeffery wrote: >On Tue, 6 Oct 2020, at 14:55, Milton Miller II wrote: >> On October 5, 2020 about 10:23 in some timezone, Joel Stanley >wrote: >> >Subject: [EXTERNAL] Re: [PATCH 1/3] ARM: dts: rainier: Add >reserved >> >memory for ramoops >> > >> >On Fri, 2 Oct 2020 at 06:35, Andrew Jeffery <andrew@aj.id.au> >wrote: >> >> >> >> Reserve a 1MiB region of memory to record kmsg dumps and console >> >state >> >> into 16kiB ring-buffer slots. The sizing allows for up to 32 >dumps >> >to be >> >> captured and read out. >> >> >> >> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> ... >> The admin guide lists KMSG_DUMP_OOPS and KMSG_DUMP_PANIC ?>> >INVALID URI REMOVED >oc_html_latest_admin-2Dguide_ramoops.html&d=DwIBAg&c=jf_iaSHvJObTbx-s >iA1ZOg&r=bvv7AJEECoRKBU02rcu4F5DWd-EwX8As2xrXeO9ZSo4&m=223EDL7j0GQPUf >WYDv8kduEPnexbpo3b00uQAlK8YSo&s=MfYAUnU2h1TdWyBC7tQoG3fVUTBTwXTFurBsK >oZw34E&e= >> >> We could have something monitoring for OOPS , copying to a log and >> then unlinking the pstore after committed. > >systemd-pstore already does this for us, no further configuration >required. Do we have something that creates service records for these messages? I was hoping for something like a PEL for the bmc software. Not as part of this kernel series though. milton
On Tue, 6 Oct 2020, at 13:52, Joel Stanley wrote: > On Fri, 2 Oct 2020 at 06:35, Andrew Jeffery <andrew@aj.id.au> wrote: > > > > Reserve a 1MiB region of memory to record kmsg dumps and console state > > into 16kiB ring-buffer slots. The sizing allows for up to 32 dumps to be > > captured and read out. > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > --- > > arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > > index e6f422edf454..46a0e95049fd 100644 > > --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > > +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts > > @@ -47,6 +47,14 @@ reserved-memory { > > #size-cells = <1>; > > ranges; > > > > + ramoops@b7f00000 { > > + compatible = "ramoops"; > > + reg = <0xb7f00000 0x100000>; > > + record-size = <0x4000>; > > + console-size = <0x4000>; > > This is conserative. We've got plenty of space, how about we make it bigger? > > $ git grep console-size *.dts* | grep -Po "0x([0-9]+)" | xargs printf > "%x\n" | sort -n > 8000 > 8000 > 10000 > 10000 > 20000 > 20000 > 20000 > 20000 > 20000 > 60000 > 100000 > > The median is 128KB, which sounds reasonable. Well, maybe? Why should we basing it on what everyone else has done rather than what we need? We're compressing the data before it's written to the pstore ring. Uncompressed, 16k is in the order of 200 lines of text. With the default DEFLATE compression we can fit 3-4x more: root@rain15bmc:~# uptime 14:44:20 up 14:44, load average: 0.01, 0.01, 0.01 root@rain15bmc:~# dmesg | wc -l 640 root@rain15bmc:~# dmesg >/tmp/dmesg root@rain15bmc:~# stat -c "%s" /tmp/dmesg 44032 root@rain15bmc:~# gzip /tmp/dmesg root@rain15bmc:~# stat -c "%s" /tmp/dmesg.gz 11059 I think 16k is more than enough for now? > > $ git grep record-size *.dts* | grep -Po "0x([0-9]+)" | xargs printf "%x\n" > 20000 > 400 > 400 > 20000 > 20000 > 20000 > 10000 > 10000 > 10000 > 10000 > 20000 > > 64KB is the median record size. > > > + pmsg-size = <0x4000>; > > Do we want to add ftrace too? I figured getting an oops/panic stack dump might be enough for the moment. > > Should we also add max-reason = KMSG_DUMP_EMERG? > > Logging reboots and shutdowns is informative (you know if a reboot was > intentional or due to a crash that wasn't recorded) and allows for > testing. Yeah, that's a good idea. Thanks. Andrew
diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts index e6f422edf454..46a0e95049fd 100644 --- a/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts +++ b/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts @@ -47,6 +47,14 @@ reserved-memory { #size-cells = <1>; ranges; + ramoops@b7f00000 { + compatible = "ramoops"; + reg = <0xb7f00000 0x100000>; + record-size = <0x4000>; + console-size = <0x4000>; + pmsg-size = <0x4000>; + }; + flash_memory: region@B8000000 { no-map; reg = <0xB8000000 0x04000000>; /* 64M */
Reserve a 1MiB region of memory to record kmsg dumps and console state into 16kiB ring-buffer slots. The sizing allows for up to 32 dumps to be captured and read out. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dts | 8 ++++++++ 1 file changed, 8 insertions(+)