Re: [PATCH] Only allow to set crash_kexec_post_notifiers on boot time

[Eric W. Biederman]

Konrad Rzeszutek Wilk  writes:

> On Fri, Sep 18, 2020 at 05:47:43PM -0700, Andrew Morton wrote:
>> On Fri, 18 Sep 2020 11:25:46 +0800 Dave Young  wrote:
>> 
>> > crash_kexec_post_notifiers enables running various panic notifier
>> > before kdump kernel booting. This increases risks of kdump failure.
>> > It is well documented in kernel-parameters.txt. We do not suggest
>> > people to enable it together with kdump unless he/she is really sure.
>> > This is also not suggested to be enabled by default when users are
>> > not aware in distributions.
>> > 
>> > But unfortunately it is enabled by default in systemd, see below
>> > discussions in a systemd report, we can not convince systemd to change
>> > it:
>> > https://github.com/systemd/systemd/issues/16661
>> > 
>> > Actually we have got reports about kdump kernel hangs in both s390x
>> > and powerpcle cases caused by the systemd change,  also some x86 cases
>> > could also be caused by the same (although that is in Hyper-V code
>> > instead of systemd, that need to be addressed separately).
>
> Perhaps it may be better to fix the issus on s390x and PowerPC as well?
>
>> > 
>> > Thus to avoid the auto enablement here just disable the param writable
>> > permission in sysfs.
>> > 
>> 
>> Well.  I don't think this is at all a desirable way of resolving a
>> disagreement with the systemd developers
>> 
>> At the above github address I'm seeing "ryncsn added a commit to
>> ryncsn/systemd that referenced this issue 9 days ago", "pstore: don't
>> enable crash_kexec_post_notifiers by default".  So didn't that address
>> the issue?
>
> It does in systemd, but there is a strong interest in making this on
> by default.

There is also a strong interest in removing this code entirely from the
kernel.

This failure is a case in point.

I think I am at my I told you so point.  This is what all of the testing
over all the years has said.  Leaving functionality to the peculiarities
of firmware when you don't have to, and can actually control what is
going on doesn't work.

Eric



___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Wir finanzieren Projekte und Unternehmen

[Blue Oak Mortgage and Loans]

Dies ist ein Newsletter von Blue Oak Mortgage and Loans. Bitte melden Sie sich 
ab, wenn Sie keine E-Mail mehr von uns erhalten möchten.


Eine kurze Einführung.

Wir sind ein führendes Finanzierungsunternehmen in Europa. Wir finanzieren 
Startups / etablierte Unternehmen, finanzieren Großprojekte (Bau, 
Landwirtschaft, Immobilien und dergleichen) zu einem niedrigen Zinssatz von 2% 
pro Jahr.


Darlehensverfahren

1. Sie müssen das Online-Bewerbungsformular ausfüllen und eine ordnungsgemäß 
unterschriebene Kopie an uns zurücksenden.

2. Möglicherweise müssen Sie Finanzdokumente als unterstützenden Nachweis für 
die Fähigkeit zur Rückzahlung von Krediten vorlegen.

3. Wenn Ihr Darlehen genehmigt wurde, müssen Sie eine Versicherungsgarantie für 
die Darlehenssicherheit vorlegen. Wir empfehlen eine Versicherungsgesellschaft. 
Sie sind allein verantwortlich für die Zahlung und den Erwerb der Anleihe, die 
als Sicherheit dienen. Die Höhe der Anleihe hängt von Ihrem Darlehensbetrag ab. 
Die Versicherungsgesellschaft wird Sie durch den Prozess führen. (Für 
Großprojekte)

4. Ihr Überweisungsprozess wird eingeleitet, sobald die Versicherungsanleihe 
überprüft wurde. Ihr Darlehensrückzahlungsplan wird im 
NC-Darlehensvertragsformular aufgeführt.

Wenn die Bedingungen Sie beruhigen, können Sie uns über die WhatsApp-Nummer / 
E-Mail kontaktieren und auch unsere Website besuchen, um weitere Informationen 
zu erhalten. Wir freuen uns darauf, von Ihnen zu hören.

WhatsApp: + 90-552-365-3483
E-Mail: i...@bluelmtg.net

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Only allow to set crash_kexec_post_notifiers on boot time

[Konrad Rzeszutek Wilk]

