* [RFC 1/5] ext4: mark inode format migration fast-commit ineligible
From: Li Chen @ 2025-12-11 11:51 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, linux-ext4, linux-kernel,
linux-trace-kernel
Cc: Li Chen
In-Reply-To: <20251211115146.897420-1-me@linux.beauty>
Fast commits only log operations that have dedicated replay support.
Inode format migration (indirect<->extent layout changes via
EXT4_IOC_MIGRATE or toggling EXT4_EXTENTS_FL) rewrites the block mapping
representation without going through the fast commit tracking paths.
In practice these migrations are rare and usually followed by further
updates, but mixing them into a fast commit makes the overall semantics
harder to reason about and risks replay gaps if new call sites appear.
Teach ext4 to mark the filesystem fast-commit ineligible when
ext4_ext_migrate() or ext4_ind_migrate() start their journal transactions.
This forces those transactions to fall back to a full commit, ensuring
that the entire inode layout change is captured by the normal journal
rather than partially encoded in fast commit TLVs. This change should
not affect common workloads but makes format migrations safer and easier
to reason about under fast commit.
Testing:
1. prepare:
dd if=/dev/zero of=/root/fc.img bs=1M count=0 seek=128
mkfs.ext4 -O fast_commit -F /root/fc.img
mkdir -p /mnt/fc && mount -t ext4 -o loop /root/fc.img /mnt/fc
2. Created a test file and toggled the extents flag to exercise both
ext4_ind_migrate() and ext4_ext_migrate():
touch /mnt/fc/migtest
chattr -e /mnt/fc/migtest
chattr +e /mnt/fc/migtest
3. Verified fast-commit ineligible statistics:
tail -n 1 /proc/fs/ext4/loop0/fc_info
"Inode format migration": 2
Signed-off-by: Li Chen <me@linux.beauty>
---
fs/ext4/fast_commit.c | 1 +
fs/ext4/fast_commit.h | 1 +
fs/ext4/migrate.c | 12 ++++++++++++
include/trace/events/ext4.h | 4 +++-
4 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index fa66b08de999..afb28b3e52bb 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -2302,6 +2302,7 @@ static const char * const fc_ineligible_reasons[] = {
[EXT4_FC_REASON_FALLOC_RANGE] = "Falloc range op",
[EXT4_FC_REASON_INODE_JOURNAL_DATA] = "Data journalling",
[EXT4_FC_REASON_ENCRYPTED_FILENAME] = "Encrypted filename",
+ [EXT4_FC_REASON_MIGRATE] = "Inode format migration",
};
int ext4_fc_info_show(struct seq_file *seq, void *v)
diff --git a/fs/ext4/fast_commit.h b/fs/ext4/fast_commit.h
index 3bd534e4dbbf..be3b84a74c32 100644
--- a/fs/ext4/fast_commit.h
+++ b/fs/ext4/fast_commit.h
@@ -97,6 +97,7 @@ enum {
EXT4_FC_REASON_FALLOC_RANGE,
EXT4_FC_REASON_INODE_JOURNAL_DATA,
EXT4_FC_REASON_ENCRYPTED_FILENAME,
+ EXT4_FC_REASON_MIGRATE,
EXT4_FC_REASON_MAX
};
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c
index 1b0dfd963d3f..96ab95167bd6 100644
--- a/fs/ext4/migrate.c
+++ b/fs/ext4/migrate.c
@@ -449,6 +449,12 @@ int ext4_ext_migrate(struct inode *inode)
retval = PTR_ERR(handle);
goto out_unlock;
}
+ /*
+ * This operation rewrites the inode's block mapping layout
+ * (indirect to extents) and is not tracked in the fast commit
+ * log, so disable fast commits for this transaction.
+ */
+ ext4_fc_mark_ineligible(inode->i_sb, EXT4_FC_REASON_MIGRATE, handle);
goal = (((inode->i_ino - 1) / EXT4_INODES_PER_GROUP(inode->i_sb)) *
EXT4_INODES_PER_GROUP(inode->i_sb)) + 1;
owner[0] = i_uid_read(inode);
@@ -630,6 +636,12 @@ int ext4_ind_migrate(struct inode *inode)
ret = PTR_ERR(handle);
goto out_unlock;
}
+ /*
+ * This operation rewrites the inode's block mapping layout
+ * (extents to indirect blocks) and is not tracked in the fast
+ * commit log, so disable fast commits for this transaction.
+ */
+ ext4_fc_mark_ineligible(inode->i_sb, EXT4_FC_REASON_MIGRATE, handle);
down_write(&EXT4_I(inode)->i_data_sem);
ret = ext4_ext_check_inode(inode);
diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
index a374e7ea7e57..8f75d41ae5ef 100644
--- a/include/trace/events/ext4.h
+++ b/include/trace/events/ext4.h
@@ -102,6 +102,7 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_RENAME_DIR);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_FALLOC_RANGE);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_INODE_JOURNAL_DATA);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_ENCRYPTED_FILENAME);
+TRACE_DEFINE_ENUM(EXT4_FC_REASON_MIGRATE);
TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX);
#define show_fc_reason(reason) \
@@ -115,7 +116,8 @@ TRACE_DEFINE_ENUM(EXT4_FC_REASON_MAX);
{ EXT4_FC_REASON_RENAME_DIR, "RENAME_DIR"}, \
{ EXT4_FC_REASON_FALLOC_RANGE, "FALLOC_RANGE"}, \
{ EXT4_FC_REASON_INODE_JOURNAL_DATA, "INODE_JOURNAL_DATA"}, \
- { EXT4_FC_REASON_ENCRYPTED_FILENAME, "ENCRYPTED_FILENAME"})
+ { EXT4_FC_REASON_ENCRYPTED_FILENAME, "ENCRYPTED_FILENAME"}, \
+ { EXT4_FC_REASON_MIGRATE, "MIGRATE"})
TRACE_DEFINE_ENUM(CR_POWER2_ALIGNED);
TRACE_DEFINE_ENUM(CR_GOAL_LEN_FAST);
--
2.51.0
^ permalink raw reply related
* [RFC 0/5] ext4: mark more ops fast-commit ineligible
From: Li Chen @ 2025-12-11 11:51 UTC (permalink / raw)
To: Theodore Ts'o, Andreas Dilger, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, linux-ext4, linux-kernel,
linux-trace-kernel
ext4 fast commit only logs operations with replay support. This series
marks a few more operations as fast-commit ineligible and accounts
them via fc_info so behaviour under fast commit is easier to reason
about.
Testing was done in a QEMU guest on loopback ext4 filesystems created
with -O fast_commit[/,verity] by exercising each operation and checking
/proc/fs/ext4/*/fc_info for the corresponding ineligible reason and
ineligible commit counters. Detailed steps are in each commit's message.
Li Chen (5):
ext4: mark inode format migration fast-commit ineligible
ext4: mark fs-verity enable fast-commit ineligible
ext4: mark move extents fast-commit ineligible
ext4: mark group add fast-commit ineligible
ext4: mark group extend fast-commit ineligible
fs/ext4/fast_commit.c | 3 +++
fs/ext4/fast_commit.h | 3 +++
fs/ext4/ioctl.c | 3 +++
fs/ext4/migrate.c | 12 ++++++++++++
fs/ext4/move_extent.c | 1 +
fs/ext4/verity.c | 2 ++
include/trace/events/ext4.h | 8 +++++++-
7 files changed, 31 insertions(+), 1 deletion(-)
--
2.51.0
^ permalink raw reply
* [PATCH] tracing: Fix error handling in event_hist_trigger_parse
From: Miaoqian Lin @ 2025-12-11 10:00 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Tom Zanussi,
linux-kernel, linux-trace-kernel
Cc: linmq006, stable
Memory allocated with trigger_data_alloc() require trigger_data_free()
for proper cleanup.
Replace kfree() with trigger_data_free() to fix this.
Found via static analysis and code review.
Fixes: e1f187d09e11 ("tracing: Have existing event_command.parse() implementations use helpers")
Cc: stable@vger.kernel.org
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
kernel/trace/trace_events_hist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 5e6e70540eef..f9886fff7123 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6902,7 +6902,7 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
remove_hist_vars(hist_data);
- kfree(trigger_data);
+ trigger_data_free(trigger_data);
destroy_hist_data(hist_data);
goto out;
--
2.25.1
^ permalink raw reply related
* Re: [RFC PATCH v3 05/17] s390: asm/dwarf.h should only be included in assembly files
From: Jens Remus @ 2025-12-11 9:43 UTC (permalink / raw)
To: Heiko Carstens
Cc: linux-kernel, linux-trace-kernel, linux-s390, bpf, x86,
Steven Rostedt, Vasily Gorbik, Ilya Leoshkevich, Josh Poimboeuf,
Masami Hiramatsu, Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar,
Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim,
Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
Kees Cook, Carlos O'Donell, Sam James, Dylan Hatch
In-Reply-To: <20251210151636.40732C96-hca@linux.ibm.com>
On 12/10/2025 4:16 PM, Heiko Carstens wrote:
> On Mon, Dec 08, 2025 at 06:15:47PM +0100, Jens Remus wrote:
>> Align to x86 and add a compile-time check that asm/dwarf.h is only
>> included in pure assembly files.
> Is there a reason why this and the next two patches couldn't go upstream
> already now? It looks like they improve things in any case.
> No dependency to the sframe work.
They probably could go upstream now. At least this one. For the two
other patches the question is whether we want to wait for whether the
respective x86 changes go upstream, as their descriptions claim to align
to x86.
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com
IBM
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
^ permalink raw reply
* Re: [RFC LPC2025 PATCH 0/4] Deprecate zone_reclaim_mode
From: mawupeng @ 2025-12-11 6:37 UTC (permalink / raw)
To: joshua.hahnjy
Cc: mawupeng1, willy, david, linux-doc, linux-kernel, linux-mm,
linux-trace-kernel, linuxppc-dev, kernel-team
In-Reply-To: <20251210123003.424248-1-joshua.hahnjy@gmail.com>
On 2025/12/10 20:30, Joshua Hahn wrote:
> On Tue, 9 Dec 2025 20:43:01 +0800 mawupeng <mawupeng1@huawei.com> wrote:
>>
>> On 2025/12/6 7:32, Joshua Hahn wrote:
>>> Hello folks,
>>> This is a code RFC for my upcoming discussion at LPC 2025 in Tokyo [1].
>>>
>>> zone_reclaim_mode was introduced in 2005 to prevent the kernel from facing
>>> the high remote access latency associated with NUMA systems. With it enabled,
>>> when the kernel sees that the local node is full, it will stall allocations and
>>> trigger direct reclaim locally, instead of making a remote allocation, even
>>> when there may still be free memory. Thsi is the preferred way to consume memory
>>> if remote memory access is more expensive than performing direct reclaim.
>>> The choice is made on a system-wide basis, but can be toggled at runtime.
>>>
>>> This series deprecates the zone_reclaim_mode sysctl in favor of other NUMA
>>> aware mechanisms, such as NUMA balancing, memory.reclaim, membind, and
>>> tiering / promotion / demotion. Let's break down what differences there are
>>> in these mechanisms, based on workload characteristics.
>
> [...snip...]
>
> Hello mawupeng, thank you for your feedback on this RFC.
>
> I was wondering if you were planning to attend LPC this year. If so, I'll be
> discussing this idea at the MM microconference tomorrow (December 11th) and
> would love to discuss this after the presentation with you in the hallway.
> I want to make sure that I'm not missing any important nuances or use cases
> for zone_reclaim_mode. After all, my only motivation for deprecating this is
> to simplify the code allocation path and reduce maintenence burden, both of
> which definitely does not outweigh valid usecases. On the other hand if we can
> find out that we can deprecate zone_reclaim_mode, and also find some
> alternatives that lead to better performance on your end, that sounds
> like the ultimate win-win scenario for me : -)
>
>> In real-world scenarios, we have observed on a dual-socket (2P) server with multiple
>> NUMA nodes—each having relatively limited local memory capacity—that page cache
>> negatively impacts overall performance. The zone_reclaim_node feature is used to
>> alleviate performance issues.
>>
>> The main reason is that page cache consumes free memory on the local node, causing
>> processes without mbind restrictions to fall back to other nodes that still have free
>> memory. Accessing remote memory comes with a significant latency penalty. In extreme
>> testing, if a system is fully populated with page cache beforehand, Spark application
>> performance can drop by 80%. However, with zone_reclaim enabled, the performance
>> degradation is limited to only about 30%.
>
> This sounds right to me. In fact, I have observed similar results in some
> experiments that I ran myself, where on a 2-NUMA system with 125GB memory each,
> I fill up one node with 100G of garbage filecache and try to run a 60G anon
> workload in it. Here are the average access latency results:
>
> - zone_reclaim_mode enabled: 56.34 ns/access
> - zone_reclaim_mode disabled: 67.86 ns/access
>
> However, I was able to achieve better results by disabling zone_reclaim_mode
> and using membind instead:
>
> - zone_reclaim_mode disabled + membind: 52.98 ns/access
>
> Of course, these are on my specific system with my specific workload so the
> numbers (and results) may be different on your end. You specifically mentioned
> "processes without mbind restrictions". Is there a reason why these workloads
> cannot be membound to a node?
My apologies for the delayed response — I’ve been discussing the actual workload
scenarios with HPC experts.
In HPC workloads, certain specialized tasks are responsible for initialization,
checkpointing by specific processes, post‑processing, I/O operations, and so on.
Since the memory access patterns are not completely uniform, binding them to a
specific NUMA node via membind is often a more suitable approach.
However, two main concerns arise here:
If a workload spans more than a single NUMA node, binding it to just one node may
not be appropriate. Binding to multiple NUMA nodes might be feasible, but the
impact on page cache behavior requires further evaluation.
HPC applications vary widely, and not all can be addressed by membind. At the same
time, zone_reclaim_mode may not be a complete solution either—this also needs
further investigation.
We should explore whether there are better ways to balance memory locality with
memory availability.
>
> On that note, I had another follow-up question. If remote latency really is a
> big concern, I am wondering if you have seen remote allocations despite
> enabling zone_reclaim_mode. From my understanding of the code, zone_reclaim_mode
> is not a strict guarantee of memory locality. If direct reclaim fails and
> we fail to reclaim enough, the allocation is serviced from a remote node anyways.
>
> Maybe I did not make this clear in my RFC, but I definitely believe that there
> are workloads out there that benefit from zone_reclaim_mode. However, I
> also believe that membind is just a better alternative for all the scenarios
> that I can think of, so it would really be helpful for my education to learn
> about workloads that benefit from zone_reclaim_mode but cannot use membind.
>
>> Furthermore, for typical HPC applications, memory pressure tends to be balanced
>> across NUMA nodes. Yet page cache is often generated by background tasks—such as
>> logging modules—which breaks memory locality and adversely affects overall performance.
>
> I see. From my very limited understanding of HPC applications, they tend to be
> perfectly sized for the nodes they run on, so having logging agents generate
> additional page cache really does sound like a problem to me.
>
>> At the same time, there are a large number of __GFP_THISNODE memory allocation requests in
>> the system. Anonymous pages that fall back from other nodes cannot be migrated or easily
>> reclaimed (especially when swap is disabled), leading to uneven distribution of available
>> memory within a single node. By enabling zone_reclaim_mode, the kernel preferentially reclaims
>> file pages within the local NUMA node to satisfy local anonymous-page allocations, which
>> effectively avoids warn_alloc problems caused by uneven distribution of anonymous pages.
>>
>> In such scenarios, relying solely on mbind may offer limited flexibility.
>
> I see. So if I understand your scenario correctly, what you want is something
> between mbind which is strict in guaranteeing that memory comes locally, and
> the default memory allocation preference, which prefers allocating from
> remote nodes when the local node runs out of memory.
>
> I have some follow-up questions here.
> It seems like the fact that anonymous memory from remote processes leaking
> their memory into the current node is actually caused by two characteristics
> of zone_reclaim_mode. Namely, that it does not guarantee memory locality,
> and that it is a system-wide setting. Under your scenario, we cannot have
> a mixture of HPC workloads that cannot handle remote memory access latency,
> as well as non-HPC workloads that would actually benefit from being able to
> consume free memory from remote nodes before triggering reclaim.
>
> So in a scenario where we have multiple HPC workloads running on a multi-NUMA
> system, we can just size each workload to fit the nodes, and membind them so
> that we don't have to worry about migrating or reclaiming remote processes'
> anonymous memory.
>
> In a scenario where we have an HPC workload + non-HPC workloads, we can membind
> the HPC workload to a single node, and exclude that node from the other
> workloads' nodemasks to prevent anonymous memory from leaking into it.
Maybe we can try to mbind to multiple node for this scenario as above.
>
>> We have also experimented with proactively waking kswapd to improve synchronous reclaim
>> efficiency. Our actual tests show that this can roughly double the memory allocation rate[1].
>
> Personally I believe that this could be the way forward. However, there are
> still some problems that we have to address, the biggest one being: pagecache
> can be considered "garbage" in both your HPC workloads and my microbenchmark.
> However, the pagecache can be very valuable in certain scenarios. What if
> the workload will access the pagecache in the future? I'm not really sure if
> it makes sense to clean up that pagecache and allocate locally, when the
> worst-case scenario is that we have to incur much more latency reading from
> disk and bringing in those pages again, when there is free memory still
> available in the system.
If a larger file cache is required, disabling zone_reclaim_mode may help alleviate the
issue, but it indeed lacks some flexibility.
In my personal understanding, the main problem is that page cache tends to occupy all
available free memory, leading to an unreasonable increase in fallback.
>
> Perhaps the real solution is to deprecate zone_reclaim_mode and offer more
> granular (per-workload basis), and sane (guarantee memory locality and also
> perform kswapd when the ndoe is full) options for the user.
I think it’s worth looking into whether there are better approaches to address the
current issue.
>
>> We could also discuss whether there are better solutions for such HPC scenarios.
>
> Yes, I really hope that we can reach the win-win scenario that I mentioned at
> the beginning of the reply. I really want to help users achieve the best
> performance they can, and also help keep the kernel easy to maintain in the
> long-run. Thank you for sharing your perspective, I really learned a lot.
> Looking forward to your response, or if you are coming to LPC, would love to
> grab a coffee. Have a grat day!
> Joshua
>
>> [1]: https://lore.kernel.org/all/20251011062043.772549-1-mawupeng1@huawei.com/
^ permalink raw reply
* Re: [PATCH v14 0/3] PCI: trace: Add a RAS tracepoint to monitor link speed changes
From: Bjorn Helgaas @ 2025-12-11 0:30 UTC (permalink / raw)
To: Shuai Xue
Cc: rostedt, lukas, linux-pci, linux-kernel, linux-edac,
linux-trace-kernel, ilpo.jarvinen, mattc, Jonathan.Cameron,
alok.a.tiwari, bhelgaas, tony.luck, bp, mhiramat,
mathieu.desnoyers, oleg, naveen, davem, anil.s.keshavamurthy,
mark.rutland, peterz, tianruidong
In-Reply-To: <20251210132907.58799-1-xueshuai@linux.alibaba.com>
On Wed, Dec 10, 2025 at 09:29:04PM +0800, Shuai Xue wrote:
> changes since v13:
> - fix doc typos per ALOK TIWARI
>
> changes since v12:
> - add Reviewed-by tag for PATCH 1 from Steve
> - add Reviewed-by tag for PATCH 1-3 from Ilpo
> - add comments for why use string to define tracepoint per Steve
> - minor doc improvements from Ilpo
> - remove use pci_speed_string to fix PCI dependends which cause build error on sparc64
>
> changes since v11:
> - rebase to Linux 6.18-rc1 (no functional changes)
>
> changes since v10:
> - explicitly include header file per Ilpo
> - add comma on any non-terminator entry per Ilpo
> - compile trace.o under CONFIG_TRACING per Ilpo
>
> changes since v9:
> - add a documentation about PCI tracepoints per Bjorn
> - create a dedicated drivers/pci/trace.c that always defines the PCI tracepoints per Steve
> - move tracepoint callite into __pcie_update_link_speed() per Lukas and Bjorn
>
> changes since v8:
> - rewrite commit log from Bjorn
> - move pci_hp_event to a common place (include/trace/events/pci.h) per Ilpo
> - rename hotplug event strings per Bjorn and Lukas
> - add PCIe link tracepoint per Bjorn, Lukas, and Ilpo
>
> changes since v7:
> - replace the TRACE_INCLUDE_PATH to avoid macro conflict per Steven
> - pick up Reviewed-by from Lukas Wunner
>
> Hotplug events are critical indicators for analyzing hardware health, and
> surprise link downs can significantly impact system performance and reliability.
> In addition, PCIe link speed degradation directly impacts system performance and
> often indicates hardware issues such as faulty devices, physical layer problems,
> or configuration errors.
>
> This patch set add PCI hotplug and PCIe link tracepoint to help analyze PCI
> hotplug events and PCIe link speed degradation.
>
> Shuai Xue (3):
> PCI: trace: Add a generic RAS tracepoint for hotplug event
> PCI: trace: Add a RAS tracepoint to monitor link speed changes
> Documentation: tracing: Add documentation about PCI tracepoints
>
> Documentation/trace/events-pci.rst | 74 +++++++++++++++++
> drivers/pci/Makefile | 3 +
> drivers/pci/hotplug/pciehp_ctrl.c | 31 +++++--
> drivers/pci/hotplug/pciehp_hpc.c | 3 +-
> drivers/pci/pci.c | 2 +-
> drivers/pci/pci.h | 21 ++++-
> drivers/pci/pcie/bwctrl.c | 4 +-
> drivers/pci/probe.c | 9 +-
> drivers/pci/trace.c | 11 +++
> include/trace/events/pci.h | 129 +++++++++++++++++++++++++++++
> include/uapi/linux/pci.h | 7 ++
> 11 files changed, 279 insertions(+), 15 deletions(-)
> create mode 100644 Documentation/trace/events-pci.rst
> create mode 100644 drivers/pci/trace.c
> create mode 100644 include/trace/events/pci.h
Applied to pci/trace for v6.20, thanks! This will be rebased after
v6.19-rc1.
^ permalink raw reply
* [PATCH v3] kallsyms: Always initialize modbuildid
From: Maurice Hieronymus @ 2025-12-10 17:03 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers
Cc: georges.aureau, Maurice Hieronymus, bpf, linux-kernel,
linux-trace-kernel
modbuildid is never set when kallsyms_lookup_buildid is returning via
successful bpf_address_lookup or ftrace_mod_address_lookup.
This leads to an uninitialized pointer dereference on x86 when
CONFIG_STACKTRACE_BUILD_ID=y inside __sprint_symbol.
Prevent this by always initializing modbuildid.
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220717
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
Changes to v2:
- Check if CONFIG_STACKTRACE_BUILD_ID is enabled to prevent build fail
Changes to v1:
- Set modbuildid in ftrace_func_address_lookup
include/linux/filter.h | 6 ++++--
include/linux/ftrace.h | 4 ++--
kernel/kallsyms.c | 4 ++--
kernel/trace/ftrace.c | 8 +++++++-
4 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index fd54fed8f95f..eb1d1c876503 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1384,12 +1384,14 @@ struct bpf_prog *bpf_prog_ksym_find(unsigned long addr);
static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym)
{
int ret = __bpf_address_lookup(addr, size, off, sym);
if (ret && modname)
*modname = NULL;
+ if (ret && modbuildid)
+ *modbuildid = NULL;
return ret;
}
@@ -1455,7 +1457,7 @@ static inline struct bpf_prog *bpf_prog_ksym_find(unsigned long addr)
static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym)
{
return 0;
}
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 770f0dc993cc..ed673fa2536b 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -87,11 +87,11 @@ struct ftrace_hash;
defined(CONFIG_DYNAMIC_FTRACE)
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym);
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym);
#else
static inline int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym)
{
return 0;
}
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 049e296f586c..b1516d3fa9c5 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -378,11 +378,11 @@ static int kallsyms_lookup_buildid(unsigned long addr,
modname, modbuildid, namebuf);
if (!ret)
ret = bpf_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ offset, modname, modbuildid, namebuf);
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ offset, modname, modbuildid, namebuf);
return ret;
}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3ec2033c0774..4e4aef987747 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7749,7 +7749,7 @@ ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym)
{
struct ftrace_mod_map *mod_map;
int ret = 0;
@@ -7761,6 +7761,12 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
if (ret) {
if (modname)
*modname = mod_map->mod->name;
+ if (modbuildid)
+#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
+ *modbuildid = mod_map->mod->build_id;
+#else
+ *modbuildid = NULL;
+#endif
break;
}
}
base-commit: 0048fbb4011ec55c32d3148b2cda56433f273375
--
2.50.1
^ permalink raw reply related
* [PATCH v2] kallsyms: Always initialize modbuildid
From: Maurice Hieronymus @ 2025-12-10 16:28 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers
Cc: Maurice Hieronymus, bpf, linux-kernel, linux-trace-kernel
modbuildid is never set when kallsyms_lookup_buildid is returning via
successful bpf_address_lookup or ftrace_mod_address_lookup.
This leads to an uninitialized pointer dereference on x86 when
CONFIG_STACKTRACE_BUILD_ID=y inside __sprint_symbol.
Prevent this by always initializing modbuildid.
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220717
Signed-off-by: Maurice Hieronymus <mhi@mailbox.org>
---
include/linux/filter.h | 6 ++++--
include/linux/ftrace.h | 4 ++--
kernel/kallsyms.c | 4 ++--
kernel/trace/ftrace.c | 4 +++-
4 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index fd54fed8f95f..eb1d1c876503 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1384,12 +1384,14 @@ struct bpf_prog *bpf_prog_ksym_find(unsigned long addr);
static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym)
{
int ret = __bpf_address_lookup(addr, size, off, sym);
if (ret && modname)
*modname = NULL;
+ if (ret && modbuildid)
+ *modbuildid = NULL;
return ret;
}
@@ -1455,7 +1457,7 @@ static inline struct bpf_prog *bpf_prog_ksym_find(unsigned long addr)
static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym)
{
return 0;
}
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 770f0dc993cc..ed673fa2536b 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -87,11 +87,11 @@ struct ftrace_hash;
defined(CONFIG_DYNAMIC_FTRACE)
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym);
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym);
#else
static inline int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym)
{
return 0;
}
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 049e296f586c..b1516d3fa9c5 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -378,11 +378,11 @@ static int kallsyms_lookup_buildid(unsigned long addr,
modname, modbuildid, namebuf);
if (!ret)
ret = bpf_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ offset, modname, modbuildid, namebuf);
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ offset, modname, modbuildid, namebuf);
return ret;
}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 3ec2033c0774..63a926926709 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7749,7 +7749,7 @@ ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname, const unsigned char **modbuildid, char *sym)
{
struct ftrace_mod_map *mod_map;
int ret = 0;
@@ -7761,6 +7761,8 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
if (ret) {
if (modname)
*modname = mod_map->mod->name;
+ if (modbuildid)
+ *modbuildid = mod_map->mod->build_id;
break;
}
}
base-commit: 0048fbb4011ec55c32d3148b2cda56433f273375
--
2.50.1
^ permalink raw reply related
* Re: [RFC PATCH v3 13/17] s390/ptrace: Provide frame_pointer()
From: Heiko Carstens @ 2025-12-10 15:19 UTC (permalink / raw)
To: Jens Remus
Cc: linux-kernel, linux-trace-kernel, linux-s390, bpf, x86,
Steven Rostedt, Vasily Gorbik, Ilya Leoshkevich, Josh Poimboeuf,
Masami Hiramatsu, Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar,
Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim,
Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
Kees Cook, Carlos O'Donell, Sam James, Dylan Hatch
In-Reply-To: <20251208171559.2029709-14-jremus@linux.ibm.com>
On Mon, Dec 08, 2025 at 06:15:55PM +0100, Jens Remus wrote:
> On s390 64-bit the s390x ELF ABI [1] designates register 11 as the
> "preferred" frame pointer (FP) register in user space.
>
> While at it convert instruction_pointer() and user_stack_pointer()
> from macros to inline functions, to align their definition with
> x86 and arm64.
>
> Use const qualifier on struct pt_regs pointers to prevent compiler
> warnings:
>
> arch/s390/kernel/stacktrace.c: In function ‘arch_stack_walk_user_common’:
> arch/s390/kernel/stacktrace.c:114:34: warning: passing argument 1 of
> ‘instruction_pointer’ discards ‘const’ qualifier from pointer target
> type [-Wdiscarded-qualifiers]
> ...
> arch/s390/kernel/stacktrace.c:117:48: warning: passing argument 1 of
> ‘user_stack_pointer’ discards ‘const’ qualifier from pointer target
> type [-Wdiscarded-qualifiers]
> ...
>
> [1]: s390x ELF ABI, https://github.com/IBM/s390x-abi/releases
>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>
> Notes (jremus):
> Changes in RFC v2:
> - Separate provide frame_pointer() into this new commit.
>
> arch/s390/include/asm/ptrace.h | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
> index dfa770b15fad..455c119167fc 100644
> --- a/arch/s390/include/asm/ptrace.h
> +++ b/arch/s390/include/asm/ptrace.h
> @@ -212,8 +212,6 @@ void update_cr_regs(struct task_struct *task);
> #define arch_has_block_step() (1)
>
> #define user_mode(regs) (((regs)->psw.mask & PSW_MASK_PSTATE) != 0)
> -#define instruction_pointer(regs) ((regs)->psw.addr)
> -#define user_stack_pointer(regs)((regs)->gprs[15])
> #define profile_pc(regs) instruction_pointer(regs)
"while at it", and then you don't convert user_mode() to a function? :)
Please provide a stand-alone patch for the "while at it" stuff, so it
can go independently.
^ permalink raw reply
* Re: [RFC PATCH v3 05/17] s390: asm/dwarf.h should only be included in assembly files
From: Heiko Carstens @ 2025-12-10 15:16 UTC (permalink / raw)
To: Jens Remus
Cc: linux-kernel, linux-trace-kernel, linux-s390, bpf, x86,
Steven Rostedt, Vasily Gorbik, Ilya Leoshkevich, Josh Poimboeuf,
Masami Hiramatsu, Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar,
Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim,
Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
Kees Cook, Carlos O'Donell, Sam James, Dylan Hatch
In-Reply-To: <20251208171559.2029709-6-jremus@linux.ibm.com>
On Mon, Dec 08, 2025 at 06:15:47PM +0100, Jens Remus wrote:
> Align to x86 and add a compile-time check that asm/dwarf.h is only
> included in pure assembly files.
>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>
> Notes (jremus):
> Changes in RFC v2:
> - Adjust to upstream change of __ASSEMBLY__ to __ASSEMBLER__.
>
> arch/s390/include/asm/dwarf.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Is there a reason why this and the next two patches couldn't go upstream
already now? It looks like they improve things in any case.
No dependency to the sframe work.
^ permalink raw reply
* Re: [RFC PATCH v3 14/17] s390/unwind_user/sframe: Enable HAVE_UNWIND_USER_SFRAME
From: Heiko Carstens @ 2025-12-10 15:10 UTC (permalink / raw)
To: Jens Remus
Cc: linux-kernel, linux-trace-kernel, linux-s390, bpf, x86,
Steven Rostedt, Vasily Gorbik, Ilya Leoshkevich, Josh Poimboeuf,
Masami Hiramatsu, Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar,
Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim,
Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
Kees Cook, Carlos O'Donell, Sam James, Dylan Hatch
In-Reply-To: <20251208171559.2029709-15-jremus@linux.ibm.com>
Hi Jens,
On Mon, Dec 08, 2025 at 06:15:56PM +0100, Jens Remus wrote:
> +static inline int __s390_get_dwarf_fpr(unsigned long *val, int regnum)
> +{
> + switch (regnum) {
> + case 16:
> + fpu_std(0, (freg_t *)val);
> + break;
> + case 17:
> + fpu_std(2, (freg_t *)val);
> + break;
> + case 18:
> + fpu_std(4, (freg_t *)val);
> + break;
> + case 19:
> + fpu_std(6, (freg_t *)val);
> + break;
> + case 20:
> + fpu_std(1, (freg_t *)val);
> + break;
IIRC, I mentioned this already last time. But it is not correct to access user
space floating point register contents like this. Due to in-kernel fpu/vector
register usage the user space register contents may have been saved away to
the per-thread vxrs save area, and registers may have been used for in-kernel
usage instead.
Read: the above code could access lazy register contents of in-kernel usage.
Change the above to something like:
struct fpu *fpu = ¤t->thread.ufpu;
save_user_fpu_regs();
switch (regnum) {
case 16: return fpu->vxrs[0].high;
case 17: return fpu->vxrs[2].high;
case 18: return fpu->vxrs[4].high;
case 19: return fpu->vxrs[6].high;
case 20: return fpu->vxrs[1].high;
...
save_user_fpu_regs() will write all user space fpu/vector register contents to
the per-thread save area (if not already saved), and then it is possible to
read contents from there.
I'll see if I can provide something better for this use case, since this code
needs to access only the first 16 registers; so no need to write contents of
all registers to the save area.
^ permalink raw reply
* [PATCH v14 3/3] Documentation: tracing: Add documentation about PCI tracepoints
From: Shuai Xue @ 2025-12-10 13:29 UTC (permalink / raw)
To: rostedt, lukas, linux-pci, linux-kernel, linux-edac,
linux-trace-kernel, helgaas, ilpo.jarvinen, mattc,
Jonathan.Cameron, alok.a.tiwari
Cc: bhelgaas, tony.luck, bp, xueshuai, mhiramat, mathieu.desnoyers,
oleg, naveen, davem, anil.s.keshavamurthy, mark.rutland, peterz,
tianruidong
In-Reply-To: <20251210132907.58799-1-xueshuai@linux.alibaba.com>
The PCI tracing system provides tracepoints to monitor critical hardware
events that can impact system performance and reliability. Add
documentation about it.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
Documentation/trace/events-pci.rst | 74 ++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
create mode 100644 Documentation/trace/events-pci.rst
diff --git a/Documentation/trace/events-pci.rst b/Documentation/trace/events-pci.rst
new file mode 100644
index 000000000000..b9f7a9edee35
--- /dev/null
+++ b/Documentation/trace/events-pci.rst
@@ -0,0 +1,74 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===========================
+Subsystem Trace Points: PCI
+===========================
+
+Overview
+========
+The PCI tracing system provides tracepoints to monitor critical hardware events
+that can impact system performance and reliability. These events normally show
+up here:
+
+ /sys/kernel/tracing/events/pci
+
+Cf. include/trace/events/pci.h for the events definitions.
+
+Available Tracepoints
+=====================
+
+pci_hp_event
+------------
+
+Monitors PCI hotplug events including card insertion/removal and link
+state changes.
+::
+
+ pci_hp_event "%s slot:%s, event:%s\n"
+
+**Event Types**:
+
+* ``LINK_UP`` - PCIe link established
+* ``LINK_DOWN`` - PCIe link lost
+* ``CARD_PRESENT`` - Card detected in slot
+* ``CARD_NOT_PRESENT`` - Card removed from slot
+
+**Example Usage**:
+
+ # Enable the tracepoint
+ echo 1 > /sys/kernel/debug/tracing/events/pci/pci_hp_event/enable
+
+ # Monitor events (the following output is generated when a device is hotplugged)
+ cat /sys/kernel/debug/tracing/trace_pipe
+ irq/51-pciehp-88 [001] ..... 1311.177459: pci_hp_event: 0000:00:02.0 slot:10, event:CARD_PRESENT
+
+ irq/51-pciehp-88 [001] ..... 1311.177566: pci_hp_event: 0000:00:02.0 slot:10, event:LINK_UP
+
+pcie_link_event
+---------------
+
+Monitors PCIe link speed changes and provides detailed link status information.
+::
+
+ pcie_link_event "%s type:%d, reason:%d, cur_bus_speed:%d, max_bus_speed:%d, width:%u, flit_mode:%u, status:%s\n"
+
+**Parameters**:
+
+* ``type`` - PCIe device type (4=Root Port, etc.)
+* ``reason`` - Reason for link change:
+
+ - ``0`` - Link retrain
+ - ``1`` - Bus enumeration
+ - ``2`` - Bandwidth notification enable
+ - ``3`` - Bandwidth notification IRQ
+ - ``4`` - Hotplug event
+
+
+**Example Usage**:
+
+ # Enable the tracepoint
+ echo1 > /sys/kernel/debug/tracing/events/pci/pcie_link_event/enable
+
+ # Monitor events (the following output is generated when a device is hotplugged)
+ cat /sys/kernel/debug/tracing/trace_pipe
+ irq/51-pciehp-88 [001] ..... 381.545386: pcie_link_event: 0000:00:02.0 type:4, reason:4, cur_bus_speed:20, max_bus_speed:23, width:1, flit_mode:0, status:DLLLA
--
2.39.3
^ permalink raw reply related
* [PATCH v14 1/3] PCI: trace: Add a generic RAS tracepoint for hotplug event
From: Shuai Xue @ 2025-12-10 13:29 UTC (permalink / raw)
To: rostedt, lukas, linux-pci, linux-kernel, linux-edac,
linux-trace-kernel, helgaas, ilpo.jarvinen, mattc,
Jonathan.Cameron, alok.a.tiwari
Cc: bhelgaas, tony.luck, bp, xueshuai, mhiramat, mathieu.desnoyers,
oleg, naveen, davem, anil.s.keshavamurthy, mark.rutland, peterz,
tianruidong
In-Reply-To: <20251210132907.58799-1-xueshuai@linux.alibaba.com>
Hotplug events are critical indicators for analyzing hardware health,
and surprise link downs can significantly impact system performance and
reliability.
Define a new TRACING_SYSTEM named "pci", add a generic RAS tracepoint
for hotplug event to help health checks. Add enum pci_hotplug_event in
include/uapi/linux/pci.h so applications like rasdaemon can register
tracepoint event handlers for it.
The following output is generated when a device is hotplugged:
$ echo 1 > /sys/kernel/debug/tracing/events/pci/pci_hp_event/enable
$ cat /sys/kernel/debug/tracing/trace_pipe
irq/51-pciehp-88 [001] ..... 1311.177459: pci_hp_event: 0000:00:02.0 slot:10, event:CARD_PRESENT
irq/51-pciehp-88 [001] ..... 1311.177566: pci_hp_event: 0000:00:02.0 slot:10, event:LINK_UP
Suggested-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: "Steven Rostedt (Google)" <rostedt@goodmis.org> # for trace event
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
drivers/pci/Makefile | 3 ++
drivers/pci/hotplug/pciehp_ctrl.c | 31 ++++++++++---
drivers/pci/trace.c | 11 +++++
include/trace/events/pci.h | 72 +++++++++++++++++++++++++++++++
include/uapi/linux/pci.h | 7 +++
5 files changed, 118 insertions(+), 6 deletions(-)
create mode 100644 drivers/pci/trace.c
create mode 100644 include/trace/events/pci.h
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index e10cfe5a280b..8c259a9a8796 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -47,3 +47,6 @@ obj-y += controller/
obj-y += switch/
subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
+
+CFLAGS_trace.o := -I$(src)
+obj-$(CONFIG_TRACING) += trace.o
diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
index bcc938d4420f..7805f697a02c 100644
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -19,6 +19,7 @@
#include <linux/types.h>
#include <linux/pm_runtime.h>
#include <linux/pci.h>
+#include <trace/events/pci.h>
#include "../pci.h"
#include "pciehp.h"
@@ -244,12 +245,20 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
case ON_STATE:
ctrl->state = POWEROFF_STATE;
mutex_unlock(&ctrl->state_lock);
- if (events & PCI_EXP_SLTSTA_DLLSC)
+ if (events & PCI_EXP_SLTSTA_DLLSC) {
ctrl_info(ctrl, "Slot(%s): Link Down\n",
slot_name(ctrl));
- if (events & PCI_EXP_SLTSTA_PDC)
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_LINK_DOWN);
+ }
+ if (events & PCI_EXP_SLTSTA_PDC) {
ctrl_info(ctrl, "Slot(%s): Card not present\n",
slot_name(ctrl));
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_CARD_NOT_PRESENT);
+ }
pciehp_disable_slot(ctrl, SURPRISE_REMOVAL);
break;
default:
@@ -269,6 +278,9 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
INDICATOR_NOOP);
ctrl_info(ctrl, "Slot(%s): Card not present\n",
slot_name(ctrl));
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_CARD_NOT_PRESENT);
}
mutex_unlock(&ctrl->state_lock);
return;
@@ -281,12 +293,19 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
case OFF_STATE:
ctrl->state = POWERON_STATE;
mutex_unlock(&ctrl->state_lock);
- if (present)
+ if (present) {
ctrl_info(ctrl, "Slot(%s): Card present\n",
slot_name(ctrl));
- if (link_active)
- ctrl_info(ctrl, "Slot(%s): Link Up\n",
- slot_name(ctrl));
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_CARD_PRESENT);
+ }
+ if (link_active) {
+ ctrl_info(ctrl, "Slot(%s): Link Up\n", slot_name(ctrl));
+ trace_pci_hp_event(pci_name(ctrl->pcie->port),
+ slot_name(ctrl),
+ PCI_HOTPLUG_LINK_UP);
+ }
ctrl->request_result = pciehp_enable_slot(ctrl);
break;
default:
diff --git a/drivers/pci/trace.c b/drivers/pci/trace.c
new file mode 100644
index 000000000000..cf11abca8602
--- /dev/null
+++ b/drivers/pci/trace.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Tracepoints for PCI system
+ *
+ * Copyright (C) 2025 Alibaba Corporation
+ */
+
+#include <linux/pci.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/pci.h>
diff --git a/include/trace/events/pci.h b/include/trace/events/pci.h
new file mode 100644
index 000000000000..39e512a167ee
--- /dev/null
+++ b/include/trace/events/pci.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM pci
+
+#if !defined(_TRACE_HW_EVENT_PCI_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_HW_EVENT_PCI_H
+
+#include <linux/tracepoint.h>
+
+#define PCI_HOTPLUG_EVENT \
+ EM(PCI_HOTPLUG_LINK_UP, "LINK_UP") \
+ EM(PCI_HOTPLUG_LINK_DOWN, "LINK_DOWN") \
+ EM(PCI_HOTPLUG_CARD_PRESENT, "CARD_PRESENT") \
+ EMe(PCI_HOTPLUG_CARD_NOT_PRESENT, "CARD_NOT_PRESENT")
+
+/* Enums require being exported to userspace, for user tool parsing */
+#undef EM
+#undef EMe
+#define EM(a, b) TRACE_DEFINE_ENUM(a);
+#define EMe(a, b) TRACE_DEFINE_ENUM(a);
+
+PCI_HOTPLUG_EVENT
+
+/*
+ * Now redefine the EM() and EMe() macros to map the enums to the strings
+ * that will be printed in the output.
+ */
+#undef EM
+#undef EMe
+#define EM(a, b) {a, b},
+#define EMe(a, b) {a, b}
+
+/*
+ * Note: For generic PCI hotplug events, we pass already-resolved strings
+ * (port_name, slot) instead of driver-specific structures like 'struct
+ * controller'. This is because different PCI hotplug drivers (pciehp, cpqphp,
+ * ibmphp, shpchp) define their own versions of 'struct controller' with
+ * different fields and helper functions. Using driver-specific structures would
+ * make the tracepoint interface non-generic and cause compatibility issues
+ * across different drivers.
+ */
+TRACE_EVENT(pci_hp_event,
+
+ TP_PROTO(const char *port_name,
+ const char *slot,
+ const int event),
+
+ TP_ARGS(port_name, slot, event),
+
+ TP_STRUCT__entry(
+ __string( port_name, port_name )
+ __string( slot, slot )
+ __field( int, event )
+ ),
+
+ TP_fast_assign(
+ __assign_str(port_name);
+ __assign_str(slot);
+ __entry->event = event;
+ ),
+
+ TP_printk("%s slot:%s, event:%s\n",
+ __get_str(port_name),
+ __get_str(slot),
+ __print_symbolic(__entry->event, PCI_HOTPLUG_EVENT)
+ )
+);
+
+#endif /* _TRACE_HW_EVENT_PCI_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/include/uapi/linux/pci.h b/include/uapi/linux/pci.h
index a769eefc5139..4f150028965d 100644
--- a/include/uapi/linux/pci.h
+++ b/include/uapi/linux/pci.h
@@ -39,4 +39,11 @@
#define PCIIOC_MMAP_IS_MEM (PCIIOC_BASE | 0x02) /* Set mmap state to MEM space. */
#define PCIIOC_WRITE_COMBINE (PCIIOC_BASE | 0x03) /* Enable/disable write-combining. */
+enum pci_hotplug_event {
+ PCI_HOTPLUG_LINK_UP,
+ PCI_HOTPLUG_LINK_DOWN,
+ PCI_HOTPLUG_CARD_PRESENT,
+ PCI_HOTPLUG_CARD_NOT_PRESENT,
+};
+
#endif /* _UAPILINUX_PCI_H */
--
2.39.3
^ permalink raw reply related
* [PATCH v14 0/3] PCI: trace: Add a RAS tracepoint to monitor link speed changes
From: Shuai Xue @ 2025-12-10 13:29 UTC (permalink / raw)
To: rostedt, lukas, linux-pci, linux-kernel, linux-edac,
linux-trace-kernel, helgaas, ilpo.jarvinen, mattc,
Jonathan.Cameron, alok.a.tiwari
Cc: bhelgaas, tony.luck, bp, xueshuai, mhiramat, mathieu.desnoyers,
oleg, naveen, davem, anil.s.keshavamurthy, mark.rutland, peterz,
tianruidong
changes since v13:
- fix doc typos per ALOK TIWARI
changes since v12:
- add Reviewed-by tag for PATCH 1 from Steve
- add Reviewed-by tag for PATCH 1-3 from Ilpo
- add comments for why use string to define tracepoint per Steve
- minor doc improvements from Ilpo
- remove use pci_speed_string to fix PCI dependends which cause build error on sparc64
changes since v11:
- rebase to Linux 6.18-rc1 (no functional changes)
changes since v10:
- explicitly include header file per Ilpo
- add comma on any non-terminator entry per Ilpo
- compile trace.o under CONFIG_TRACING per Ilpo
changes since v9:
- add a documentation about PCI tracepoints per Bjorn
- create a dedicated drivers/pci/trace.c that always defines the PCI tracepoints per Steve
- move tracepoint callite into __pcie_update_link_speed() per Lukas and Bjorn
changes since v8:
- rewrite commit log from Bjorn
- move pci_hp_event to a common place (include/trace/events/pci.h) per Ilpo
- rename hotplug event strings per Bjorn and Lukas
- add PCIe link tracepoint per Bjorn, Lukas, and Ilpo
changes since v7:
- replace the TRACE_INCLUDE_PATH to avoid macro conflict per Steven
- pick up Reviewed-by from Lukas Wunner
Hotplug events are critical indicators for analyzing hardware health, and
surprise link downs can significantly impact system performance and reliability.
In addition, PCIe link speed degradation directly impacts system performance and
often indicates hardware issues such as faulty devices, physical layer problems,
or configuration errors.
This patch set add PCI hotplug and PCIe link tracepoint to help analyze PCI
hotplug events and PCIe link speed degradation.
Shuai Xue (3):
PCI: trace: Add a generic RAS tracepoint for hotplug event
PCI: trace: Add a RAS tracepoint to monitor link speed changes
Documentation: tracing: Add documentation about PCI tracepoints
Documentation/trace/events-pci.rst | 74 +++++++++++++++++
drivers/pci/Makefile | 3 +
drivers/pci/hotplug/pciehp_ctrl.c | 31 +++++--
drivers/pci/hotplug/pciehp_hpc.c | 3 +-
drivers/pci/pci.c | 2 +-
drivers/pci/pci.h | 21 ++++-
drivers/pci/pcie/bwctrl.c | 4 +-
drivers/pci/probe.c | 9 +-
drivers/pci/trace.c | 11 +++
include/trace/events/pci.h | 129 +++++++++++++++++++++++++++++
include/uapi/linux/pci.h | 7 ++
11 files changed, 279 insertions(+), 15 deletions(-)
create mode 100644 Documentation/trace/events-pci.rst
create mode 100644 drivers/pci/trace.c
create mode 100644 include/trace/events/pci.h
--
2.39.3
^ permalink raw reply
* [PATCH v14 2/3] PCI: trace: Add a RAS tracepoint to monitor link speed changes
From: Shuai Xue @ 2025-12-10 13:29 UTC (permalink / raw)
To: rostedt, lukas, linux-pci, linux-kernel, linux-edac,
linux-trace-kernel, helgaas, ilpo.jarvinen, mattc,
Jonathan.Cameron, alok.a.tiwari
Cc: bhelgaas, tony.luck, bp, xueshuai, mhiramat, mathieu.desnoyers,
oleg, naveen, davem, anil.s.keshavamurthy, mark.rutland, peterz,
tianruidong
In-Reply-To: <20251210132907.58799-1-xueshuai@linux.alibaba.com>
PCIe link speed degradation directly impacts system performance and
often indicates hardware issues such as faulty devices, physical layer
problems, or configuration errors.
To this end, add a RAS tracepoint to monitor link speed changes,
enabling proactive health checks and diagnostic analysis.
The following output is generated when a device is hotplugged:
$ echo 1 > /sys/kernel/debug/tracing/events/pci/pcie_link_event/enable
$ cat /sys/kernel/debug/tracing/trace_pipe
irq/51-pciehp-88 [001] ..... 381.545386: pcie_link_event: 0000:00:02.0 type:4, reason:4, cur_bus_speed:20, max_bus_speed:23, width:1, flit_mode:0, status:DLLLA
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Suggested-by: Matthew W Carlis <mattc@purestorage.com>
Suggested-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/pci/hotplug/pciehp_hpc.c | 3 +-
drivers/pci/pci.c | 2 +-
drivers/pci/pci.h | 21 ++++++++++--
drivers/pci/pcie/bwctrl.c | 4 +--
drivers/pci/probe.c | 9 +++--
include/trace/events/pci.h | 57 ++++++++++++++++++++++++++++++++
6 files changed, 87 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index bcc51b26d03d..ad5f28f6a8b1 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -320,7 +320,8 @@ int pciehp_check_link_status(struct controller *ctrl)
}
pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &linksta2);
- __pcie_update_link_speed(ctrl->pcie->port->subordinate, lnk_status, linksta2);
+ __pcie_update_link_speed(ctrl->pcie->port->subordinate, PCIE_HOTPLUG,
+ lnk_status, linksta2);
if (!found) {
ctrl_info(ctrl, "Slot(%s): No device found\n",
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 13dbb405dc31..f034e173819f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4550,7 +4550,7 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
* Link Speed.
*/
if (pdev->subordinate)
- pcie_update_link_speed(pdev->subordinate);
+ pcie_update_link_speed(pdev->subordinate, PCIE_LINK_RETRAIN);
return rc;
}
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0e67014aa001..c71cfbe78cc3 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -5,6 +5,7 @@
#include <linux/align.h>
#include <linux/bitfield.h>
#include <linux/pci.h>
+#include <trace/events/pci.h>
struct pcie_tlp_log;
@@ -555,12 +556,28 @@ const char *pci_speed_string(enum pci_bus_speed speed);
void __pcie_print_link_status(struct pci_dev *dev, bool verbose);
void pcie_report_downtraining(struct pci_dev *dev);
-static inline void __pcie_update_link_speed(struct pci_bus *bus, u16 linksta, u16 linksta2)
+enum pcie_link_change_reason {
+ PCIE_LINK_RETRAIN,
+ PCIE_ADD_BUS,
+ PCIE_BWCTRL_ENABLE,
+ PCIE_BWCTRL_IRQ,
+ PCIE_HOTPLUG,
+};
+
+static inline void __pcie_update_link_speed(struct pci_bus *bus,
+ enum pcie_link_change_reason reason,
+ u16 linksta, u16 linksta2)
{
bus->cur_bus_speed = pcie_link_speed[linksta & PCI_EXP_LNKSTA_CLS];
bus->flit_mode = (linksta2 & PCI_EXP_LNKSTA2_FLIT) ? 1 : 0;
+
+ trace_pcie_link_event(bus,
+ reason,
+ FIELD_GET(PCI_EXP_LNKSTA_NLW, linksta),
+ linksta & PCI_EXP_LNKSTA_LINK_STATUS_MASK);
}
-void pcie_update_link_speed(struct pci_bus *bus);
+
+void pcie_update_link_speed(struct pci_bus *bus, enum pcie_link_change_reason reason);
/* Single Root I/O Virtualization */
struct pci_sriov {
diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c
index 36f939f23d34..32f1b30ecb84 100644
--- a/drivers/pci/pcie/bwctrl.c
+++ b/drivers/pci/pcie/bwctrl.c
@@ -199,7 +199,7 @@ static void pcie_bwnotif_enable(struct pcie_device *srv)
* Update after enabling notifications & clearing status bits ensures
* link speed is up to date.
*/
- pcie_update_link_speed(port->subordinate);
+ pcie_update_link_speed(port->subordinate, PCIE_BWCTRL_ENABLE);
}
static void pcie_bwnotif_disable(struct pci_dev *port)
@@ -234,7 +234,7 @@ static irqreturn_t pcie_bwnotif_irq(int irq, void *context)
* speed (inside pcie_update_link_speed()) after LBMS has been
* cleared to avoid missing link speed changes.
*/
- pcie_update_link_speed(port->subordinate);
+ pcie_update_link_speed(port->subordinate, PCIE_BWCTRL_IRQ);
return IRQ_HANDLED;
}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 41183aed8f5d..392b7dc3d391 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -22,6 +22,7 @@
#include <linux/irqdomain.h>
#include <linux/pm_runtime.h>
#include <linux/bitfield.h>
+#include <trace/events/pci.h>
#include "pci.h"
#define CARDBUS_LATENCY_TIMER 176 /* secondary latency timer */
@@ -824,14 +825,16 @@ const char *pci_speed_string(enum pci_bus_speed speed)
}
EXPORT_SYMBOL_GPL(pci_speed_string);
-void pcie_update_link_speed(struct pci_bus *bus)
+void pcie_update_link_speed(struct pci_bus *bus,
+ enum pcie_link_change_reason reason)
{
struct pci_dev *bridge = bus->self;
u16 linksta, linksta2;
pcie_capability_read_word(bridge, PCI_EXP_LNKSTA, &linksta);
pcie_capability_read_word(bridge, PCI_EXP_LNKSTA2, &linksta2);
- __pcie_update_link_speed(bus, linksta, linksta2);
+
+ __pcie_update_link_speed(bus, reason, linksta, linksta2);
}
EXPORT_SYMBOL_GPL(pcie_update_link_speed);
@@ -918,7 +921,7 @@ static void pci_set_bus_speed(struct pci_bus *bus)
pcie_capability_read_dword(bridge, PCI_EXP_LNKCAP, &linkcap);
bus->max_bus_speed = pcie_link_speed[linkcap & PCI_EXP_LNKCAP_SLS];
- pcie_update_link_speed(bus);
+ pcie_update_link_speed(bus, PCIE_ADD_BUS);
}
}
diff --git a/include/trace/events/pci.h b/include/trace/events/pci.h
index 39e512a167ee..9a9122f62fd3 100644
--- a/include/trace/events/pci.h
+++ b/include/trace/events/pci.h
@@ -5,6 +5,7 @@
#if !defined(_TRACE_HW_EVENT_PCI_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_HW_EVENT_PCI_H
+#include <uapi/linux/pci_regs.h>
#include <linux/tracepoint.h>
#define PCI_HOTPLUG_EVENT \
@@ -66,6 +67,62 @@ TRACE_EVENT(pci_hp_event,
)
);
+#define PCI_EXP_LNKSTA_LINK_STATUS_MASK (PCI_EXP_LNKSTA_LBMS | \
+ PCI_EXP_LNKSTA_LABS | \
+ PCI_EXP_LNKSTA_LT | \
+ PCI_EXP_LNKSTA_DLLLA)
+
+#define LNKSTA_FLAGS \
+ { PCI_EXP_LNKSTA_LT, "LT"}, \
+ { PCI_EXP_LNKSTA_DLLLA, "DLLLA"}, \
+ { PCI_EXP_LNKSTA_LBMS, "LBMS"}, \
+ { PCI_EXP_LNKSTA_LABS, "LABS"}
+
+TRACE_EVENT(pcie_link_event,
+
+ TP_PROTO(struct pci_bus *bus,
+ unsigned int reason,
+ unsigned int width,
+ unsigned int status
+ ),
+
+ TP_ARGS(bus, reason, width, status),
+
+ TP_STRUCT__entry(
+ __string( port_name, pci_name(bus->self))
+ __field( unsigned int, type )
+ __field( unsigned int, reason )
+ __field( unsigned int, cur_bus_speed )
+ __field( unsigned int, max_bus_speed )
+ __field( unsigned int, width )
+ __field( unsigned int, flit_mode )
+ __field( unsigned int, link_status )
+ ),
+
+ TP_fast_assign(
+ __assign_str(port_name);
+ __entry->type = pci_pcie_type(bus->self);
+ __entry->reason = reason;
+ __entry->cur_bus_speed = bus->cur_bus_speed;
+ __entry->max_bus_speed = bus->max_bus_speed;
+ __entry->width = width;
+ __entry->flit_mode = bus->flit_mode;
+ __entry->link_status = status;
+ ),
+
+ TP_printk("%s type:%d, reason:%d, cur_bus_speed:%d, max_bus_speed:%d, width:%u, flit_mode:%u, status:%s\n",
+ __get_str(port_name),
+ __entry->type,
+ __entry->reason,
+ __entry->cur_bus_speed,
+ __entry->max_bus_speed,
+ __entry->width,
+ __entry->flit_mode,
+ __print_flags((unsigned long)__entry->link_status, "|",
+ LNKSTA_FLAGS)
+ )
+);
+
#endif /* _TRACE_HW_EVENT_PCI_H */
/* This part must be outside protection */
--
2.39.3
^ permalink raw reply related
* Re: [PATCH v1 1/1] bpf: Mark BPF printing functions with __printf() attribute
From: Andy Shevchenko @ 2025-12-10 13:13 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, bpf, LKML, linux-trace-kernel,
Daniel Borkmann, John Fastabend, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Alan Maguire,
kernel test robot
In-Reply-To: <CAADnVQLkSoOL8+kELdmX5nzNcXm-s4VbA5+Q-MPcNySsSiu+RQ@mail.gmail.com>
On Wed, Dec 10, 2025 at 04:09:19PM +0900, Alexei Starovoitov wrote:
> On Tue, Dec 9, 2025 at 10:37 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Tue, Dec 09, 2025 at 06:12:46PM +0900, Alexei Starovoitov wrote:
> > > On Tue, Dec 9, 2025 at 1:21 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > The printing functions in BPF code are using printf() type of format,
> > > > and compiler is not happy about them as is:
> > > >
> > > > kernel/bpf/helpers.c:1069:9: error: function ‘____bpf_snprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > > 1069 | err = bstr_printf(str, str_size, fmt, data.bin_args);
> > > > | ^~~
> > > >
> > > > kernel/trace/bpf_trace.c:377:9: error: function ‘____bpf_trace_printk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > > 377 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
> > > > | ^~~
> > > >
> > > > kernel/trace/bpf_trace.c:433:9: error: function ‘____bpf_trace_vprintk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > > 433 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
> > > > | ^~~
> > > >
> > > > kernel/trace/bpf_trace.c:475:9: error: function ‘____bpf_seq_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > > 475 | seq_bprintf(m, fmt, data.bin_args);
> > > > | ^~~~~~~~~~~
> > > >
> > > > Fix the compilation errors by adding __printf() attribute. For that
> > > > we need to pass it down to the BPF_CALL_x() and wrap into PRINTF_BPF_CALL_*()
> > > > to make code neater.
> >
> > > This is pointless churn to shut up a warning.
> >
> > In some cases, like mine, it's an error.
> >
> > > Teach syzbot to stop this spam instead.
> >
> > It prevents to perform `make W=1` builds with the default CONFIG_WERROR,
> > which is 'y'.
> >
> > > At the end this patch doesn't make any visible difference,
> > > since user declarations of these helpers are auto generated
> > > from uapi/bpf.h file and __printf attribute is not there.
> >
> > I see, thanks for the review.
> > Any recommendations on how to fix this properly?
>
> Add -Wno-suggest-attribute=format
> to corresponding files in Makefile.
Thanks, I just sent a new patch.
> I think it's cleaner than __diag_ignore() in the .c
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v1 1/1] bpf: Disable -Wsuggest-attribute=format
From: Andy Shevchenko @ 2025-12-10 13:12 UTC (permalink / raw)
To: Alexei Starovoitov, Steven Rostedt, bpf, linux-kernel,
linux-trace-kernel
Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Masami Hiramatsu, Mathieu Desnoyers, Andy Shevchenko,
kernel test robot
The printing functions in BPF code are using printf() type of format,
and compiler is not happy about them as is:
kernel/bpf/helpers.c:1069:9: error: function ‘____bpf_snprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
1069 | err = bstr_printf(str, str_size, fmt, data.bin_args);
| ^~~
kernel/bpf/stream.c:241:9: error: function ‘bpf_stream_vprintk_impl’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
241 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt__str, data.bin_args);
| ^~~
kernel/trace/bpf_trace.c:377:9: error: function ‘____bpf_trace_printk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
377 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
| ^~~
kernel/trace/bpf_trace.c:433:9: error: function ‘____bpf_trace_vprintk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
433 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
| ^~~
kernel/trace/bpf_trace.c:475:9: error: function ‘____bpf_seq_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
475 | seq_bprintf(m, fmt, data.bin_args);
| ^~~~~~~~~~~
Fix the compilation errors by disabling that warning since the code is
generated and warning is not so useful in this case — it can't check
the parameters for now.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202512061425.x0qTt9ww-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202512061640.9hKTnB8p-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202512081321.2h9ThWTg-lkp@intel.com/
Fixes: 5ab154f1463a ("bpf: Introduce BPF standard streams")
Fixes: 10aceb629e19 ("bpf: Add bpf_trace_vprintk helper")
Fixes: 7b15523a989b ("bpf: Add a bpf_snprintf helper")
Fixes: 492e639f0c22 ("bpf: Add bpf_seq_printf and bpf_seq_write helpers")
Fixes: f3694e001238 ("bpf: add BPF_CALL_x macros for declaring helpers")
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
kernel/bpf/Makefile | 11 +++++++++--
kernel/trace/Makefile | 6 ++++++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 232cbc97434d..cf7e8a972f98 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -6,7 +6,14 @@ cflags-nogcse-$(CONFIG_X86)$(CONFIG_CC_IS_GCC) := -fno-gcse
endif
CFLAGS_core.o += -Wno-override-init $(cflags-nogcse-yy)
-obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o log.o token.o liveness.o
+obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o tnum.o log.o token.o liveness.o
+
+obj-$(CONFIG_BPF_SYSCALL) += helpers.o stream.o
+# The ____bpf_snprintf() uses the format string that triggers a compiler warning.
+CFLAGS_helpers.o += -Wno-suggest-attribute=format
+# The bpf_stream_vprintk_impl() uses the format string that triggers a compiler warning.
+CFLAGS_stream.o += -Wno-suggest-attribute=format
+
obj-$(CONFIG_BPF_SYSCALL) += bpf_iter.o map_iter.o task_iter.o prog_iter.o link_iter.o
obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o bloom_filter.o
obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o ringbuf.o bpf_insn_array.o
@@ -14,7 +21,7 @@ obj-$(CONFIG_BPF_SYSCALL) += bpf_local_storage.o bpf_task_storage.o
obj-${CONFIG_BPF_LSM} += bpf_inode_storage.o
obj-$(CONFIG_BPF_SYSCALL) += disasm.o mprog.o
obj-$(CONFIG_BPF_JIT) += trampoline.o
-obj-$(CONFIG_BPF_SYSCALL) += btf.o memalloc.o rqspinlock.o stream.o
+obj-$(CONFIG_BPF_SYSCALL) += btf.o memalloc.o rqspinlock.o
ifeq ($(CONFIG_MMU)$(CONFIG_64BIT),yy)
obj-$(CONFIG_BPF_SYSCALL) += arena.o range_tree.o
endif
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index fc5dcc888e13..1673b395c14c 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -104,7 +104,13 @@ obj-$(CONFIG_TRACE_EVENT_INJECT) += trace_events_inject.o
obj-$(CONFIG_SYNTH_EVENTS) += trace_events_synth.o
obj-$(CONFIG_HIST_TRIGGERS) += trace_events_hist.o
obj-$(CONFIG_USER_EVENTS) += trace_events_user.o
+
obj-$(CONFIG_BPF_EVENTS) += bpf_trace.o
+# The BPF printing functions use the format string that triggers a compiler warning.
+# Since the code is generated and warning is not so useful in this case (it can't
+# check the parameters for now) disable the warning.
+CFLAGS_bpf_trace.o += -Wno-suggest-attribute=format
+
obj-$(CONFIG_KPROBE_EVENTS) += trace_kprobe.o
obj-$(CONFIG_TRACEPOINTS) += error_report-traces.o
obj-$(CONFIG_TRACEPOINTS) += power-traces.o
--
2.50.1
^ permalink raw reply related
* Re: [RFC LPC2025 PATCH 0/4] Deprecate zone_reclaim_mode
From: Joshua Hahn @ 2025-12-10 12:30 UTC (permalink / raw)
To: mawupeng
Cc: joshua.hahnjy, willy, david, linux-doc, linux-kernel, linux-mm,
linux-trace-kernel, linuxppc-dev, kernel-team
In-Reply-To: <fc00c53c-ab54-42a2-979b-0ecb49ff6b48@huawei.com>
On Tue, 9 Dec 2025 20:43:01 +0800 mawupeng <mawupeng1@huawei.com> wrote:
>
> On 2025/12/6 7:32, Joshua Hahn wrote:
> > Hello folks,
> > This is a code RFC for my upcoming discussion at LPC 2025 in Tokyo [1].
> >
> > zone_reclaim_mode was introduced in 2005 to prevent the kernel from facing
> > the high remote access latency associated with NUMA systems. With it enabled,
> > when the kernel sees that the local node is full, it will stall allocations and
> > trigger direct reclaim locally, instead of making a remote allocation, even
> > when there may still be free memory. Thsi is the preferred way to consume memory
> > if remote memory access is more expensive than performing direct reclaim.
> > The choice is made on a system-wide basis, but can be toggled at runtime.
> >
> > This series deprecates the zone_reclaim_mode sysctl in favor of other NUMA
> > aware mechanisms, such as NUMA balancing, memory.reclaim, membind, and
> > tiering / promotion / demotion. Let's break down what differences there are
> > in these mechanisms, based on workload characteristics.
[...snip...]
Hello mawupeng, thank you for your feedback on this RFC.
I was wondering if you were planning to attend LPC this year. If so, I'll be
discussing this idea at the MM microconference tomorrow (December 11th) and
would love to discuss this after the presentation with you in the hallway.
I want to make sure that I'm not missing any important nuances or use cases
for zone_reclaim_mode. After all, my only motivation for deprecating this is
to simplify the code allocation path and reduce maintenence burden, both of
which definitely does not outweigh valid usecases. On the other hand if we can
find out that we can deprecate zone_reclaim_mode, and also find some
alternatives that lead to better performance on your end, that sounds
like the ultimate win-win scenario for me : -)
> In real-world scenarios, we have observed on a dual-socket (2P) server with multiple
> NUMA nodes—each having relatively limited local memory capacity—that page cache
> negatively impacts overall performance. The zone_reclaim_node feature is used to
> alleviate performance issues.
>
> The main reason is that page cache consumes free memory on the local node, causing
> processes without mbind restrictions to fall back to other nodes that still have free
> memory. Accessing remote memory comes with a significant latency penalty. In extreme
> testing, if a system is fully populated with page cache beforehand, Spark application
> performance can drop by 80%. However, with zone_reclaim enabled, the performance
> degradation is limited to only about 30%.
This sounds right to me. In fact, I have observed similar results in some
experiments that I ran myself, where on a 2-NUMA system with 125GB memory each,
I fill up one node with 100G of garbage filecache and try to run a 60G anon
workload in it. Here are the average access latency results:
- zone_reclaim_mode enabled: 56.34 ns/access
- zone_reclaim_mode disabled: 67.86 ns/access
However, I was able to achieve better results by disabling zone_reclaim_mode
and using membind instead:
- zone_reclaim_mode disabled + membind: 52.98 ns/access
Of course, these are on my specific system with my specific workload so the
numbers (and results) may be different on your end. You specifically mentioned
"processes without mbind restrictions". Is there a reason why these workloads
cannot be membound to a node?
On that note, I had another follow-up question. If remote latency really is a
big concern, I am wondering if you have seen remote allocations despite
enabling zone_reclaim_mode. From my understanding of the code, zone_reclaim_mode
is not a strict guarantee of memory locality. If direct reclaim fails and
we fail to reclaim enough, the allocation is serviced from a remote node anyways.
Maybe I did not make this clear in my RFC, but I definitely believe that there
are workloads out there that benefit from zone_reclaim_mode. However, I
also believe that membind is just a better alternative for all the scenarios
that I can think of, so it would really be helpful for my education to learn
about workloads that benefit from zone_reclaim_mode but cannot use membind.
> Furthermore, for typical HPC applications, memory pressure tends to be balanced
> across NUMA nodes. Yet page cache is often generated by background tasks—such as
> logging modules—which breaks memory locality and adversely affects overall performance.
I see. From my very limited understanding of HPC applications, they tend to be
perfectly sized for the nodes they run on, so having logging agents generate
additional page cache really does sound like a problem to me.
> At the same time, there are a large number of __GFP_THISNODE memory allocation requests in
> the system. Anonymous pages that fall back from other nodes cannot be migrated or easily
> reclaimed (especially when swap is disabled), leading to uneven distribution of available
> memory within a single node. By enabling zone_reclaim_mode, the kernel preferentially reclaims
> file pages within the local NUMA node to satisfy local anonymous-page allocations, which
> effectively avoids warn_alloc problems caused by uneven distribution of anonymous pages.
>
> In such scenarios, relying solely on mbind may offer limited flexibility.
I see. So if I understand your scenario correctly, what you want is something
between mbind which is strict in guaranteeing that memory comes locally, and
the default memory allocation preference, which prefers allocating from
remote nodes when the local node runs out of memory.
I have some follow-up questions here.
It seems like the fact that anonymous memory from remote processes leaking
their memory into the current node is actually caused by two characteristics
of zone_reclaim_mode. Namely, that it does not guarantee memory locality,
and that it is a system-wide setting. Under your scenario, we cannot have
a mixture of HPC workloads that cannot handle remote memory access latency,
as well as non-HPC workloads that would actually benefit from being able to
consume free memory from remote nodes before triggering reclaim.
So in a scenario where we have multiple HPC workloads running on a multi-NUMA
system, we can just size each workload to fit the nodes, and membind them so
that we don't have to worry about migrating or reclaiming remote processes'
anonymous memory.
In a scenario where we have an HPC workload + non-HPC workloads, we can membind
the HPC workload to a single node, and exclude that node from the other
workloads' nodemasks to prevent anonymous memory from leaking into it.
> We have also experimented with proactively waking kswapd to improve synchronous reclaim
> efficiency. Our actual tests show that this can roughly double the memory allocation rate[1].
Personally I believe that this could be the way forward. However, there are
still some problems that we have to address, the biggest one being: pagecache
can be considered "garbage" in both your HPC workloads and my microbenchmark.
However, the pagecache can be very valuable in certain scenarios. What if
the workload will access the pagecache in the future? I'm not really sure if
it makes sense to clean up that pagecache and allocate locally, when the
worst-case scenario is that we have to incur much more latency reading from
disk and bringing in those pages again, when there is free memory still
available in the system.
Perhaps the real solution is to deprecate zone_reclaim_mode and offer more
granular (per-workload basis), and sane (guarantee memory locality and also
perform kswapd when the ndoe is full) options for the user.
> We could also discuss whether there are better solutions for such HPC scenarios.
Yes, I really hope that we can reach the win-win scenario that I mentioned at
the beginning of the reply. I really want to help users achieve the best
performance they can, and also help keep the kernel easy to maintain in the
long-run. Thank you for sharing your perspective, I really learned a lot.
Looking forward to your response, or if you are coming to LPC, would love to
grab a coffee. Have a grat day!
Joshua
> [1]: https://lore.kernel.org/all/20251011062043.772549-1-mawupeng1@huawei.com/
^ permalink raw reply
* Re: [PATCH bpf v5 0/2] bpf: fix bpf_d_path() helper prototype
From: patchwork-bot+netdevbpf @ 2025-12-10 9:40 UTC (permalink / raw)
To: Shuran Liu
Cc: song, mattbobrowski, bpf, ast, daniel, andrii, martin.lau,
eddyz87, yonghong.song, john.fastabend, kpsingh, sdf, haoluo,
jolsa, rostedt, mhiramat, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, dxu, linux-kselftest, shuah
In-Reply-To: <20251206141210.3148-1-electronlsr@gmail.com>
Hello:
This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Sat, 6 Dec 2025 22:12:08 +0800 you wrote:
> Hi,
>
> This series fixes a verifier issue with bpf_d_path() and adds a
> regression test to cover its use within a hook function.
>
> Patch 1 updates the bpf_d_path() helper prototype so that the second
> argument is marked as MEM_WRITE. This makes it explicit to the verifier
> that the helper writes into the provided buffer.
>
> [...]
Here is the summary with links:
- [bpf,v5,1/2] bpf: mark bpf_d_path() buffer as writeable
https://git.kernel.org/bpf/bpf/c/ac44dcc788b9
- [bpf,v5,2/2] selftests/bpf: add regression test for bpf_d_path()
https://git.kernel.org/bpf/bpf/c/79e247d66088
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v1 1/1] bpf: Mark BPF printing functions with __printf() attribute
From: Alexei Starovoitov @ 2025-12-10 7:09 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Alexei Starovoitov, bpf, LKML, linux-trace-kernel,
Daniel Borkmann, John Fastabend, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Alan Maguire,
kernel test robot
In-Reply-To: <aTgl_bjO1O9Ddpmv@smile.fi.intel.com>
On Tue, Dec 9, 2025 at 10:37 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Dec 09, 2025 at 06:12:46PM +0900, Alexei Starovoitov wrote:
> > On Tue, Dec 9, 2025 at 1:21 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > The printing functions in BPF code are using printf() type of format,
> > > and compiler is not happy about them as is:
> > >
> > > kernel/bpf/helpers.c:1069:9: error: function ‘____bpf_snprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 1069 | err = bstr_printf(str, str_size, fmt, data.bin_args);
> > > | ^~~
> > >
> > > kernel/trace/bpf_trace.c:377:9: error: function ‘____bpf_trace_printk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 377 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
> > > | ^~~
> > >
> > > kernel/trace/bpf_trace.c:433:9: error: function ‘____bpf_trace_vprintk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 433 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
> > > | ^~~
> > >
> > > kernel/trace/bpf_trace.c:475:9: error: function ‘____bpf_seq_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 475 | seq_bprintf(m, fmt, data.bin_args);
> > > | ^~~~~~~~~~~
> > >
> > > Fix the compilation errors by adding __printf() attribute. For that
> > > we need to pass it down to the BPF_CALL_x() and wrap into PRINTF_BPF_CALL_*()
> > > to make code neater.
>
> > This is pointless churn to shut up a warning.
>
> In some cases, like mine, it's an error.
>
> > Teach syzbot to stop this spam instead.
>
> It prevents to perform `make W=1` builds with the default CONFIG_WERROR,
> which is 'y'.
>
> > At the end this patch doesn't make any visible difference,
> > since user declarations of these helpers are auto generated
> > from uapi/bpf.h file and __printf attribute is not there.
>
> I see, thanks for the review.
> Any recommendations on how to fix this properly?
Add -Wno-suggest-attribute=format
to corresponding files in Makefile.
I think it's cleaner than __diag_ignore() in the .c
^ permalink raw reply
* [[PATCH v2] 1/1] kprobes: retry pending optprobe after freeing blocker
From: hongao @ 2025-12-10 3:33 UTC (permalink / raw)
To: naveen, anil.s.keshavamurthy, davem, mhiramat
Cc: linux-kernel, linux-trace-kernel, hongao
The freeing_list cleanup now retries optimizing any sibling probe that was
deferred while this aggregator was being torn down. Track the pending
address in struct optimized_kprobe so __disarm_kprobe() can defer the
retry until kprobe_optimizer() finishes disarming.
Signed-off-by: hongao <hongao@uniontech.com>
---
Changes since v1:
- Replace `kprobe_opcode_t *pending_reopt_addr` with `bool reopt_unblocked_probes`
in `struct optimized_kprobe` to avoid storing an address and simplify logic.
- Use `op->kp.addr` when looking up the sibling optimized probe instead of
keeping a separate stored address.
- Defer re-optimization by setting/clearing `op->reopt_unblocked_probes` in
`__disarm_kprobe()` / consuming it in `do_free_cleaned_kprobes()` so the
retry runs after the worker finishes disarming.
- Link to v1: https://lore.kernel.org/all/2B0BC73E9D190B7B+20251027130535.2296913-1-hongao@uniontech.com/
---
include/linux/kprobes.h | 1 +
kernel/kprobes.c | 28 ++++++++++++++++++++++------
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 8c4f3bb24..4f49925a4 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -338,6 +338,7 @@ DEFINE_INSN_CACHE_OPS(insn);
struct optimized_kprobe {
struct kprobe kp;
struct list_head list; /* list for optimizing queue */
+ bool reopt_unblocked_probes;
struct arch_optimized_insn optinsn;
};
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index da59c68df..799542dff 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -514,6 +514,7 @@ static LIST_HEAD(freeing_list);
static void kprobe_optimizer(struct work_struct *work);
static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
+static void optimize_kprobe(struct kprobe *p);
#define OPTIMIZE_DELAY 5
/*
@@ -591,6 +592,21 @@ static void do_free_cleaned_kprobes(void)
*/
continue;
}
+ if (op->reopt_unblocked_probes) {
+ struct kprobe *unblocked;
+
+ /*
+ * The aggregator was holding back another probe while it sat on the
+ * unoptimizing/freeing lists. Now that the aggregator has been fully
+ * reverted we can safely retry the optimization of that sibling.
+ */
+
+ unblocked = get_optimized_kprobe(op->kp.addr);
+ if (unlikely(unblocked))
+ optimize_kprobe(unblocked);
+ op->reopt_unblocked_probes = false;
+ }
+
free_aggr_kprobe(&op->kp);
}
}
@@ -1009,13 +1025,13 @@ static void __disarm_kprobe(struct kprobe *p, bool reopt)
_p = get_optimized_kprobe(p->addr);
if (unlikely(_p) && reopt)
optimize_kprobe(_p);
+ } else if (reopt && kprobe_aggrprobe(p)) {
+ struct optimized_kprobe *op =
+ container_of(p, struct optimized_kprobe, kp);
+
+ /* Defer the re-optimization until the worker finishes disarming. */
+ op->reopt_unblocked_probes = true;
}
- /*
- * TODO: Since unoptimization and real disarming will be done by
- * the worker thread, we can not check whether another probe are
- * unoptimized because of this probe here. It should be re-optimized
- * by the worker thread.
- */
}
#else /* !CONFIG_OPTPROBES */
--
2.47.2
^ permalink raw reply related
* Re: [PATCH 2/2] mm: vmscan: add PIDs to vmscan tracepoints
From: Steven Rostedt @ 2025-12-10 3:09 UTC (permalink / raw)
To: Thomas Ballasi
Cc: Masami Hiramatsu, Andrew Morton, linux-mm, linux-trace-kernel
In-Reply-To: <20251208181413.4722-3-tballasi@linux.microsoft.com>
On Mon, 8 Dec 2025 10:14:13 -0800
Thomas Ballasi <tballasi@linux.microsoft.com> wrote:
> ---
> include/trace/events/vmscan.h | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index afc9f80d03f34..eddb4e75e2e23 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -121,18 +121,21 @@ DECLARE_EVENT_CLASS(mm_vmscan_direct_reclaim_begin_template,
> TP_STRUCT__entry(
> __field( int, order )
> __field( unsigned long, gfp_flags )
> + __field( int, pid )
This puts a hole in the ring buffer on 64 bit machines. Please keep pid
next to order as they are both 'int' and not have an "unsigned long"
between the two.
> __field( unsigned short, memcg_id )
> ),
-- Steve
^ permalink raw reply
* Re: [PATCH v1] tracing: Avoid possible signed 64-bit truncation
From: Steven Rostedt @ 2025-12-10 2:31 UTC (permalink / raw)
To: Ian Rogers
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251209224024.2322124-1-irogers@google.com>
On Tue, 9 Dec 2025 14:40:24 -0800
Ian Rogers <irogers@google.com> wrote:
> 64-bit truncation to 32-bit can result in the sign of the truncated
> value changing. The cmp_mod_entry is used in bsearch and so the
> truncation could result in an invalid search order. This would only
> happen were the addresses more than 2GB apart and so unlikely, but
> let's fix the potentially broken compare anyway.
I'm fine with fixing this but I believe if the addresses are more than
2GB apart there could be other issues elsewhere ;-)
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> kernel/trace/trace.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index d1e527cf2aae..e6a80cbe9326 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6057,8 +6057,10 @@ static int cmp_mod_entry(const void *key, const void *pivot)
>
> if (addr >= ent[0].mod_addr && addr < ent[1].mod_addr)
> return 0;
> + else if (addr > ent->mod_addr)
> + return 1;
> else
> - return addr - ent->mod_addr;
> + return -1;
Could we still keep this down to a single if check?
if (addr < ent->mod_addr)
return -1;
return addr >= ent[1].mod_addr;
-- Steve
> }
>
> /**
^ permalink raw reply
* Re: [External] : [PATCH v13 3/3] Documentation: tracing: Add documentation about PCI tracepoints
From: Shuai Xue @ 2025-12-10 1:56 UTC (permalink / raw)
To: ALOK TIWARI, rostedt, lukas, linux-pci, linux-kernel, linux-edac,
linux-trace-kernel, helgaas, ilpo.jarvinen, mattc,
Jonathan.Cameron
Cc: bhelgaas, tony.luck, bp, mhiramat, mathieu.desnoyers, oleg,
naveen, davem, anil.s.keshavamurthy, mark.rutland, peterz,
tianruidong
In-Reply-To: <1ad20492-f342-4a4b-8a2f-228db7c5673a@oracle.com>
Hi, Alok,
在 2025/12/9 21:59, ALOK TIWARI 写道:
>
>
> On 10/25/2025 5:11 PM, Shuai Xue wrote:
>> The PCI tracing system provides tracepoints to monitor critical hardware
>> events that can impact system performance and reliability. Add
>> documentation about it.
>>
>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> ---
>> Documentation/trace/events-pci.rst | 74 ++++++++++++++++++++++++++++++
>> 1 file changed, 74 insertions(+)
>> create mode 100644 Documentation/trace/events-pci.rst
>>
>> diff --git a/Documentation/trace/events-pci.rst b/Documentation/trace/events-pci.rst
>> new file mode 100644
>> index 000000000000..88bd38fcc184
>> --- /dev/null
>> +++ b/Documentation/trace/events-pci.rst
>> @@ -0,0 +1,74 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +===========================
>> +Subsystem Trace Points: PCI
>> +===========================
>> +
>> +Overview
>> +========
>> +The PCI tracing system provides tracepoints to monitor critical hardware events
>> +that can impact system performance and reliability. These events normally show
>> +up here:
>> +
>> + /sys/kernel/tracing/events/pci
>> +
>> +Cf. include/trace/events/pci.h for the events definitions.
>> +
>> +Available Tracepoints
>> +=====================
>> +
>> +pci_hp_event
>> +------------
>> +
>> +Monitors PCI hotplug events including card insertion/removal and link
>> +state changes.
>> +::
>> +
>> + pci_hp_event "%s slot:%s, event:%s\n"
>> +
>> +**Event Types**:
>> +
>> +* ``LINK_UP`` - PCIe link established
>> +* ``LINK_DOWN`` - PCIe link lost
>> +* ``CARD_PRESENT`` - Card detected in slot
>> +* ``CARD_NOT_PRESENT`` - Card removed from slot
>> +
>> +**Example Usage**:
>> +
>> + # Enable the tracepoint
>> + echo 1> /sys/kernel/debug/tracing/events/pci/pci_hp_event/enable
>
> missing space echo 1 >"
Good catch.
>
>> +
>> + # Monitor events (the following output is generated when a device is hotplugged)
>> + cat /sys/kernel/debug/tracing/trace_pipe
>> + irq/51-pciehp-88 [001] ..... 1311.177459: pci_hp_event: 0000:00:02.0 slot:10, event:CARD_PRESENT
>> +
>> + irq/51-pciehp-88 [001] ..... 1311.177566: pci_hp_event: 0000:00:02.0 slot:10, event:LINK_UP
>> +
>> +pcie_link_event
>> +---------------
>> +
>> +Monitors PCIe link speed changes and provides detailed link status information.
>> +::
>> +
>> + pcie_link_event "%s type:%d, reason:%d, cur_bus_speed:%s, max_bus_speed:%s, width:%u, flit_mode:%u, status:%s\n"
>
> %s -> %d mismatch for cur_bus_speed and max_bus_speed
>
> TP_printk("%s type:%d, reason:%d, cur_bus_speed:%d, max_bus_speed:%d, width:%u, flit_mode:%u, status:%s\n",
Sorry for missing the format.
>
>> +
>> +**Parameters**:
>> +
>> +* ``type`` - PCIe device type (4=Root Port, etc.)
>> +* ``reason`` - Reason for link change:
>> +
>> + - ``0`` - Link retrain
>> + - ``1`` - Bus enumeration
>> + - ``2`` - Bandwidth notification enable
>> + - ``3`` - Bandwidth notification IRQ
>> + - ``4`` - Hotplug event
>> +
>> +
>> +**Example Usage**:
>> +
>> + # Enable the tracepoint
>> + echo1 > /sys/kernel/debug/tracing/events/pci/pcie_link_event/enable
>> +
>> + # Monitor events (the following output is generated when a device is hotplugged)
>> + cat /sys/kernel/debug/tracing/trace_pipe
>> + irq/51-pciehp-88 [001] ..... 381.545386: pcie_link_event: 0000:00:02.0 type:4, reason:4, cur_bus_speed:20, max_bus_speed:23, width:1, flit_mode:0, status:DLLLA
>
>
> Thanks,
> Alok
I will send a new version to fix above issues.
Thanks.
Shuai
^ permalink raw reply
* [PATCH v2] tracing: Fix unused tracepoints when module uses only exported ones
From: Steven Rostedt @ 2025-12-10 1:40 UTC (permalink / raw)
To: LKML, Linux trace kernel, Masami Hiramatsu, Mathieu Desnoyers,
Masahiro Yamada
From: Steven Rostedt <rostedt@goodmis.org>
Building the KVM intel module failed to build with UT=1:
no __tracepoint_strings in file: arch/x86/kvm/kvm-intel.o
make[3]: *** [/work/git/test-linux.git/scripts/Makefile.modfinal:62: arch/x86/kvm/kvm-intel.ko] Error 1
The reason is that the module only uses the tracepoints defined and
exported by the main kvm module. The tracepoint-update.c code fails the
build if a tracepoint is used, but there's no tracepoints defined. But
this is acceptable in modules if the tracepoints are defined in the vmlinux
proper or another module and exported.
Do not fail to build if a tracepoint is used but no tracepoints are
defined if the code is a module. This should still never happen for the
vmlinux itself.
Fixes: e30f8e61e2518 ("tracing: Add a tracepoint verification check at build time")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://patch.msgid.link/20251208085336.6658743c@debian
- Just fixed the change log for grammar and spelling erros (Mathieu Desnoyers)
scripts/tracepoint-update.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/scripts/tracepoint-update.c b/scripts/tracepoint-update.c
index 7f7d90df14ce..90046aedc97b 100644
--- a/scripts/tracepoint-update.c
+++ b/scripts/tracepoint-update.c
@@ -210,6 +210,9 @@ static int process_tracepoints(bool mod, void *addr, const char *fname)
}
if (!tracepoint_data_sec) {
+ /* A module may reference only exported tracepoints */
+ if (mod)
+ return 0;
fprintf(stderr, "no __tracepoint_strings in file: %s\n", fname);
return -1;
}
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox