Re: [PATCH 1/3] memblock: define functions to set the usable memory range
On Thu, Jan 13, 2022 at 07:33:11PM +0200, Mike Rapoport wrote: > On Tue, Jan 11, 2022 at 08:44:41PM +, Frank van der Linden wrote: > > On Tue, Jan 11, 2022 at 12:31:58PM +0200, Mike Rapoport wrote: > > > > --- a/include/linux/memblock.h > > > > +++ b/include/linux/memblock.h > > > > @@ -481,6 +481,8 @@ phys_addr_t memblock_reserved_size(void); > > > > phys_addr_t memblock_start_of_DRAM(void); > > > > phys_addr_t memblock_end_of_DRAM(void); > > > > void memblock_enforce_memory_limit(phys_addr_t memory_limit); > > > > +void memblock_set_usable_range(phys_addr_t base, phys_addr_t size); > > > > +void memblock_enforce_usable_range(void); > > > > void memblock_cap_memory_range(phys_addr_t base, phys_addr_t size); > > > > void memblock_mem_limit_remove_map(phys_addr_t limit); > > > > > > We already have 3 very similar interfaces that deal with memory capping. > > > Now you suggest to add fourth that will "generically" solve a single use > > > case of DT, EFI and kdump interaction on arm64. > > > > > > Looks like a workaround for a fundamental issue of incompatibility between > > > DT and EFI wrt memory registration. > > > > Yep, I figured this would be the main argument against this - arm64 > > already added several other more-or-less special cased interfaces over > > time. > > > > I'm more than happy to solve this in a different way. > > > > What would you suggest: > > > > 1) Try to merge the similar interfaces in to one. > > 2) Just deal with it at a lower (arm64) level? > > 3) Some other way? > > We've discussed this with Ard on IRC, and our conclusion was that on arm64 > kdump kernel should have memblock.memory exactly the same as the normal > kernel. Then, the memory outside usable-memory-range should be reserved so > that kdump kernel won't step over it. > > With that, simple (untested) patch below could be what we need: > > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > index bdca35284ceb..371418dffaf1 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -1275,7 +1275,8 @@ void __init early_init_dt_scan_nodes(void) > of_scan_flat_dt(early_init_dt_scan_memory, NULL); > > /* Handle linux,usable-memory-range property */ > - memblock_cap_memory_range(cap_mem_addr, cap_mem_size); > + memblock_reserve(0, cap_mem_addr); > + memblock_reserve(cap_mem_addr + cap_mem_size, PHYS_ADDR_MAX); > } > > bool __init early_init_dt_scan(void *params) Ok, tested this on 5.17-rc, and it's working OK there. Main kernel has 32G, crash kernel gets 512M: Main kernel: [0.00] Zone ranges: [0.00] DMA [mem 0x4000-0x] [0.00] DMA32empty [0.00] Normal [mem 0x0001-0x000b96ff] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x4000-0x786e] [0.00] node 0: [mem 0x786f-0x7872] [0.00] node 0: [mem 0x7873-0x7bbf] [0.00] node 0: [mem 0x7bc0-0x7bfd] [0.00] node 0: [mem 0x7bfe-0x7fff] [0.00] node 0: [mem 0x0004-0x000b96ff] [0.00] Initmem setup node 0 [mem 0x4000-0x000b96ff] [0.00] On node 0, zone Normal: 4096 pages in unavailable ranges [0.00] cma: Reserved 64 MiB at 0x7c00 [0.00] crashkernel reserved: 0x5440 - 0x7440 (512 MB) Crash kernel: [0.00] Zone ranges: [0.00] DMA [mem 0x5440-0x7bfd] [0.00] DMA32empty [0.00] Normal empty [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x5440-0x743f] [0.00] node 0: [mem 0x786f-0x7872] [0.00] node 0: [mem 0x7bc0-0x7bfd] [0.00] Initmem setup node 0 [mem 0x5440-0x7bfd] [0.00] On node 0, zone DMA: 17408 pages in unavailable ranges [0.00] On node 0, zone DMA: 17136 pages in unavailable ranges [0.00] On node 0, zone DMA: 13520 pages in unavailable ranges [0.00] On node 0, zone DMA: 16416 pages in unavailable ranges Not sure why I had trouble with the same on 5.15, I'll have to look at that again. But this seems fine for 5.16+ Thanks, - Frank ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V4] notifier/panic: Introduce panic_notifier_filter
Hey folks, sorry for the ping. But is there any extra reviews? All comments are much appreciated! Dave, what do you think about the patch? I remember we talked about it in [0], seems you considered that a good idea right? Thanks in advance, Guilherme [0] https://lore.kernel.org/lkml/yckaz79zg5hde...@dhcp-128-65.nay.redhat.com/ ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH V3] panic: Move panic_print before kmsg dumpers
The panic_print setting allows users to collect more information in a panic event, like memory stats, tasks, CPUs backtraces, etc. This is a pretty interesting debug mechanism, but currently the print event happens *after* kmsg_dump(), meaning that pstore, for example, cannot collect a dmesg with the panic_print information. This patch changes that in 2 ways: (a) First, after a good discussion with Petr in the mailing-list[0], he noticed that one specific setting of panic_print (the console replay, bit 5) should not be executed before console proper flushing; hence we hereby split the panic_print_sys_info() function in upper and lower portions: if the parameter "after_kmsg_dumpers" is passed, only bit 5 (the console replay thing) is evaluated and the function returns - this is the lower portion. Otherwise all other bits are checked and the function prints the user required information; this is the upper/earlier mode. (b) With the above change, we can safely insert a panic_print_sys_info() call up some lines, in order kmsg_dump() accounts this new information and exposes it through pstore or other kmsg dumpers. Notice that this new earlier call doesn't set "after_kmsg_dumpers" so we print the information set by user in panic_print, except the console replay that, if set, will be executed after the console flushing. Also, worth to notice we needed to guard the panic_print_sys_info(false) calls against double print - we use kexec_crash_loaded() helper for that (see discussion [1] for more details). In the first version of this patch we discussed the risks in the mailing-list [0], and seems this approach is the least terrible, despite still having risks of polluting the log with the bulk of information that panic_print may bring, losing older messages. In order to attenuate that, we've added a warning in the kernel-parameters.txt so that users enabling this mechanism consider to increase the log buffer size via "log_buf_len" as well. Finally, another decision was to keep 2 panic_print_sys_info(false) calls (instead of just bringing it up some code lines and keep a single call) due to the panic notifiers: if kdump is not set, currently the panic_print information is collected after the notifiers and since it's a bit safer this way, we decided to keep it as is, only modifying the kdump case as per the previous commit [2] (see more details about this discussion also in thread [1]). [0] https://lore.kernel.org/lkml/20211230161828.121858-1-gpicc...@igalia.com [1] https://lore.kernel.org/lkml/f25672a4-e4dd-29e8-b2db-f92dd9ff9...@igalia.com [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=5613b7538f69 Cc: Feng Tang Cc: Petr Mladek Signed-off-by: Guilherme G. Piccoli --- V3: Added a guard in the 2nd panic_print_sys_info(false) to prevent double print - thanks for catching this Petr! I didn't implement your final suggestion Petr, i.e., putting the first panic_print_sys_info(false) inside the if (!_crash_kexec_post_notifiers) block, and the reason is that when we do this, there's 4 cases to consider: !kexec_crash_load() && !_crash_kexec_post_notifiers kexec_crash_load() && !_crash_kexec_post_notifiers kexec_crash_load() && _crash_kexec_post_notifiers !kexec_crash_load() && _crash_kexec_post_notifiers The 3rd case, which means user enabled kdump and set the post_notifiers in the cmdline fails - we end-up not reaching panic_print_sys_info(false) in this case, unless we add another variable to track the function call and prevent double print. My preference was to keep the first call as introduced by commit [2] (mentioned above) and not rely in another variable. Thanks again for the great reviews, Guilherme V2: https://lore.kernel.org/lkml/20220106212835.119409-1-gpicc...@igalia.com .../admin-guide/kernel-parameters.txt | 4 kernel/panic.c| 22 ++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index a069d8fe2fee..0f5cbe141bfd 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3727,6 +3727,10 @@ bit 4: print ftrace buffer bit 5: print all printk messages in buffer bit 6: print all CPUs backtrace (if available in the arch) + *Be aware* that this option may print a _lot_ of lines, + so there are risks of losing older messages in the log. + Use this option carefully, maybe worth to setup a + bigger log buffer with "log_buf_len" along with this. panic_on_taint= Bitmask for conditionally calling panic() in add_taint() Format: [,nousertaint] diff --git a/kernel/panic.c b/kernel/panic.c index 41ecf9ab824a..4ae712665f75 100644 ---
Re: [PATCH v3 0/5] s390: add support for extended cmdline length
On Thu, Dec 16, 2021 at 12:43:51PM +0100, Sven Schnelle wrote:
> Hi,
>
> this patchset adds support for extended command line lengths on s390.
> The former limit of 896 has been too short in a some configurations. The
> linux kernel now provides an additional field in the parameter area
> which contains the maximum allowed command line length. In older kernels
> this field is zero. In that case the old limit of 896 bytes is used.
> This was introduced with 5ecb2da660ab ("s390: support command lines
> longer than 896 bytes") in linux.
>
> And while at it, also add the --reuse-cmdline option and use
> KEXEC_ALL_OPTIONS.
>
> Changes in v3:
> - handle short reads and interruptions in slurp_proc_file()
>
> Changes in v2:
>
> - add slurp_proc_file() to read proc files without knowing the size upfront
> - move command_line string from global to function scope
Thanks, applied.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 1/1] s390: handle R_390_PLT32DBL reloc entries in machine_apply_elf_rel()
On Mon, Jan 10, 2022 at 06:26:54PM +0100, Heiko Carstens wrote: > On Mon, Jan 10, 2022 at 04:23:05PM +0100, Alexander Egorenkov wrote: > > Heiko Carstens writes: > > > Given that Alexander is currently not available, I will resend his > > > patch with an updated commit message. > > > > Many thanks for the review and taking care of the review finding. > > But it looks like this still hasn't been applied to the kexec-tools > git tree, unless I'm mistaken(?) Sorry for the delay, I have now applied v2. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v5] arm64: support more than one crash kernel regions
On Mon, Jan 10, 2022 at 06:20:08PM +0800, Zhen Lei wrote: > From: Chen Zhou > > When crashkernel is reserved above 4G in memory, kernel should > reserve some amount of low memory for swiotlb and some DMA buffers. > So there may be two crash kernel regions, one is below 4G, the other > is above 4G. > > Currently, there is only one crash kernel region on arm64, and pass > "linux,usable-memory-range = " property to crash dump > kernel. > Now, we pass "linux,usable-memory-range = " > to crash dump kernel to support two crash kernel regions and load crash > kernel high. Make the low memory region as the second range "BASE2 SIZE2" > to keep compatibility with existing user-space and older kdump kernels. > > Signed-off-by: Chen Zhou > Co-developed-by: Zhen Lei > Signed-off-by: Zhen Lei Thanks, applied. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv2 1/1] s390: handle R_390_PLT32DBL reloc entries in machine_apply_elf_rel()
On Wed, Dec 15, 2021 at 06:48:53PM +0100, Heiko Carstens wrote: > From: Alexander Egorenkov > > Starting with gcc 11.3, the C compiler will generate PLT-relative function > calls even if they are local and do not require it. Later on during linking, > the linker will replace all PLT-relative calls to local functions with > PC-relative ones. Unfortunately, the purgatory code of kexec/kdump is > not being linked as a regular executable or shared library would have been, > and therefore, all PLT-relative addresses remain in the generated purgatory > object code unresolved. This in turn lets kexec-tools fail with > "Unknown rela relocation: 0x14 0x73c0901c" for such relocation types. > > Furthermore, the clang C compiler has always behaved like described above > and this commit should fix the purgatory code built with the latter. > > Because the purgatory code is no regular executable or shared library, > contains only calls to local functions and has no PLT, all R_390_PLT32DBL > relocation entries can be resolved just like a R_390_PC32DBL one. > > * > https://refspecs.linuxfoundation.org/ELF/zSeries/lzsabi0_zSeries/x1633.html#AEN1699 > > Relocation entries of purgatory code generated with gcc 11.3 > > > $ readelf -r purgatory/purgatory.o > > Relocation section '.rela.text' at offset 0x6e8 contains 27 entries: > Offset Info Type Sym. ValueSym. Name + > Addend > 000c 00030013 R_390_PC32DBL .data + 2 > 001a 00100014 R_390_PLT32DBL sha256_starts + > 2 > 0030 00110014 R_390_PLT32DBL sha256_update + > 2 > 0046 00120014 R_390_PLT32DBL sha256_finish + > 2 > 0050 00030013 R_390_PC32DBL .data + 102 > 005a 00130014 R_390_PLT32DBL memcmp + 2 > ... > 0118 00160014 R_390_PLT32DBL setup_arch + 2 > 011e 00030013 R_390_PC32DBL .data + 2 > 012c 000f0014 R_390_PLT32DBL > verify_sha256_digest + 2 > 0142 00170014 R_390_PLT32DBL > post_verification[...] + 2 > > Relocation entries of purgatory code generated with gcc 11.2 > > > $ readelf -r purgatory/purgatory.o > > Relocation section '.rela.text' at offset 0x6e8 contains 27 entries: > Offset Info Type Sym. ValueSym. Name + > Addend > 000e 00030013 R_390_PC32DBL .data + 2 > 001c 00100013 R_390_PC32DBL sha256_starts + > 2 > 0036 00110013 R_390_PC32DBL sha256_update + > 2 > 0048 00120013 R_390_PC32DBL sha256_finish + > 2 > 0052 00030013 R_390_PC32DBL .data + 102 > 005c 00130013 R_390_PC32DBL memcmp + 2 > ... > 011a 00160013 R_390_PC32DBL setup_arch + 2 > 0120 00030013 R_390_PC32DBL .data + 122 > 0130 000f0013 R_390_PC32DBL > verify_sha256_digest + 2 > 0146 00170013 R_390_PC32DBL > post_verification[...] + 2 > > Corresponding s390 kernel discussion: > * > https://lore.kernel.org/linux-s390/20211208105801.188140-1-egore...@linux.ibm.com/T/#u > > Signed-off-by: Alexander Egorenkov > Reported-by: Tao Liu > Suggested-by: Philipp Rudo > Reviewed-by: Philipp Rudo > [h...@linux.ibm.com: changed commit message as requested by Philipp Rudo] > Signed-off-by: Heiko Carstens > --- > kexec/arch/s390/kexec-elf-rel-s390.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Sorry for the delay, applied. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv3 0/4] arm64: make phys_to_virt() correct
Hi Pingfan, On Tue, Dec 28, 2021 at 09:26:55PM +0800, Pingfan Liu wrote: > Currently phys_to_virt() does not work well on 52-bits VA arm64 kernel. > One issue is contributed by phys_offset not signed. > The other is contributed by wrong page_offset. > > v2 -> v3: > Discussed with Kairui off-list, 48-bits VA kernel can not handle flipped > mm yet. So introducing [4/4], which judges whether the kernel is with > flipped mm layout and adopt different formula accordingly. > > As for [1-3/4], they are the same as [1-3/3] in V2. > > v1 -> v2 > Fix broken patch [2/3] in v1 > Move arch_scan_vmcoreinfo declaration to util_lib/include/elf_info.h > Using UINT64_MAX instread of 0x Thanks for the updated patches and sorry for the delay in reviewing them. I responded with some minor nits on patches 1/4 and 2/4. The other patches 3/4 and 4/4 look good to me. Modulo any feedback from others I'd be happy to move forwards with applying this series once my nits have been addressed (via email or fresh patches). Thanks again, Simon ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv3 2/4] arm64/crashdump: unify routine to get page_offset
On Tue, Dec 28, 2021 at 09:26:57PM +0800, Pingfan Liu wrote:
> There are two funcs to get page_offset:
> get_kernel_page_offset()
> get_page_offset()
>
> Since get_kernel_page_offset() does not observe the kernel formula, and
> remove it. Unify them in order to introduce 52-bits VA kernel more
> easily in the coming patch.
>
> Signed-off-by: Pingfan Liu
> Cc: Kairui Song
> Cc: Simon Horman
> Cc: Philipp Rudo
> To: kexec@lists.infradead.org
> ---
> kexec/arch/arm64/crashdump-arm64.c | 23 +--
> kexec/arch/arm64/kexec-arm64.c | 8
> kexec/arch/arm64/kexec-arm64.h | 1 +
> 3 files changed, 6 insertions(+), 26 deletions(-)
>
> diff --git a/kexec/arch/arm64/crashdump-arm64.c
> b/kexec/arch/arm64/crashdump-arm64.c
> index a02019a..0a8d44c 100644
> --- a/kexec/arch/arm64/crashdump-arm64.c
> +++ b/kexec/arch/arm64/crashdump-arm64.c
> @@ -46,27 +46,6 @@ static struct crash_elf_info elf_info = {
> .machine= EM_AARCH64,
> };
>
> -/*
> - * Note: The returned value is correct only if !CONFIG_RANDOMIZE_BASE.
> - */
> -static uint64_t get_kernel_page_offset(void)
> -{
> - int i;
> -
> - if (elf_info.kern_vaddr_start == UINT64_MAX)
> - return UINT64_MAX;
> -
> - /* Current max virtual memory range is 48-bits. */
> - for (i = 48; i > 0; i--)
> - if (!(elf_info.kern_vaddr_start & (1UL << i)))
> - break;
> -
> - if (i <= 0)
> - return UINT64_MAX;
> - else
> - return UINT64_MAX << i;
> -}
> -
> /*
> * iomem_range_callback() - callback called for each iomem region
> * @data: not used
> @@ -198,7 +177,7 @@ int load_crashdump_segments(struct kexec_info *info)
> if (err)
> return EFAILED;
>
> - elf_info.page_offset = get_kernel_page_offset();
> + get_page_offset(_info.page_offset);
> dbgprintf("%s: page_offset: %016llx\n", __func__,
> elf_info.page_offset);
>
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 7373fa3..037e51c 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -909,7 +909,7 @@ static int get_va_bits(void)
> * get_page_offset - Helper for getting PAGE_OFFSET
> */
>
> -static int get_page_offset(void)
> +int get_page_offset(unsigned long *page_offset)
> {
> int ret;
>
> @@ -917,8 +917,8 @@ static int get_page_offset(void)
> if (ret < 0)
> return ret;
>
> - page_offset = (0xUL) << (va_bits - 1);
> - dbgprintf("page_offset : %lx\n", page_offset);
> + *page_offset = (0xUL) << (va_bits - 1);
As this line is being updated, perhaps we could also make use of
UINT64_MAX or ULONG_MAX here.
> + dbgprintf("page_offset : %lx\n", *page_offset);
>
> return 0;
> }
> @@ -954,7 +954,7 @@ int get_phys_base_from_pt_load(long *phys_offset)
> unsigned long long phys_start;
> unsigned long long virt_start;
>
> - ret = get_page_offset();
> + ret = get_page_offset(_offset);
> if (ret < 0)
> return ret;
>
> diff --git a/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
> index 1844b67..ed99d9d 100644
> --- a/kexec/arch/arm64/kexec-arm64.h
> +++ b/kexec/arch/arm64/kexec-arm64.h
> @@ -69,6 +69,7 @@ extern struct arm64_mem arm64_mem;
>
> uint64_t get_phys_offset(void);
> uint64_t get_vp_offset(void);
> +int get_page_offset(unsigned long *offset);
>
> static inline void reset_vp_offset(void)
> {
> --
> 2.31.1
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCHv3 1/4] arm64: make phys_offset signed
On Tue, Dec 28, 2021 at 09:26:56PM +0800, Pingfan Liu wrote:
> After kernel commit 7bc1a0f9e176 ("arm64: mm: use single quantity to
> represent the PA to VA translation"), phys_offset can be negative if
> running 52-bits kernel on 48-bits hardware.
>
> So changing phys_offset from unsigned to signed.
>
> Signed-off-by: Pingfan Liu
> Cc: Kairui Song
> Cc: Simon Horman
> Cc: Philipp Rudo
> To: kexec@lists.infradead.org
> ---
> kexec/arch/arm64/kexec-arm64.c | 8
> kexec/arch/arm64/kexec-arm64.h | 2 +-
> util_lib/elf_info.c| 2 +-
> util_lib/include/elf_info.h| 2 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/kexec/arch/arm64/kexec-arm64.c b/kexec/arch/arm64/kexec-arm64.c
> index 6f572ed..7373fa3 100644
> --- a/kexec/arch/arm64/kexec-arm64.c
> +++ b/kexec/arch/arm64/kexec-arm64.c
> @@ -859,7 +859,7 @@ void add_segment(struct kexec_info *info, const void
> *buf, size_t bufsz,
> add_segment_phys_virt(info, buf, bufsz, base, memsz, 1);
> }
>
> -static inline void set_phys_offset(uint64_t v, char *set_method)
> +static inline void set_phys_offset(long v, char *set_method)
Perhaps it would be more in keeping with the current implementation
to use int64_t rather than long ?
> {
> if (arm64_mem.phys_offset == arm64_mem_ngv
> || v < arm64_mem.phys_offset) {
> @@ -928,7 +928,7 @@ static int get_page_offset(void)
> * from VMCOREINFO note inside 'kcore'.
> */
>
> -static int get_phys_offset_from_vmcoreinfo_pt_note(unsigned long
> *phys_offset)
> +static int get_phys_offset_from_vmcoreinfo_pt_note(long *phys_offset)
> {
> int fd, ret = 0;
>
> @@ -948,7 +948,7 @@ static int
> get_phys_offset_from_vmcoreinfo_pt_note(unsigned long *phys_offset)
> * from PT_LOADs inside 'kcore'.
> */
>
> -int get_phys_base_from_pt_load(unsigned long *phys_offset)
> +int get_phys_base_from_pt_load(long *phys_offset)
> {
> int i, fd, ret;
> unsigned long long phys_start;
> @@ -997,7 +997,7 @@ static bool to_be_excluded(char *str)
> int get_memory_ranges(struct memory_range **range, int *ranges,
> unsigned long kexec_flags)
> {
> - unsigned long phys_offset = UINT64_MAX;
> + long phys_offset = (long)UINT64_MAX;
Effectively this sets phys_offset to -1.
Is that ok / intended ? If so, perhaps it's clearer to just use -1.
Regardless, can clean up other uses of long with UINT64_MAX introduced
by this patch?
I see the following:
make
...
kexec/arch/arm64/kexec-arm64.c: In function ‘get_memory_ranges’:
kexec/arch/arm64/kexec-arm64.c:1022:20: warning: comparison between signed and u
nsigned integer expressions [-Wsign-compare]
if (phys_offset != UINT64_MAX)
...
kexec/arch/arm64/kexec-arm64.c:1034:21: warning: comparison between signed and
unsigned integer expressions [-Wsign-compare]
if (phys_offset != UINT64_MAX)
> FILE *fp;
> const char *iomem = proc_iomem();
> char line[MAX_LINE], *str;
> diff --git a/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
> index ed447ac..1844b67 100644
> --- a/kexec/arch/arm64/kexec-arm64.h
> +++ b/kexec/arch/arm64/kexec-arm64.h
> @@ -58,7 +58,7 @@ extern off_t initrd_size;
> */
>
> struct arm64_mem {
> - uint64_t phys_offset;
> + long phys_offset;
> uint64_t text_offset;
> uint64_t image_size;
> uint64_t vp_offset;
> diff --git a/util_lib/elf_info.c b/util_lib/elf_info.c
> index 51d8b92..5574c7f 100644
> --- a/util_lib/elf_info.c
> +++ b/util_lib/elf_info.c
> @@ -1236,7 +1236,7 @@ int read_elf(int fd)
> return 0;
> }
>
> -int read_phys_offset_elf_kcore(int fd, unsigned long *phys_off)
> +int read_phys_offset_elf_kcore(int fd, long *phys_off)
> {
> int ret;
>
> diff --git a/util_lib/include/elf_info.h b/util_lib/include/elf_info.h
> index 4bc9279..f550d86 100644
> --- a/util_lib/include/elf_info.h
> +++ b/util_lib/include/elf_info.h
> @@ -28,7 +28,7 @@ int get_pt_load(int idx,
> unsigned long long *phys_end,
> unsigned long long *virt_start,
> unsigned long long *virt_end);
> -int read_phys_offset_elf_kcore(int fd, unsigned long *phys_off);
> +int read_phys_offset_elf_kcore(int fd, long *phys_off);
> int read_elf(int fd);
> void dump_dmesg(int fd, void (*handler)(char*, unsigned int));
>
> --
> 2.31.1
>
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V2] panic: Move panic_print before kmsg dumpers
On 14/01/2022 09:26, Petr Mladek wrote:
> [...]
> I see. OK, it makes sense to call it after the panic notifiers when
> they are used. It would be nice to mention the above in the commit
> message and explain why the 2nd call is there.
>
> Just an idea. It might be better to move the 1st call below
> if (!_crash_kexec_post_notifiers). It would make it more
> clear that it is intended for this code path. I mean:
>
> if (!_crash_kexec_post_notifiers) {
> /* ... */
> if (kexec_crash_loaded())
> panic_print_sys_info(false);
>
> __crash_kexec(NULL);
> ...
>
> Best Regards,
> Petr
Perfect Petr, thanks again for the very good ideas! I'll work on V3 and
submit today.
Cheers,
Guilherme
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH V2] panic: Move panic_print before kmsg dumpers
On Thu 2022-01-13 12:15:08, Guilherme G. Piccoli wrote:
> On 13/01/2022 11:22, Petr Mladek wrote:
> > [...]
> > OK, do we have any specific reason why panic_print_sys_info()
> > should get called right before kmsg_dump() when this code patch
> > is used?
> >
> > Alternative solution would be to remove the check of
> > kexec_crash_loaded() and always call panic_print_sys_info(false)
> > at the beginning (after kgdb_panic(buf)).
> >
> > The advantage is that panic_print_sys_info(false) will be always
> > called on the same location. It will give the same results
> > in all code paths so that it will be easier to interpret them.
> > And it will have the same problems so it should be easier
> > to debug and maintain.
> >
> > It is possible that it will not work for some users. Also it is
> > possible that it might cause some problems. But it is hard to
> > guess at least for me.
> >
> > I think that we might try it and see if anyone complains.
> > Honestly, I think that only few people use panic_printk_sys_info().
> > And your use-case makes sense.
> >
> > Best Regards,
> > Petr
>
> Hi Petr, thanks for your idea - it's simple and effective, would resolve
> the problems in a straightforward way. But there are some cons, let me
> detail more.
>
> Currently (in linux-next), if users set panic_print but not kdump, the
> panic_print_sys_info() is called *after* the panic notifiers. If user
> has kdump configured and still sets panic_print (which is our use case),
> then panic_print_sys_info() is called _before_ the panic notifiers. In
> other words, the behavior for non-kdump users is the same as before,
> since the merge of panic_print functionality.
>
> Your idea brings the printing earlier, always before the panic
> notifiers. This isn't terrible, but might lead to unfortunate and
> hard-to-debug problems; for example, some panic notifiers are
> rcu_panic() and hung_task_panic(), both are simple functions to disable
> RCU warnings and hung task detector in panic scenarios. If we go with
> your idea, these won't get called before panic_print_sys_info(), even if
> kdump is not set. So, since the panic_print triggers a lot of printing
> in the console, we could face a stall and trigger RCU messages, maybe
> intermixed with the panic_print information.
I see. OK, it makes sense to call it after the panic notifiers when
they are used. It would be nice to mention the above in the commit
message and explain why the 2nd call is there.
Just an idea. It might be better to move the 1st call below
if (!_crash_kexec_post_notifiers). It would make it more
clear that it is intended for this code path. I mean:
if (!_crash_kexec_post_notifiers) {
/* ... */
if (kexec_crash_loaded())
panic_print_sys_info(false);
__crash_kexec(NULL);
...
Best Regards,
Petr
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
