Re: kdump twirling - does not finish - commit 326e1b8f83a4318b09033ef754f40c785aed5e68

[David Hildenbrand <david@redhat.com>]

On 04.03.20 18:03, Marshall Midden wrote:
> I know a translator is required - trying... Mark - is this better than
> typical?
> 
> Problem: kdump is not finishing, hangs twirling ... iKVM
> Found: Power failure. Qlogic/Cavium/Marvell driver crash, but no kernel
> dump.  v5.3.8+local.
> Reproduce: Boot, "echo c > /proc/sysrq-trigger"
> Last known working release: v5.2.x (any of them).
> First release failure: v5.3-rc1
> Tried many versions up to v5.6-rc4 (2020-03-01).
> 
> Repository: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
> System: Supermicro X10 dual socket x86_64, 64g/256g/512g (different
> systems/MB/CPUs).
> UserLand: RedHat7.5
> Tried (did not fix) kexec-tools fetched Feb 25th, 2020 from:
>     git://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git
> <http://git.kernel.org/pub/scm/utils/kernel/kexec/kexec-tools.git>
> .config: No modules, except those listed below (only 10 "=m").
> No modules in initramfs, /etc/dracut.conf.d/xxxx:
>     omit_drivers+="qla2xxx nfs_layout_flexfiles"
>     omit_drivers+=" tcm_loop tcm_fc target_core_mod iscsi_target_mod
> target_core_file"
>     omit_drivers+=" target_core_iblock target_core_mod target_core_pscsi"
> 
> Commit where problem appeared (git bisect):
>     commit 326e1b8f83a4318b09033ef754f40c785aed5e68
>     Author: Dan Williams <dan.j.williams@intel.com
> <mailto:dan.j.williams@intel.com>>
>     Date:   Thu Jul 18 15:58:00 2019 -0700
> ------------------------------------------------------------------------------
> -rw------- 1 root root 2174976 Feb 28 13:33 vmcore-incomplete
> -rw-r--r-- 1 root root   87611 Feb 28 13:33 vmcore-dmesg.txt
> verses
> -rw------- 1 root root 1186929986 Mar  3 18:57 vmcore
> -rw-r--r-- 1 root root     181709 Mar  3 18:57 vmcore-dmesg.txt
> ------------------------------------------------------------------------------
> What am I breaking -- if I comment out the usage of early_section(ms)
> in file mm/sparse.c -- although kdump's seems to work fine, etc.
> //      bool section_is_early = early_section(ms);
> ...
> //              if (!section_is_early) {
>                         kfree(ms->usage);
>                         ms->usage = NULL;
> //              }
> ------------------------------------------------------------------------------
> Humor:
>     commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b (tag: v5.3-rc1)
>     Author: Linus Torvalds <torvalds@linux-foundation.org
> <mailto:torvalds@linux-foundation.org>>
>     Date:   Sun Jul 21 14:05:38 2019 -0700
>         Linus 5.3-rc1
>             ^
> Horror:
>     No one else has run into this since the end of July?
> ------------------------------------------------------------------------------
> Full git show below:
> 
> commit 326e1b8f83a4318b09033ef754f40c785aed5e68
> Author: Dan Williams <dan.j.williams@intel.com
> <mailto:dan.j.williams@intel.com>>
> Date:   Thu Jul 18 15:58:00 2019 -0700
> 
>     mm/sparsemem: introduce a SECTION_IS_EARLY flag
> 
>     In preparation for sub-section hotplug, track whether a given section
>     was created during early memory initialization, or later via memory
>     hotplug.  This distinction is needed to maintain the coarse expectation
>     that pfn_valid() returns true for any pfn within a given section even if
>     that section has pages that are reserved from the page allocator.
> 
>     For example one of the of goals of subsection hotplug is to support
>     cases where the system physical memory layout collides System RAM and
>     PMEM within a section.  Several pfn_valid() users expect to just check
>     if a section is valid, but they are not careful to check if the given
>     pfn is within a "System RAM" boundary and instead expect pgdat
>     information to further validate the pfn.
> 
>     Rather than unwind those paths to make their pfn_valid() queries more
>     precise a follow on patch uses the SECTION_IS_EARLY flag to maintain the
>     traditional expectation that pfn_valid() returns true for all early
>     sections.
> 
>     Link:
> https://lore.kernel.org/lkml/1560366952-10660-1-git-send-email-cai@lca.pw/
>     Link:
> http://lkml.kernel.org/r/156092350358.979959.5817209875548072819.stgit@dwillia2-desk3.amr.corp.intel.com
>     Signed-off-by: Dan Williams <dan.j.williams@intel.com
> <mailto:dan.j.williams@intel.com>>
>     Reported-by: Qian Cai <cai@lca.pw <mailto:cai@lca.pw>>
>     Tested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com
> <mailto:aneesh.kumar@linux.ibm.com>>        [ppc64]
>     Reviewed-by: Oscar Salvador <osalvador@suse.de
> <mailto:osalvador@suse.de>>
>     Cc: Michal Hocko <mhocko@suse.com <mailto:mhocko@suse.com>>
>     Cc: Logan Gunthorpe <logang@deltatee.com <mailto:logang@deltatee.com>>
>     Cc: David Hildenbrand <david@redhat.com <mailto:david@redhat.com>>
>     Cc: Pavel Tatashin <pasha.tatashin@soleen.com
> <mailto:pasha.tatashin@soleen.com>>
>     Cc: Jane Chu <jane.chu@oracle.com <mailto:jane.chu@oracle.com>>
>     Cc: Jeff Moyer <jmoyer@redhat.com <mailto:jmoyer@redhat.com>>
>     Cc: Jérôme Glisse <jglisse@redhat.com <mailto:jglisse@redhat.com>>
>     Cc: Jonathan Corbet <corbet@lwn.net <mailto:corbet@lwn.net>>
>     Cc: Mike Rapoport <rppt@linux.ibm.com <mailto:rppt@linux.ibm.com>>
>     Cc: Toshi Kani <toshi.kani@hpe.com <mailto:toshi.kani@hpe.com>>
>     Cc: Vlastimil Babka <vbabka@suse.cz <mailto:vbabka@suse.cz>>
>     Cc: Wei Yang <richardw.yang@linux.intel.com
> <mailto:richardw.yang@linux.intel.com>>
>     Cc: Jason Gunthorpe <jgg@mellanox.com <mailto:jgg@mellanox.com>>
>     Cc: Christoph Hellwig <hch@lst.de <mailto:hch@lst.de>>
>     Signed-off-by: Andrew Morton <akpm@linux-foundation.org
> <mailto:akpm@linux-foundation.org>>
>     Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org
> <mailto:torvalds@linux-foundation.org>>
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 2520336bdfd1..4be40634238b 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -1260,7 +1260,8 @@ extern size_t mem_section_usage_size(void);
>  #define        SECTION_MARKED_PRESENT  (1UL<<0)
>  #define SECTION_HAS_MEM_MAP    (1UL<<1)
>  #define SECTION_IS_ONLINE      (1UL<<2)
> -#define SECTION_MAP_LAST_BIT   (1UL<<3)
> +#define SECTION_IS_EARLY       (1UL<<3)
> +#define SECTION_MAP_LAST_BIT   (1UL<<4)
>  #define SECTION_MAP_MASK       (~(SECTION_MAP_LAST_BIT-1))
>  #define SECTION_NID_SHIFT      3
> 
> @@ -1286,6 +1287,11 @@ static inline int valid_section(struct
> mem_section *section)
>         return (section && (section->section_mem_map &
> SECTION_HAS_MEM_MAP));
>  }
> 
> +static inline int early_section(struct mem_section *section)
> +{
> +       return (section && (section->section_mem_map & SECTION_IS_EARLY));
> +}
> +
>  static inline int valid_section_nr(unsigned long nr)
>  {
>         return valid_section(__nr_to_section(nr));
> diff --git a/mm/sparse.c b/mm/sparse.c
> index 41bef8e1f65c..6d23a526279a 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -288,11 +288,11 @@ struct page *sparse_decode_mem_map(unsigned long
> coded_mem_map, unsigned long pn
> 
>  static void __meminit sparse_init_one_section(struct mem_section *ms,
>                 unsigned long pnum, struct page *mem_map,
> -               struct mem_section_usage *usage)
> +               struct mem_section_usage *usage, unsigned long flags)
>  {
>         ms->section_mem_map &= ~SECTION_MAP_MASK;
> -       ms->section_mem_map |= sparse_encode_mem_map(mem_map, pnum) |
> -                                                       SECTION_HAS_MEM_MAP;
> +       ms->section_mem_map |= sparse_encode_mem_map(mem_map, pnum)
> +               | SECTION_HAS_MEM_MAP | flags;
>         ms->usage = usage;
>  }
> 
> @@ -497,7 +497,8 @@ static void __init sparse_init_nid(int nid, unsigned
> long pnum_begin,
>                         goto failed;
>                 }
>                 check_usemap_section_nr(nid, usage);
> -               sparse_init_one_section(__nr_to_section(pnum), pnum,
> map, usage);
> +               sparse_init_one_section(__nr_to_section(pnum), pnum,
> map, usage,
> +                               SECTION_IS_EARLY);
>                 usage = (void *) usage + mem_section_usage_size();
>         }
>         sparse_buffer_fini();
> @@ -732,7 +733,7 @@ int __meminit sparse_add_one_section(int nid,
> unsigned long start_pfn,
> 
>         set_section_nid(section_nr, nid);
>         section_mark_present(ms);
> -       sparse_init_one_section(ms, section_nr, memmap, usage);
> +       sparse_init_one_section(ms, section_nr, memmap, usage, 0);
> 
>  out:
>         if (ret < 0) {
> @@ -772,19 +773,16 @@ static inline void clear_hwpoisoned_pages(struct
> page *memmap, int nr_pages)
>  }
>  #endif
> 
> -static void free_section_usage(struct page *memmap,
> +static void free_section_usage(struct mem_section *ms, struct page *memmap,
>                 struct mem_section_usage *usage, struct vmem_altmap *altmap)
>  {
> -       struct page *usage_page;
> -
>         if (!usage)
>                 return;
> 
> -       usage_page = virt_to_page(usage);
>         /*
>          * Check to see if allocation came from hot-plug-add
>          */
> -       if (PageSlab(usage_page) || PageCompound(usage_page)) {
> +       if (!early_section(ms)) {
>                 kfree(usage);
>                 if (memmap)
>                         __kfree_section_memmap(memmap, altmap);
> @@ -816,6 +814,6 @@ void sparse_remove_one_section(struct mem_section
> *ms, unsigned long map_offset,
> 
>         clear_hwpoisoned_pages(memmap + map_offset,
>                         PAGES_PER_SECTION - map_offset);
> -       free_section_usage(memmap, usage, altmap);
> +       free_section_usage(ms, memmap, usage, altmap);
>  }
>  #endif /* CONFIG_MEMORY_HOTPLUG */
> 

I am most probably missing some context here. But we do have

commit 8068df3b60373c390198f660574ea14c8098de57
Author: David Hildenbrand <david@redhat.com>
Date:   Mon Jan 13 16:29:07 2020 -0800

    mm/memory_hotplug: don't free usage map when removing a re-added
early section

Does that fix your issue?

-- 
Thanks,

David / dhildenb