[makedumpfile PATCH] Always use bigger SECTION_MAP_MASK

[Masaki Tachibana]

Hi Petr,

Thank you for your explanation.
I understand.
I think we have to show it's not a bug for the future people.
Would you insert comments before definition of SECTION_MAP_LAST_BIT
and resend the patch ?
For example. (Better English is welcome.)

#define SECTION_IS_ONLINE       (1UL<<2)
/*
 * In case of upstream kernels before 4.13;
 * 1. SECTION_MAP_LAST_BIT must be (1UL<<2), but it's dependent on distribution
 *    whether the bit shift should be 2 or 3.
 * 2. It has been verified that (1UL<<2) bit is not set.
 * So for both old kernels and new ones, we use 3.
 */
#define SECTION_MAP_LAST_BIT    (1UL<<3)

Thanks
Tachibana

> -----Original Message-----
> From: Petr Tesarik [mailto:ptesarik at suse.com]
> Sent: Tuesday, March 13, 2018 7:00 PM
> To: Tachibana Masaki() <mas-tachibana at vf.jp.nec.com>
> Cc: kexec mailing list <kexec at lists.infradead.org>; Hayashi Masahiko() <mas-hayashi at tg.jp.nec.com>
> Subject: Re: [makedumpfile PATCH] Always use bigger SECTION_MAP_MASK
> 
> Hi,
> 
> On Tue, 13 Mar 2018 09:18:54 +0000
> Masaki Tachibana <mas-tachibana at vf.jp.nec.com> wrote:
> 
> > Hi Petr,
> >
> > Sorry for the late reply.
> 
> Never mind. I was on vacation, anyway. ;-)
> 
> > > -----Original Message-----
> > > From: kexec [mailto:kexec-bounces at lists.infradead.org] On Behalf Of Petr Tesarik
> > > Sent: Friday, February 02, 2018 5:19 PM
> > > To: kexec mailing list <kexec at lists.infradead.org>; Tachibana Masaki() <mas-tachibana at vf.jp.nec.com>; Nakayama
> > > Takuya() <tak-nakayama at tg.jp.nec.com>; Nishimura Daisuke() <dai-nishimura at rc.jp.nec.com>
> > > Subject: [makedumpfile PATCH] Always use bigger SECTION_MAP_MASK
> > >
> > > It is not necessary to distinguish between different kernel
> > > versions. Kernel commit 2d070eab2e82 merely reuses a previously
> > > unused bit (see clarification in kernel commit def9b71ee651a). The
> > > bit was always zero even before that commit, so it is safe to mask
> > > it off even for kernel versions before 4.13, removing some of the
> > > complexity.
> > >
> > > More importantly, makedumpfile fails on kernels which backport the
> > > patch on top of an older version (for example SLES15).
> >
> > I understand circumstances of SLES.
> > However I am worried;
> > - Is it good that we correct the following specified code for the
> >   particular distribution?
> 
> No, absolutely not. That's why I'm not trying to fix a particular
> distribution, but rather make it work for all distributions (including
> my own).
> 
> > - Is it good that makedumpfile ignores the bit because the kernel
> >   isn't using it?
> 
> I've given it a serious thought before writing my patch. Let me explain.
> 
> First, what happens if the bit is set?
> 
>   A. If the kernel version is at least 4.13.0, then my patch does not
>      change makedumpfile's behaviour.
> 
>   B. If the kernel version is older than 4.13.0, then I have verified
>      that the bit must not be set. If it is set, then this is a bug in
>      the kernel or random memory corruption. The kernel would then use
>      an incorrect memmap address, probably crashing in short time.
>      Without my patch, makedumpfile will most likely fail to produce
>      any output. With my patch, it may produce some output.
> 
> Second, my patch simplifies makedumpfile code, which is good in itself,
> because it reduces the maintenance burden going forward. A few years
> from now everybody will be too afraid of regressions to do such cleanup.
> 
> The only downside is that in case B above, makedumpfile may silently
> produce incorrect output instead of failing mysteriously with an
> error message such as: "Can't convert a virtual address
> (f000000001000000) to physical address".
> 
> Just my two cents,
> Petr Tesarik