On Fri, Sep 18, 2020 at 05:47:43PM -0700, Andrew Morton wrote:
> On Fri, 18 Sep 2020 11:25:46 +0800 Dave Young  wrote:
> 
> > crash_kexec_post_notifiers enables running various panic notifier
> > before kdump kernel booting. This increases risks of kdump failure.
> > It is well documented in kernel-parameters.txt. We do not suggest
> > people to enable it together with kdump unless he/she is really sure.
> > This is also not suggested to be enabled by default when users are
> > not aware in distributions.
> > 
> > But unfortunately it is enabled by default in systemd, see below
> > discussions in a systemd report, we can not convince systemd to change
> > it:
> > https://github.com/systemd/systemd/issues/16661
> > 
> > Actually we have got reports about kdump kernel hangs in both s390x
> > and powerpcle cases caused by the systemd change,  also some x86 cases
> > could also be caused by the same (although that is in Hyper-V code
> > instead of systemd, that need to be addressed separately).

Perhaps it may be better to fix the issus on s390x and PowerPC as well?

> > 
> > Thus to avoid the auto enablement here just disable the param writable
> > permission in sysfs.
> > 
> 
> Well.  I don't think this is at all a desirable way of resolving a
> disagreement with the systemd developers
> 
> At the above github address I'm seeing "ryncsn added a commit to
> ryncsn/systemd that referenced this issue 9 days ago", "pstore: don't
> enable crash_kexec_post_notifiers by default".  So didn't that address
> the issue?

It does in systemd, but there is a strong interest in making this on by default.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] arm64 : fix makedumpfile failure on 5.4+ kernels

[Ioanna Alifieraki]

Hi Bhupesh,

Sorry for the late reply, I tested your v5 patches on arm64 and x86_64 vms
with 4.15, 5.4 and the latest upstream kernel and makedumpfile works fine.

Cheers,
Ioanna

