From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 10F793C0C for ; Sat, 7 Mar 2026 08:35:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772872520; cv=none; b=nDnd/HH37oYuuNM4vKR+HXhrOgc683Go8X7t+jgi3qKQcrPN9M9w9sapDfN/OjTyHtMWSs2Cvl7SjEsOlW59UiuEn0WZUIH9DxxcuXHfMp8geY56fasmuBlVfUuskWNKjiutw8s41Eud8Ikh1KlCsrBL4DSZKaf6tyLDRiNF4W8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772872520; c=relaxed/simple; bh=IUec4phBo+gJNl11iivFJAzINunEnkm6/ApxvPCt44U=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Jbr2KHr+4Czw5W7SOew9SpdsEiUss08mSQo0t707WyUhVsz2Bm5bWo5cPSAtml8xIjN63ue1DYy65chj0w6EsSb7jVOUjq9fOGH1IrMGAycMHpU8byn2VKLktWCK43UcmmWRPTrCR8an9nivJeGazqhEc48SuKEECSilUWuV/ck= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=U1V06B4R; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="U1V06B4R" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A26D0C19422; Sat, 7 Mar 2026 08:35:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1772872519; bh=IUec4phBo+gJNl11iivFJAzINunEnkm6/ApxvPCt44U=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=U1V06B4RqNsdkoHtTSGDFOu1K3g3KGkd6lAeIujivtukZ7J0lvRKG8FmX86lql1vh 1EUL8kstKKHpQKm/+CTv5L/EkvH6PGZtqOuz8tPHMi2eERCtRe3+LzjKH9En+UAu8K MhGgl2wwBcfdm+V1sCnc/VgdIDPOm+Tl2Kn6TP59okq3OYhaVYAdF5HtL+Kfr9B97L bCuO4RMnZjwgszr6lY6UXIPxLuBN2UbHVtatHMB/88KvQHr8QRKGRtBqOnCHGAsMhh lGjv9yviIQgwyzjJFNE9IK+YnV54SMW+CahlWMKTzCqIhARqmsuAvq72JylB0dFqFE ClCPJHihLU8uQ== Date: Sat, 7 Mar 2026 10:35:13 +0200 From: Mike Rapoport To: "Guilherme G. Piccoli" Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-dev@igalia.com, kernel@gpiccoli.net, Andrew Morton , Steven Rostedt Subject: Re: [PATCH V2 2/2] mm/memblock: Add reserve_mem debugfs info Message-ID: References: <20260304203300.1414286-2-gpiccoli@igalia.com> <20260304203300.1414286-4-gpiccoli@igalia.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260304203300.1414286-4-gpiccoli@igalia.com> Hi Guilherme, On Wed, Mar 04, 2026 at 05:14:11PM -0300, Guilherme G. Piccoli wrote: > When using the "reserve_mem" parameter, users aim at having an > area that (hopefully) persists across boots, so pstore infrastructure > (like ramoops module) can make use of that to save oops/ftrace logs, > for example. > > There is no easy way to determine if this kernel parameter is properly > set though; the kernel doesn't show information about this memory in > memblock debugfs, neither in /proc/iomem nor dmesg. This is a relevant > information for tools like kdumpst[0], to determine if it's reliable > to use the reserved area as ramoops persistent storage; checking only > /proc/cmdline is not sufficient as it doesn't tell if the reservation > effectively succeeded or not. > > Add here a new file under memblock debugfs showing properly set memory > reservations, with name and size as passed to "reserve_mem". Notice that > if no "reserve_mem=" is passed on command-line or if the reservation > attempts fail, the file is not created. > > [0] https://aur.archlinux.org/packages/kdumpst > > Cc: Andrew Morton > Cc: Mike Rapoport > Cc: Steven Rostedt > Signed-off-by: Guilherme G. Piccoli > --- > > > Thanks a lot for the suggestions Mike! I'm not sure if you would > prefer a Co-Developed-by or Suggested-by instead of CC, you helped > a lot improving the code. Lemme know and either I can re-submit > (with potential other changes) or even, you can change while merging. > > V2: (all suggestions by Mike Rapoport) > > - Commit message (showing use case); > - Drop ifdef on include "string_helpers.h"; > - Don't show the address of reserve_mem, only name and size; > - Fixed flag names inside ARCH_KEEP_MEMBLOCK ifdef; > - Make use of its own show_attribute instead of refactoring > the memblock one; > - Use sizeof() instead of magical numbers for the size; > - Don't show memblock directory if no reserve_mem succeeded > and ARCH_KEEP_MEMBLOCK isn't defined (keeping current behavior). > > > mm/memblock.c | 37 +++++++++++++++++++++++++++++++++++-- > 1 file changed, 35 insertions(+), 2 deletions(-) > > diff --git a/mm/memblock.c b/mm/memblock.c > index 2d2646f7a120..d816796ab919 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -17,6 +17,7 @@ > #include > #include > #include > +#include This will break tests in tools/testing/memblock, please add a stub there. > #ifdef CONFIG_KEXEC_HANDOVER > #include > @@ -2711,7 +2712,8 @@ static int __init reserve_mem(char *p) > } > __setup("reserve_mem=", reserve_mem); > > -#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_ARCH_KEEP_MEMBLOCK) > +#ifdef CONFIG_DEBUG_FS > +#ifdef CONFIG_ARCH_KEEP_MEMBLOCK > static const char * const flagname[] = { > [ilog2(MEMBLOCK_HOTPLUG)] = "HOTPLUG", > [ilog2(MEMBLOCK_MIRROR)] = "MIRROR", > @@ -2758,10 +2760,40 @@ static int memblock_debug_show(struct seq_file *m, void *private) > } > DEFINE_SHOW_ATTRIBUTE(memblock_debug); > > +#endif /* CONFIG_ARCH_KEEP_MEMBLOCK */ > + > +static int memblock_reserve_mem_show(struct seq_file *m, void *private) > +{ > + struct reserve_mem_table *map; > + char txtsz[16]; > + > + for (int i = 0; i < reserved_mem_count; i++) { > + map = &reserved_mem_table[i]; > + if (!map->size) > + continue; > + > + memset(txtsz, 0, sizeof(txtsz)); > + string_get_size(map->size, 1, STRING_UNITS_2, txtsz, sizeof(txtsz)); > + seq_printf(m, "%s\t\t(%s)\n", map->name, txtsz); > + } > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(memblock_reserve_mem); > + > static int __init memblock_init_debugfs(void) > { > - struct dentry *root = debugfs_create_dir("memblock", NULL); > + struct dentry *root; > > + if (!(IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK) || reserved_mem_count)) > + return 0; > + > + root = debugfs_create_dir("memblock", NULL); > + > + if (reserved_mem_count) > + debugfs_create_file("reserve_mem_param", 0444, root, NULL, > + &memblock_reserve_mem_fops); > +#ifdef CONFIG_ARCH_KEEP_MEMBLOCK > debugfs_create_file("memory", 0444, root, > &memblock.memory, &memblock_debug_fops); > debugfs_create_file("reserved", 0444, root, > @@ -2771,6 +2803,7 @@ static int __init memblock_init_debugfs(void) > &memblock_debug_fops); > #endif > > +#endif /* CONFIG_ARCH_KEEP_MEMBLOCK */ It becomes too #ifdef'y :( Let's put the creation of these files into a helper and add an empty stub when ARCH_KEEP_MEMBLOCK=n. > return 0; > } > __initcall(memblock_init_debugfs); > -- > 2.50.1 > -- Sincerely yours, Mike.