On Thu, Sep 10, 2020 at 9:42 PM Bhupesh SHARMA  wrote:
>
> Hello Ioanna,
>
> Thanks for the patch. I am partially at blame here (and also for
> top-posting here) as this failure is caused due to the flipped VA
> address space support we have on arm64 architecture now with newer
> kernels (>= 5.4.0) due to the addition of larger VA addressing space
> features (52-bit).
>
> I had sent out a v4 series to fix this issue several months back (see
> ) and I was
> supposed to send an update for the same (v5), but the kernel patches
> (for variable export to vmcoreinfo) took a longer time to get accepted
> upstream.
>
> I have shared the updated v5 version earlier today (see
> ).
> Can you please try the same and share your testing inputs.
>
> Thanks,
> Bhupesh
>
>
>
>
> On Thu, Sep 10, 2020 at 6:25 AM Ioanna Alifieraki
>  wrote:
> >
> > Currently makedumpfile fails on arm64 for 5.4 and newer kernels with
> > the following :
> >
> > [9.595476] kdump-tools[513]: Starting kdump-tools:
> > [9.597988] kdump-tools[519]:  * running makedumpfile -c -d 31 /proc/vmcore 
> > /var/crash/202009011332/dump-incomplete
> > [9.636915] kdump-tools[537]: calculate_plat_config: PAGE SIZE 0x1000 and VA 
> > Bits 47 not supported
> > [9.639652] kdump-tools[537]: get_machdep_info_arm64: Can't determine 
> > platform config values
> > [9.642085] kdump-tools[537]: makedumpfile Failed.
> > [9.643064] kdump-tools[519]:  * kdump-tools: makedumpfile failed, falling 
> > back to 'cp'
> >
> > The problem starts at get_versiondep_info_arm64 function.
> > This functions uses _stext to calculate the va_bits.
> > Up to 5.3 kernels the _stext would be 10081000.
> > After commit 14c127c957c1c607 (arm64: mm: Flip kernel VA space),
> > _stext is 800010081000 which ends up the va_bits getting the value
> > of 47, even though the kernel configuration is 48 bits.
> > Now _stext has contiguous bits 8  and matches the 47 bits
> > while in the past it had 0 that would match the 48 bits.
> > The va_bits variable is already exported in vmcoreinfo and therefore
> > that could be solved by reading the value  from there
> > (va_bits = NUMBER(VA_BITS)) instead of relying on _stext.
> > However, if we do so, the page_offset is not calculated properly.
> > Currently :
> > info->page_offset = (0xUL) << (va_bits - 1);
> > The page_offset still depends on the _stext value.
> > So read the va_bits from vmcoreinfo and keep the old logic of calculating
> > va_bits to calculate page_offset (calc_page_offset_variable).
> >
> > Signed-off-by: Ioanna Alifieraki 
> > ---
> >  arch/arm64.c | 18 +++---
> >  1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm64.c b/arch/arm64.c
> > index 54d60b4..2b0fca3 100644
> > --- a/arch/arm64.c
> > +++ b/arch/arm64.c
> > @@ -290,6 +290,7 @@ int
> >  get_versiondep_info_arm64(void)
> >  {
> > ulong _stext;
> > +   int calc_page_offset;
> >
> > _stext = get_stext_symbol();
> > if (!_stext) {
> > @@ -297,25 +298,28 @@ get_versiondep_info_arm64(void)
> > return FALSE;
> > }
> >
> > +   va_bits = NUMBER(VA_BITS);
> > +
> > /* Derive va_bits as per arch/arm64/Kconfig */
> > if ((_stext & PAGE_OFFSET_36) == PAGE_OFFSET_36) {
> > -   va_bits = 36;
> > +   calc_page_offset = 36;
> > } else if ((_stext & PAGE_OFFSET_39) == PAGE_OFFSET_39) {
> > -   va_bits = 39;
> > +   calc_page_offset = 39;
> > } else if ((_stext & PAGE_OFFSET_42) == PAGE_OFFSET_42) {
> > -   va_bits = 42;
> > +   calc_page_offset = 42;
> > } else if ((_stext & PAGE_OFFSET_47) == PAGE_OFFSET_47) {
> > -   va_bits = 47;
> > +   calc_page_offset = 47;
> > } else if ((_stext & PAGE_OFFSET_48) == PAGE_OFFSET_48) {
> > -   va_bits = 48;
> > +   calc_page_offset = 48;
> > } else {
> > -   ERRMSG("Cannot find a proper _stext for calculating 
> > VA_BITS\n");
> > +   ERRMSG("Cannot find a proper _stext for calculating 
> > page_offset\n");
> > return FALSE;
> > }
> >
> > -   info->page_offset = (0xUL) << (va_bits - 1);
> > +   info->page_offset = (0xUL) << (calc_page_offset - 
> > 1);
> >
> > DEBUG_MSG("va_bits  : %d\n", va_bits);
> > +   DEBUG_MSG("calc_page_offset  : %d\n", calc_page_offset);
> > DEBUG_MSG("page_offset  : %lx\n", info->page_offset);
> >
> > return TRUE;
> > --
> > 2.17.1
> >
> >
> > ___
> > kexec mailing list
> >

Re: [PATCH printk v4 2/3] printk: move dictionary keys to dev_printk_info

[Petr Mladek]

On Mon 2020-09-21 13:24:45, John Ogness wrote:
> Dictionaries are only used for SUBSYSTEM and DEVICE properties. The
> current implementation stores the property names each time they are
> used. This requires more space than otherwise necessary. Also,
> because the dictionary entries are currently considered optional,
> it cannot be relied upon that they are always available, even if the
> writer wanted to store them. These issues will increase should new
> dictionary properties be introduced.
> 
> Rather than storing the subsystem and device properties in the
> dict ring, introduce a struct dev_printk_info with separate fields
> to store only the property values. Embed this struct within the
> struct printk_info to provide guaranteed availability.
> 
> Signed-off-by: John Ogness 
> Reviewed-by: Petr Mladek 
> ---
>  Sorry. v3 did not include Petr's fixup correctly. @size was wrong.
>  Now it is correct.

I could confirm that the added line and the patch looks fine now.

Best Regards,
Petr

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH printk v4 2/3] printk: move dictionary keys to dev_printk_info

[John Ogness]

Dictionaries are only used for SUBSYSTEM and DEVICE properties. The
current implementation stores the property names each time they are
used. This requires more space than otherwise necessary. Also,
because the dictionary entries are currently considered optional,
it cannot be relied upon that they are always available, even if the
writer wanted to store them. These issues will increase should new
dictionary properties be introduced.

Rather than storing the subsystem and device properties in the
dict ring, introduce a struct dev_printk_info with separate fields
to store only the property values. Embed this struct within the
struct printk_info to provide guaranteed availability.

Signed-off-by: John Ogness 
Reviewed-by: Petr Mladek 
---
 Sorry. v3 did not include Petr's fixup correctly. @size was wrong.
 Now it is correct.

 Documentation/admin-guide/kdump/gdbmacros.txt |  73 
 drivers/base/core.c   |  46 ++---
 include/linux/dev_printk.h|   8 +
 include/linux/printk.h|   6 +-
 kernel/printk/internal.h  |   4 +-
 kernel/printk/printk.c| 166 +-
 kernel/printk/printk_ringbuffer.h |   3 +
 kernel/printk/printk_safe.c   |   2 +-
 scripts/gdb/linux/dmesg.py|  16 +-
 9 files changed, 164 insertions(+), 160 deletions(-)

diff --git a/Documentation/admin-guide/kdump/gdbmacros.txt 
b/Documentation/admin-guide/kdump/gdbmacros.txt
index 94fabb165abf..82aecdcae8a6 100644
--- a/Documentation/admin-guide/kdump/gdbmacros.txt
+++ b/Documentation/admin-guide/kdump/gdbmacros.txt
@@ -172,13 +172,13 @@ end
 
 define dump_record
set var $desc = $arg0
-   if ($argc > 1)
-   set var $prev_flags = $arg1
+   set var $info = $arg1
+   if ($argc > 2)
+   set var $prev_flags = $arg2
else
set var $prev_flags = 0
end
 
-   set var $info = &$desc->info
set var $prefix = 1
set var $newline = 1
 
@@ -237,44 +237,36 @@ define dump_record
 
# handle dictionary data
 
-   set var $begin = $desc->dict_blk_lpos.begin % (1U << 
prb->dict_data_ring.size_bits)
-   set var $next = $desc->dict_blk_lpos.next % (1U << 
prb->dict_data_ring.size_bits)
-
-   # handle data-less record
-   if ($begin & 1)
-   set var $dict_len = 0
-   set var $dict = ""
-   else
-   # handle wrapping data block
-   if ($begin > $next)
-   set var $begin = 0
-   end
-
-   # skip over descriptor id
-   set var $begin = $begin + sizeof(long)
-
-   # handle truncated message
-   if ($next - $begin < $info->dict_len)
-   set var $dict_len = $next - $begin
-   else
-   set var $dict_len = $info->dict_len
+   set var $dict = &$info->dev_info.subsystem[0]
+   set var $dict_len = sizeof($info->dev_info.subsystem)
+   if ($dict[0] != '\0')
+   printf " SUBSYSTEM="
+   set var $idx = 0
+   while ($idx < $dict_len)
+   set var $c = $dict[$idx]
+   if ($c == '\0')
+   loop_break
+   else
+   if ($c < ' ' || $c >= 127 || $c == '\\')
+   printf "\\x%02x", $c
+   else
+   printf "%c", $c
+   end
+   end
+   set var $idx = $idx + 1
end
-
-   set var $dict = &prb->dict_data_ring.data[$begin]
+   printf "\n"
end
 
-   if ($dict_len > 0)
+   set var $dict = &$info->dev_info.device[0]
+   set var $dict_len = sizeof($info->dev_info.device)
+   if ($dict[0] != '\0')
+   printf " DEVICE="
set var $idx = 0
-   set var $line = 1
while ($idx < $dict_len)
-   if ($line)
-   printf " "
-   set var $line = 0
-   end
set var $c = $dict[$idx]
if ($c == '\0')
-   printf "\n"
-   set var $line = 1
+   loop_break
else
if ($c < ' ' || $c >= 127 || $c == '\\')
printf "\\x%02x", $c
@@ -288,10 +280,10 @@ define dump_record
end
 end
 document dump_record
-   Dump a single record. The first parameter is the descriptor
-   sequence number, the second is optional and specifies the
-   previous record's flags, used for properly formatting
-   continued

[PATCH printk v3 2/3] printk: move dictionary keys to dev_printk_info

[John Ogness]

Dictionaries are only used for SUBSYSTEM and DEVICE properties. The
current implementation stores the property names each time they are
used. This requires more space than otherwise necessary. Also,
because the dictionary entries are currently considered optional,
it cannot be relied upon that they are always available, even if the
writer wanted to store them. These issues will increase should new
dictionary properties be introduced.

Rather than storing the subsystem and device properties in the
dict ring, introduce a struct dev_printk_info with separate fields
to store only the property values. Embed this struct within the
struct printk_info to provide guaranteed availability.

Signed-off-by: John Ogness 
Reviewed-by: Petr Mladek 
---
 Added Petr's fixup for msg_add_dict_text() to include the prefix
 whitespace for dictionary properties. Thanks!

 Documentation/admin-guide/kdump/gdbmacros.txt |  73 
 drivers/base/core.c   |  46 ++---
 include/linux/dev_printk.h|   8 +
 include/linux/printk.h|   6 +-
 kernel/printk/internal.h  |   4 +-
 kernel/printk/printk.c| 166 +-
 kernel/printk/printk_ringbuffer.h |   3 +
 kernel/printk/printk_safe.c   |   2 +-
 scripts/gdb/linux/dmesg.py|  16 +-
 9 files changed, 164 insertions(+), 160 deletions(-)

diff --git a/Documentation/admin-guide/kdump/gdbmacros.txt 
b/Documentation/admin-guide/kdump/gdbmacros.txt
index 94fabb165abf..82aecdcae8a6 100644
--- a/Documentation/admin-guide/kdump/gdbmacros.txt
+++ b/Documentation/admin-guide/kdump/gdbmacros.txt
@@ -172,13 +172,13 @@ end
 
 define dump_record
set var $desc = $arg0
-   if ($argc > 1)
-   set var $prev_flags = $arg1
+   set var $info = $arg1
+   if ($argc > 2)
+   set var $prev_flags = $arg2
else
set var $prev_flags = 0
end
 
-   set var $info = &$desc->info
set var $prefix = 1
set var $newline = 1
 
@@ -237,44 +237,36 @@ define dump_record
 
# handle dictionary data
 
-   set var $begin = $desc->dict_blk_lpos.begin % (1U << 
prb->dict_data_ring.size_bits)
-   set var $next = $desc->dict_blk_lpos.next % (1U << 
prb->dict_data_ring.size_bits)
-
-   # handle data-less record
-   if ($begin & 1)
-   set var $dict_len = 0
-   set var $dict = ""
-   else
-   # handle wrapping data block
-   if ($begin > $next)
-   set var $begin = 0
-   end
-
-   # skip over descriptor id
-   set var $begin = $begin + sizeof(long)
-
-   # handle truncated message
-   if ($next - $begin < $info->dict_len)
-   set var $dict_len = $next - $begin
-   else
-   set var $dict_len = $info->dict_len
+   set var $dict = &$info->dev_info.subsystem[0]
+   set var $dict_len = sizeof($info->dev_info.subsystem)
+   if ($dict[0] != '\0')
+   printf " SUBSYSTEM="
+   set var $idx = 0
+   while ($idx < $dict_len)
+   set var $c = $dict[$idx]
+   if ($c == '\0')
+   loop_break
+   else
+   if ($c < ' ' || $c >= 127 || $c == '\\')
+   printf "\\x%02x", $c
+   else
+   printf "%c", $c
+   end
+   end
+   set var $idx = $idx + 1
end
-
-   set var $dict = &prb->dict_data_ring.data[$begin]
+   printf "\n"
end
 
-   if ($dict_len > 0)
+   set var $dict = &$info->dev_info.device[0]
+   set var $dict_len = sizeof($info->dev_info.device)
+   if ($dict[0] != '\0')
+   printf " DEVICE="
set var $idx = 0
-   set var $line = 1
while ($idx < $dict_len)
-   if ($line)
-   printf " "
-   set var $line = 0
-   end
set var $c = $dict[$idx]
if ($c == '\0')
-   printf "\n"
-   set var $line = 1
+   loop_break
else
if ($c < ' ' || $c >= 127 || $c == '\\')
printf "\\x%02x", $c
@@ -288,10 +280,10 @@ define dump_record
end
 end
 document dump_record
-   Dump a single record. The first parameter is the descriptor
-   sequence number, the second is optional and specifies the
-   previous record's flags, used for properly for

Re: [PATCH printk v2 3/3] printk: remove dict ring

[Petr Mladek]

On Sat 2020-09-19 00:40:21, John Ogness wrote:
> Since there is no code that will ever store anything into the dict
> ring, remove it. If any future dictionary properties are to be
> added, these should be added to the struct printk_info.
> 
> Signed-off-by: John Ogness 

Reviewed-by: Petr Mladek 

Best Regards,
Petr

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH printk v2 2/3] printk: move dictionary keys to dev_printk_info

[Petr Mladek]

On Sat 2020-09-19 00:40:20, John Ogness wrote:
> Dictionaries are only used for SUBSYSTEM and DEVICE properties. The
> current implementation stores the property names each time they are
> used. This requires more space than otherwise necessary. Also,
> because the dictionary entries are currently considered optional,
> it cannot be relied upon that they are always available, even if the
> writer wanted to store them. These issues will increase should new
> dictionary properties be introduced.
> 
> Rather than storing the subsystem and device properties in the
> dict ring, introduce a struct dev_printk_info with separate fields
> to store only the property values. Embed this struct within the
> struct printk_info to provide guaranteed availability.


> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -629,36 +624,43 @@ static ssize_t msg_print_ext_body(char *buf, size_t 
> size,
>   else
>   append_char(&p, e, c);
>   }
> - append_char(&p, e, '\n');
> + append_char(&p, e, endc);
>  
> - if (dict_len) {
> - bool line = true;
> + return p - buf;
> +}
>  
> - for (i = 0; i < dict_len; i++) {
> - unsigned char c = dict[i];
> +static ssize_t msg_add_dict_text(char *buf, size_t size,
> +  const char *key, const char *val)
> +{
> + size_t val_len = strlen(val);
> + ssize_t len;
>  
> - if (line) {
> - append_char(&p, e, ' ');
> - line = false;

I double checked this and found that the above code prefixed dict
values by ' ' in /dev/kmsg.

It slightly improves readability and it is handy for eventual filtering.
It would make sense to keep it.

> - }
> + if (!val_len)
> + return 0;
>  
> - if (c == '\0') {
> - append_char(&p, e, '\n');
> - line = true;
> - continue;
> - }
> + len = msg_add_ext_text(buf, size, key, strlen(key), '=');
> + len += msg_add_ext_text(buf + len, size - len, val, val_len, '\n');

Slightly ugly but simple solution is:

len = msg_add_ext_text(buf, size, "", 0, ' ');  /* dict prefix */
len += msg_add_ext_text(buf + len, size - len, key, strlen(key), '=');
len += msg_add_ext_text(buf + len, size - len, val, val_len, '\n');

With this fix:

Reviewed-by: Petr Mladek 


Now, this is the only problem that I have found. It is not necessary
to resend the entire patchset just because of this.

It might be enough to either respin just this patch. Or I could
commit the below one on top of the patchset. Either solution works
for me.

>From dcc5dc0467c6e7d13202d98bbefb505a1db693fd Mon Sep 17 00:00:00 2001
From: Petr Mladek 
Date: Mon, 21 Sep 2020 11:45:16 +0200
Subject: [PATCH] printk: Put back dict lines prefix into /dev/kmsg

Put back prefix for dictionary lines in /dev/kmsg. They have been removed
by the commit  XXX ("printk: move dictionary keys to dev_printk_info").

Signed-off-by: Petr Mladek 
---
 kernel/printk/printk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 77660354a7c5..1fe3d0cb2fe0 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -637,7 +637,8 @@ static ssize_t msg_add_dict_text(char *buf, size_t size,
if (!val_len)
return 0;
 
-   len = msg_add_ext_text(buf, size, key, strlen(key), '=');
+   len = msg_add_ext_text(buf, size, "", 0, ' ');  /* dict prefix */
+   len += msg_add_ext_text(buf + len, size - len, key, strlen(key), '=');
len += msg_add_ext_text(buf + len, size - len, val, val_len, '\n');
 
return len;
-- 
2.26.2


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH printk v2 1/3] printk: move printk_info into separate array

[Petr Mladek]

On Sat 2020-09-19 00:40:19, John Ogness wrote:
> The majority of the size of a descriptor is taken up by meta data,
> which is often not of interest to the ringbuffer (for example,
> when performing state checks). Since descriptors are often
> temporarily stored on the stack, keeping their size minimal will
> help reduce stack pressure.
> 
> Rather than embedding the printk_info into the descriptor, create
> a separate printk_info array. The index of a descriptor in the
> descriptor array corresponds to the printk_info with the same
> index in the printk_info array. The rules for validity of a
> printk_info match the existing rules for the data blocks: the
> descriptor must be in a consistent state.
> 
> Signed-off-by: John Ogness 

Reviewed-by: Petr Mladek 

Best Regards,
Petr

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [RFC PATCH 0/3] Add writing support to vmcore for reusing oldmem

[Kairui Song]

On Thu, Sep 10, 2020 at 12:43 AM Kairui Song  wrote:
>
> On Wed, Sep 9, 2020 at 10:04 PM Eric W. Biederman  
> wrote:
> >
> > Kairui Song  writes:
> >
> > > Currently vmcore only supports reading, this patch series is an RFC
> > > to add writing support to vmcore. It's x86_64 only yet, I'll add other
> > > architecture later if there is no problem with this idea.
> > >
> > > My purpose of adding writing support is to reuse the crashed kernel's
> > > old memory in kdump kernel, reduce kdump memory pressure, and
> > > allow kdump to run with a smaller crashkernel reservation.
> > >
> > > This is doable because in most cases, after kernel panic, user only
> > > interested in the crashed kernel itself, and userspace/cache/free
> > > memory pages are not dumped. `makedumpfile` is widely used to skip
> > > these pages. Kernel pages usually only take a small part of
> > > the whole old memory. So there will be many reusable pages.
> > >
> > > By adding writing support, userspace then can use these pages as a fast
> > > and temporary storage. This helps reduce memory pressure in many ways.
> > >
> > > For example, I've written a POC program based on this, it will find
> > > the reusable pages, and creates an NBD device which maps to these pages.
> > > The NBD device can then be used as swap, or to hold some temp files
> > > which previouly live in RAM.
> > >
> > > The link of the POC tool: https://github.com/ryncsn/kdumpd
> >
> > A couple of thoughts.
> > 1) Unless I am completely mistaken treating this as a exercise in
> >memory hotplug would be much simpler.
> >
> >AKA just plug in the memory that is not needed as part of the kdump.
> >
> >I see below that you have problems doing this because
> >of fragmentation.  I still think hotplug is doable using some
> >kind of fragmented memory zone.
> >
> > 2) The purpose of the memory reservation is because hardware is
> >still potentially running agains the memory of the old kernel.
> >
> >By the time we have brought up a new kernel enough of the hardware
> >may have been reinitialized that we don't have to worry about
> >hardware randomly dma'ing into the memory used by the old kernel.
> >
> >With IOMMUs and care we may be able to guarantee for some machine
> >configurations it is impossible for DMA to come from some piece of
> >hardware that is present but the kernel does not have a driver
> >loaded for.\
> >
> > I really do not like this approach because it is fundamentlly doing the
> > wrong thing.  Adding write support to read-only drivers.  I do not see
> > anywhere that you even mentioned the hard problem and the reason we
> > reserve memory in the first place.  Hardware spontaneously DMA'ing onto
> > it.
> >
> That POC tool looks ugly for now as it only a draft to prove this
> works, sorry about it.
>
> For the patch, yes, it is expecting IOMMU to lower the chance of
> potential DMA issue, and expecting DMA will not hit userspace/free
> page, or at least won't override a massive amount of reusable old
> memory. And I thought about some solutions for the potential DMA
> issue.
>
> As old memories are used as a block device, which is proxied by
> userspace, so upon each IO, the userspace tool could do an integrity
> check of the corresponding data stored in old mem, and keep multiple
> copies of the data. (eg. use 512M of old memory to hold a 128M block
> device). These copies will be kept far away from each other regarding
> the physical memory location. The reusable old memories are sparse so
> the actual memory containing the data should be also sparse.
> So if some part is corrupted, it is still recoverable. Unless the DMA
> went very wrong and wiped a large region of memory, but if such thing
> happens, it's most likely kernel pages are also being wiped by DMA, so
> the vmcore is already corrupted and kdump may not help. But at least
> it won't fail silently, the userspace tool can still do something like
> dump some available data to an easy to setup target.
>
> And also that's one of the reasons not using old memory as kdump's
> memory directly.
>
> > > It's have been a long time issue that kdump suffers from OOM issue
> > > with limited crashkernel memory. So reusing old memory could be very
> > > helpful.
> >
> > There is a very fine line here between reusing existing code (aka
> > drivers and userspace) and doing something that should work.
> >
> > It might make sense to figure out what is using so much memory
> > that an OOM is triggered.
> >
> > Ages ago I did something that was essentially dumping the kernels printk
> > buffer to the serial console in case of a crash and I had things down to
> > something comparatively miniscule like 8M or less.
> >
> > My memory is that historically it has been high performance scsi raid
> > drivers or something like that, that are behind the need to have such
> > large memory reservations.
> >
> > Now that I think about it, you aren't by any chance doing something