* [PATCH v2] blk-mq: use NOIO context to prevent deadlock during debugfs creation
From: Yu Kuai @ 2026-02-14 5:43 UTC (permalink / raw)
To: axboe, nilay, ming.lei, hch
Cc: yi.zhang, shinichiro.kawasaki, kbusch, rostedt, mhiramat,
mathieu.desnoyers, linux-block, linux-kernel, linux-trace-kernel
Creating debugfs entries can trigger fs reclaim, which can enter back
into the block layer request_queue. This can cause deadlock if the
queue is frozen.
Previously, a WARN_ON_ONCE check was used in debugfs_create_files()
to detect this condition, but it was racy since the queue can be frozen
from another context at any time.
Introduce blk_debugfs_lock()/blk_debugfs_unlock() helpers that combine
the debugfs_mutex with memalloc_noio_save()/restore() to prevent fs
reclaim from triggering block I/O. Also add blk_debugfs_lock_nomemsave()
and blk_debugfs_unlock_nomemrestore() variants for callers that don't
need NOIO protection (e.g., debugfs removal or read-only operations).
Replace all raw debugfs_mutex lock/unlock pairs with these helpers,
using the _nomemsave/_nomemrestore variants where appropriate.
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Closes: https://lore.kernel.org/all/CAHj4cs9gNKEYAPagD9JADfO5UH+OiCr4P7OO2wjpfOYeM-RV=A@mail.gmail.com/
Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Closes: https://lore.kernel.org/all/aYWQR7CtYdk3K39g@shinmob/
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Yu Kuai <yukuai@fnnas.com>
---
block/blk-mq-debugfs.c | 10 +++-------
block/blk-mq-sched.c | 9 +++++----
block/blk-sysfs.c | 9 +++++----
block/blk-wbt.c | 10 ++++++----
block/blk.h | 31 +++++++++++++++++++++++++++++++
kernel/trace/blktrace.c | 38 +++++++++++++++++++++-----------------
6 files changed, 71 insertions(+), 36 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index faeaa1fc86a7..28167c9baa55 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -613,11 +613,6 @@ static void debugfs_create_files(struct request_queue *q, struct dentry *parent,
const struct blk_mq_debugfs_attr *attr)
{
lockdep_assert_held(&q->debugfs_mutex);
- /*
- * Creating new debugfs entries with queue freezed has the risk of
- * deadlock.
- */
- WARN_ON_ONCE(q->mq_freeze_depth != 0);
/*
* debugfs_mutex should not be nested under other locks that can be
* grabbed while queue is frozen.
@@ -693,12 +688,13 @@ void blk_mq_debugfs_unregister_hctx(struct blk_mq_hw_ctx *hctx)
void blk_mq_debugfs_register_hctxs(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
+ unsigned int memflags;
unsigned long i;
- mutex_lock(&q->debugfs_mutex);
+ memflags = blk_debugfs_lock(q);
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_debugfs_register_hctx(q, hctx);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
}
void blk_mq_debugfs_unregister_hctxs(struct request_queue *q)
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
index e26898128a7e..97c3c8f45a9b 100644
--- a/block/blk-mq-sched.c
+++ b/block/blk-mq-sched.c
@@ -390,13 +390,14 @@ static void blk_mq_sched_tags_teardown(struct request_queue *q, unsigned int fla
void blk_mq_sched_reg_debugfs(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
+ unsigned int memflags;
unsigned long i;
- mutex_lock(&q->debugfs_mutex);
+ memflags = blk_debugfs_lock(q);
blk_mq_debugfs_register_sched(q);
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_debugfs_register_sched_hctx(q, hctx);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
}
void blk_mq_sched_unreg_debugfs(struct request_queue *q)
@@ -404,11 +405,11 @@ void blk_mq_sched_unreg_debugfs(struct request_queue *q)
struct blk_mq_hw_ctx *hctx;
unsigned long i;
- mutex_lock(&q->debugfs_mutex);
+ blk_debugfs_lock_nomemsave(q);
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_debugfs_unregister_sched_hctx(hctx);
blk_mq_debugfs_unregister_sched(q);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock_nomemrestore(q);
}
void blk_mq_free_sched_tags(struct elevator_tags *et,
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 003aa684e854..f3b1968c80ce 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -892,13 +892,13 @@ static void blk_debugfs_remove(struct gendisk *disk)
{
struct request_queue *q = disk->queue;
- mutex_lock(&q->debugfs_mutex);
+ blk_debugfs_lock_nomemsave(q);
blk_trace_shutdown(q);
debugfs_remove_recursive(q->debugfs_dir);
q->debugfs_dir = NULL;
q->sched_debugfs_dir = NULL;
q->rqos_debugfs_dir = NULL;
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock_nomemrestore(q);
}
/**
@@ -908,6 +908,7 @@ static void blk_debugfs_remove(struct gendisk *disk)
int blk_register_queue(struct gendisk *disk)
{
struct request_queue *q = disk->queue;
+ unsigned int memflags;
int ret;
ret = kobject_add(&disk->queue_kobj, &disk_to_dev(disk)->kobj, "queue");
@@ -921,11 +922,11 @@ int blk_register_queue(struct gendisk *disk)
}
mutex_lock(&q->sysfs_lock);
- mutex_lock(&q->debugfs_mutex);
+ memflags = blk_debugfs_lock(q);
q->debugfs_dir = debugfs_create_dir(disk->disk_name, blk_debugfs_root);
if (queue_is_mq(q))
blk_mq_debugfs_register(q);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
ret = disk_register_independent_access_ranges(disk);
if (ret)
diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index 1415f2bf8611..6dba71e87387 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -776,6 +776,7 @@ void wbt_init_enable_default(struct gendisk *disk)
{
struct request_queue *q = disk->queue;
struct rq_wb *rwb;
+ unsigned int memflags;
if (!__wbt_enable_default(disk))
return;
@@ -789,9 +790,9 @@ void wbt_init_enable_default(struct gendisk *disk)
return;
}
- mutex_lock(&q->debugfs_mutex);
+ memflags = blk_debugfs_lock(q);
blk_mq_debugfs_register_rq_qos(q);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
}
static u64 wbt_default_latency_nsec(struct request_queue *q)
@@ -1015,9 +1016,10 @@ int wbt_set_lat(struct gendisk *disk, s64 val)
blk_mq_unquiesce_queue(q);
out:
blk_mq_unfreeze_queue(q, memflags);
- mutex_lock(&q->debugfs_mutex);
+
+ memflags = blk_debugfs_lock(q);
blk_mq_debugfs_register_rq_qos(q);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
return ret;
}
diff --git a/block/blk.h b/block/blk.h
index 401d19ed08a6..68cef70e84e6 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -740,4 +740,35 @@ static inline void blk_unfreeze_release_lock(struct request_queue *q)
}
#endif
+/*
+ * debugfs directory and file creation can trigger fs reclaim, which can enter
+ * back into the block layer request_queue. This can cause deadlock if the
+ * queue is frozen. Use NOIO context together with debugfs_mutex to prevent fs
+ * reclaim from triggering block I/O.
+ */
+static inline void blk_debugfs_lock_nomemsave(struct request_queue *q)
+{
+ mutex_lock(&q->debugfs_mutex);
+}
+
+static inline void blk_debugfs_unlock_nomemrestore(struct request_queue *q)
+{
+ mutex_unlock(&q->debugfs_mutex);
+}
+
+static inline unsigned int __must_check blk_debugfs_lock(struct request_queue *q)
+{
+ unsigned int memflags = memalloc_noio_save();
+
+ blk_debugfs_lock_nomemsave(q);
+ return memflags;
+}
+
+static inline void blk_debugfs_unlock(struct request_queue *q,
+ unsigned int memflags)
+{
+ blk_debugfs_unlock_nomemrestore(q);
+ memalloc_noio_restore(memflags);
+}
+
#endif /* BLK_INTERNAL_H */
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index c4db5c2e7103..a3d8a68f8683 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -559,9 +559,9 @@ int blk_trace_remove(struct request_queue *q)
{
int ret;
- mutex_lock(&q->debugfs_mutex);
+ blk_debugfs_lock_nomemsave(q);
ret = __blk_trace_remove(q);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock_nomemrestore(q);
return ret;
}
@@ -767,6 +767,7 @@ int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
struct blk_user_trace_setup2 buts2;
struct blk_user_trace_setup buts;
struct blk_trace *bt;
+ unsigned int memflags;
int ret;
ret = copy_from_user(&buts, arg, sizeof(buts));
@@ -785,16 +786,16 @@ int blk_trace_setup(struct request_queue *q, char *name, dev_t dev,
.pid = buts.pid,
};
- mutex_lock(&q->debugfs_mutex);
+ memflags = blk_debugfs_lock(q);
bt = blk_trace_setup_prepare(q, name, dev, buts.buf_size, buts.buf_nr,
bdev);
if (IS_ERR(bt)) {
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
return PTR_ERR(bt);
}
blk_trace_setup_finalize(q, name, 1, bt, &buts2);
strscpy(buts.name, buts2.name, BLKTRACE_BDEV_SIZE);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
if (copy_to_user(arg, &buts, sizeof(buts))) {
blk_trace_remove(q);
@@ -809,6 +810,7 @@ static int blk_trace_setup2(struct request_queue *q, char *name, dev_t dev,
{
struct blk_user_trace_setup2 buts2;
struct blk_trace *bt;
+ unsigned int memflags;
if (copy_from_user(&buts2, arg, sizeof(buts2)))
return -EFAULT;
@@ -819,15 +821,15 @@ static int blk_trace_setup2(struct request_queue *q, char *name, dev_t dev,
if (buts2.flags != 0)
return -EINVAL;
- mutex_lock(&q->debugfs_mutex);
+ memflags = blk_debugfs_lock(q);
bt = blk_trace_setup_prepare(q, name, dev, buts2.buf_size, buts2.buf_nr,
bdev);
if (IS_ERR(bt)) {
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
return PTR_ERR(bt);
}
blk_trace_setup_finalize(q, name, 2, bt, &buts2);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
if (copy_to_user(arg, &buts2, sizeof(buts2))) {
blk_trace_remove(q);
@@ -844,6 +846,7 @@ static int compat_blk_trace_setup(struct request_queue *q, char *name,
struct blk_user_trace_setup2 buts2;
struct compat_blk_user_trace_setup cbuts;
struct blk_trace *bt;
+ unsigned int memflags;
if (copy_from_user(&cbuts, arg, sizeof(cbuts)))
return -EFAULT;
@@ -860,15 +863,15 @@ static int compat_blk_trace_setup(struct request_queue *q, char *name,
.pid = cbuts.pid,
};
- mutex_lock(&q->debugfs_mutex);
+ memflags = blk_debugfs_lock(q);
bt = blk_trace_setup_prepare(q, name, dev, buts2.buf_size, buts2.buf_nr,
bdev);
if (IS_ERR(bt)) {
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
return PTR_ERR(bt);
}
blk_trace_setup_finalize(q, name, 1, bt, &buts2);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
if (copy_to_user(arg, &buts2.name, ARRAY_SIZE(buts2.name))) {
blk_trace_remove(q);
@@ -898,9 +901,9 @@ int blk_trace_startstop(struct request_queue *q, int start)
{
int ret;
- mutex_lock(&q->debugfs_mutex);
+ blk_debugfs_lock_nomemsave(q);
ret = __blk_trace_startstop(q, start);
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock_nomemrestore(q);
return ret;
}
@@ -2020,7 +2023,7 @@ static ssize_t sysfs_blk_trace_attr_show(struct device *dev,
struct blk_trace *bt;
ssize_t ret = -ENXIO;
- mutex_lock(&q->debugfs_mutex);
+ blk_debugfs_lock_nomemsave(q);
bt = rcu_dereference_protected(q->blk_trace,
lockdep_is_held(&q->debugfs_mutex));
@@ -2041,7 +2044,7 @@ static ssize_t sysfs_blk_trace_attr_show(struct device *dev,
ret = sprintf(buf, "%llu\n", bt->end_lba);
out_unlock_bdev:
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock_nomemrestore(q);
return ret;
}
@@ -2052,6 +2055,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
struct block_device *bdev = dev_to_bdev(dev);
struct request_queue *q = bdev_get_queue(bdev);
struct blk_trace *bt;
+ unsigned int memflags;
u64 value;
ssize_t ret = -EINVAL;
@@ -2071,7 +2075,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
goto out;
}
- mutex_lock(&q->debugfs_mutex);
+ memflags = blk_debugfs_lock(q);
bt = rcu_dereference_protected(q->blk_trace,
lockdep_is_held(&q->debugfs_mutex));
@@ -2106,7 +2110,7 @@ static ssize_t sysfs_blk_trace_attr_store(struct device *dev,
}
out_unlock_bdev:
- mutex_unlock(&q->debugfs_mutex);
+ blk_debugfs_unlock(q, memflags);
out:
return ret ? ret : count;
}
--
2.51.0
^ permalink raw reply related
* Enhancing Conditional Filtering for Function Graph Tracer
From: Donglin Peng @ 2026-02-14 10:36 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: linux-trace-kernel, Linux Kernel Mailing List
Hi Steve and Masami,
I recently had an idea while using the function graph tracer.
Currently, it supports basic
filtering via set_ftrace_pid(PID-based) and
set_graph_function(function name-based).
However, this filtering mechanism feels somewhat limited. Given that
we already have
features like funcgraph-args, I wonder if we could enhance filtering by allowing
conditional tracking based on function parameters — similar to how
trace events support
filters/triggers.
To simplify implementation, I propose extending a new trigger type
(e.g., "funcgraph").
In ftrace_graph_ignore_func, we could look up the corresponding trace_fprobe and
trace_event_file based on trace->func, then decide whether to trace
the function using
a helper like the following:
static bool ftrace_graph_filter(struct trace_fprobe *tf, struct
ftrace_regs *fregs,
struct trace_event_file *trace_file)
{
struct fentry_trace_entry_head *entry;
struct trace_event_buffer fbuffer;
struct event_trigger_data *data;
int dsize;
dsize = __get_data_size(&tf->tp, fregs, NULL);
entry = trace_event_buffer_reserve(&fbuffer, trace_file,
sizeof(*entry) + tf->tp.size + dsize);
if (!entry)
return false;
entry = ring_buffer_event_data(fbuffer.event);
store_trace_args(&entry[1], &tf->tp, fregs, NULL, sizeof(*entry), dsize);
list_for_each_entry_rcu(data, &trace_file->triggers, list) {
if (data->cmd_ops->trigger_type == TRIGGER_TYPE_FUNCGRAPH) {
struct event_filter *filter = rcu_dereference_sched(data->filter);
if (filter && filter_match_preds(filter, entry))
return true; // Allow tracing
}
}
return false; // Skip tracing
}
Does this approach make sense? Any suggestions or concerns?
Thanks,
Donglin
^ permalink raw reply
* Re: [RFC PATCH v2 33/37] KVM: selftests: Make TEST_EXPECT_SIGBUS thread-safe
From: Ackerley Tng @ 2026-02-14 19:49 UTC (permalink / raw)
To: kvm, linux-doc, linux-kernel, linux-kselftest, linux-trace-kernel,
x86
Cc: aik, andrew.jones, binbin.wu, bp, brauner, chao.p.peng,
chao.p.peng, chenhuacai, corbet, dave.hansen, david, hpa,
ira.weiny, jgg, jmattson, jroedel, jthoughton, maobibo,
mathieu.desnoyers, maz, mhiramat, michael.roth, mingo, mlevitsk,
oupton, pankaj.gupta, pbonzini, prsampat, qperret, ricarkol,
rick.p.edgecombe, rientjes, rostedt, seanjc, shivankg, shuah,
steven.price, tabba, tglx, vannapurve, vbabka, willy, wyihan,
yan.y.zhao
In-Reply-To: <995398ca18fcb192444799a520cab5ea8e43df7b.1770071243.git.ackerleytng@google.com>
Ackerley Tng <ackerleytng@google.com> writes:
> The TEST_EXPECT_SIGBUS macro is not thread-safe as it uses a global
> sigjmp_buf and installs a global SIGBUS signal handler. If multiple threads
> execute the macro concurrently, they will race on installing the signal
> handler and stomp on other threads' jump buffers, leading to incorrect test
> behavior.
>
> Make TEST_EXPECT_SIGBUS thread-safe with the following changes:
>
> Share the KVM tests' global signal handler. sigaction() applies to all
> threads; without sharing a global signal handler, one thread may have
> removed the signal handler that another thread added, hence leading to
> unexpected signals.
>
> The alternative of layering signal handlers was considered, but calling
> sigaction() within TEST_EXPECT_SIGBUS() necessarily creates a race. To
> avoid adding new setup and teardown routines to do sigaction() and keep
> usage of TEST_EXPECT_SIGBUS() simple, share the KVM tests' global signal
> handler.
>
> Opportunistically rename report_unexpected_signal to
> catchall_signal_handler.
>
> To continue to only expect SIGBUS within specific regions of code, use a
> thread-specific variable, expecting_sigbus, to replace installing and
> removing signal handlers.
>
> Make the execution environment for the thread, sigjmp_buf, a
> thread-specific variable.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
> .../testing/selftests/kvm/include/test_util.h | 29 +++++++++----------
> tools/testing/selftests/kvm/lib/kvm_util.c | 18 ++++++++----
> tools/testing/selftests/kvm/lib/test_util.c | 7 -----
> 3 files changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index 2871a4292847..0e4e6f7dab8f 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -80,22 +80,19 @@ do { \
> __builtin_unreachable(); \
> } while (0)
>
> -extern sigjmp_buf expect_sigbus_jmpbuf;
> -void expect_sigbus_handler(int signum);
> -
> -#define TEST_EXPECT_SIGBUS(action) \
> -do { \
> - struct sigaction sa_old, sa_new = { \
> - .sa_handler = expect_sigbus_handler, \
> - }; \
> - \
> - sigaction(SIGBUS, &sa_new, &sa_old); \
> - if (sigsetjmp(expect_sigbus_jmpbuf, 1) == 0) { \
> - action; \
> - TEST_FAIL("'%s' should have triggered SIGBUS", #action); \
> - } \
> - sigaction(SIGBUS, &sa_old, NULL); \
> -} while (0)
> +extern __thread sigjmp_buf expect_sigbus_jmpbuf;
> +extern __thread bool expecting_sigbus;
> +
> +#define TEST_EXPECT_SIGBUS(action) \
> + do { \
> + expecting_sigbus = true; \
> + if (sigsetjmp(expect_sigbus_jmpbuf, 1) == 0) { \
> + action; \
> + TEST_FAIL("'%s' should have triggered SIGBUS", \
> + #action); \
> + } \
> + expecting_sigbus = false; \
> + } while (0)
>
> size_t parse_size(const char *size);
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index aec7b24418ab..18ced8bdde36 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -2314,13 +2314,20 @@ __weak void kvm_selftest_arch_init(void)
> {
> }
>
> -static void report_unexpected_signal(int signum)
> +__thread sigjmp_buf expect_sigbus_jmpbuf;
> +__thread bool expecting_sigbus;
> +
> +static void catchall_signal_handler(int signum)
> {
> + switch (signum) {
> + case SIGBUS: {
> + if (expecting_sigbus)
Transferring/summarizing an internal comment from Sean upstream:
This assumes that tests are indeed using the catchall_signal_handler as
the global signal handler.
In the next revision, I will make TEST_EXPECT_SIGBUS() assert that the
default signal handler is installed, so that developers get a clear,
explicit failure if/when something goes wrong.
> + siglongjmp(expect_sigbus_jmpbuf, 1);
> +
> + TEST_FAIL("Unexpected SIGBUS (%d)\n", signum);
> + }
> #define KVM_CASE_SIGNUM(sig) \
> case sig: TEST_FAIL("Unexpected " #sig " (%d)\n", signum)
> -
> - switch (signum) {
> - KVM_CASE_SIGNUM(SIGBUS);
> KVM_CASE_SIGNUM(SIGSEGV);
> KVM_CASE_SIGNUM(SIGILL);
> KVM_CASE_SIGNUM(SIGFPE);
> @@ -2332,12 +2339,13 @@ static void report_unexpected_signal(int signum)
> void __attribute((constructor)) kvm_selftest_init(void)
> {
> struct sigaction sig_sa = {
> - .sa_handler = report_unexpected_signal,
> + .sa_handler = catchall_signal_handler,
> };
>
> /* Tell stdout not to buffer its content. */
> setbuf(stdout, NULL);
>
> + expecting_sigbus = false;
> sigaction(SIGBUS, &sig_sa, NULL);
> sigaction(SIGSEGV, &sig_sa, NULL);
> sigaction(SIGILL, &sig_sa, NULL);
> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> index 8a1848586a85..03eb99af9b8d 100644
> --- a/tools/testing/selftests/kvm/lib/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -18,13 +18,6 @@
>
> #include "test_util.h"
>
> -sigjmp_buf expect_sigbus_jmpbuf;
> -
> -void __attribute__((used)) expect_sigbus_handler(int signum)
> -{
> - siglongjmp(expect_sigbus_jmpbuf, 1);
> -}
> -
> /*
> * Random number generator that is usable from guest code. This is the
> * Park-Miller LCG using standard constants.
> --
> 2.53.0.rc1.225.gd81095ad13-goog
^ permalink raw reply
* Re: [RFC PATCH v2 09/37] KVM: guest_memfd: Add support for KVM_SET_MEMORY_ATTRIBUTES2
From: Ackerley Tng @ 2026-02-14 20:09 UTC (permalink / raw)
To: kvm, linux-doc, linux-kernel, linux-kselftest, linux-trace-kernel,
x86
Cc: aik, andrew.jones, binbin.wu, bp, brauner, chao.p.peng,
chao.p.peng, chenhuacai, corbet, dave.hansen, david, hpa,
ira.weiny, jgg, jmattson, jroedel, jthoughton, maobibo,
mathieu.desnoyers, maz, mhiramat, michael.roth, mingo, mlevitsk,
oupton, pankaj.gupta, pbonzini, prsampat, qperret, ricarkol,
rick.p.edgecombe, rientjes, rostedt, seanjc, shivankg, shuah,
steven.price, tabba, tglx, vannapurve, vbabka, willy, wyihan,
yan.y.zhao
In-Reply-To: <86ad28b767524e1e654b9c960e39ca8bfb24c114.1770071243.git.ackerleytng@google.com>
Ackerley Tng <ackerleytng@google.com> writes:
>
> [...snip...]
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 23ec0b0c3e22..26e80745c8b4 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -117,7 +117,7 @@ description:
> x86 includes both i386 and x86_64.
>
> Type:
> - system, vm, or vcpu.
> + system, vm, vcpu or guest_memfd.
>
> Parameters:
> what parameters are accepted by the ioctl.
> @@ -6523,11 +6523,22 @@ the capability to be present.
> ---------------------------------
>
> :Capability: KVM_CAP_MEMORY_ATTRIBUTES2
> -:Architectures: x86
> -:Type: vm ioctl
> +:Architectures: all
> +:Type: vm, guest_memfd ioctl
> :Parameters: struct kvm_memory_attributes2 (in/out)
> :Returns: 0 on success, <0 on error
>
> +Errors:
> +
> + ========== ===============================================================
> + EINVAL The specified `offset` or `size` were invalid (e.g. not
> + page aligned, causes an overflow, or size is zero).
> + EFAULT The parameter address was invalid.
> + EAGAIN Some page within requested range had unexpected refcounts. The
> + offset of the page will be returned in `error_offset`.
> + ENOMEM Ran out of memory trying to track private/shared state
> + ========== ===============================================================
> +
> KVM_SET_MEMORY_ATTRIBUTES2 is an extension to
> KVM_SET_MEMORY_ATTRIBUTES that supports returning (writing) values to
> userspace. The original (pre-extension) fields are shared with
> @@ -6538,15 +6549,42 @@ Attribute values are shared with KVM_SET_MEMORY_ATTRIBUTES.
> ::
>
> struct kvm_memory_attributes2 {
> - __u64 address;
> + /* in */
> + union {
> + __u64 address;
> + __u64 offset;
> + };
> __u64 size;
> __u64 attributes;
> __u64 flags;
> - __u64 reserved[12];
> + /* out */
> + __u64 error_offset;
> + __u64 reserved[11];
> };
>
> #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)
>
> +Set attributes for a range of offsets within a guest_memfd to
> +KVM_MEMORY_ATTRIBUTE_PRIVATE to limit the specified guest_memfd backed
> +memory range for guest_use. Even if KVM_CAP_GUEST_MEMFD_MMAP is
> +supported, after a successful call to set
> +KVM_MEMORY_ATTRIBUTE_PRIVATE, the requested range will not be mappable
> +into host userspace and will only be mappable by the guest.
> +
> +To allow the range to be mappable into host userspace again, call
> +KVM_SET_MEMORY_ATTRIBUTES2 on the guest_memfd again with
> +KVM_MEMORY_ATTRIBUTE_PRIVATE unset.
> +
> +If this ioctl returns -EAGAIN, the offset of the page with unexpected
> +refcounts will be returned in `error_offset`. This can occur if there
> +are transient refcounts on the pages, taken by other parts of the
> +kernel.
> +
> +Userspace is expected to figure out how to remove all known refcounts
> +on the shared pages, such as refcounts taken by get_user_pages(), and
> +try the ioctl again. A possible source of these long term refcounts is
> +if the guest_memfd memory was pinned in IOMMU page tables.
> +
> See also: :ref: `KVM_SET_MEMORY_ATTRIBUTES`.
>
Transferring/re-summarizing an internal comment from Sean upstream here!
We can also follow up on this topic at the next guest_memfd biweekly.
Before this lands, Sean wants, at the very minimum, an in-principle
agreement on guest_memfd behavior with respect to whether or not memory
should be preserved on conversion.
Sean is against deferring whether to preserve memory to the underlying
hardware because that is letting (effectively) micro-architectural
behavior to define KVM's ABI. KVM's uAPI cannot let behavior be
undefined, or be based on vendor, and maybe even on firmware version.
Sean says that all decisions that affect guest data must be made by
userspace. The architecture can restrict what is possible, e.g. neither
SNP nor TDX currently support "generic" in-place conversion, but whether
or not data is to be preserved must be an explicit request from
userspace. If preserving data is impossible, then KVM needs to reject
the request.
(Vendor specific ioctls are out-of-scope, SNP and TDX cases were brought
up purely to highlight that there's nothing that fundamentally prevents
preserving data on conversion.)
I suggested a few uAPI options for configuring content preservation on
conversion:
1. guest_memfd creation time flag like
GUEST_MEMFD_FLAG_PRESERVE_CONTENTS. This can be valid only if the
kernel and vendor support content preservation
This was rejected because we should not assume all current and future
use cases will want the same content preservation config for a given
guest_memfd.
2. KConfig: automatically select to preserve contents if the
architecture supports content preservation
This was rejected because it's not a decision explicitly made by
userspace.
3. KVM module param to configure content preservation.
This was rejected because the configuration may not generalize across
all VMs on the same host.
4. guest_memfd ioctl flag
SET_MEMORY_ATTRIBUTES2_FLAG_PRESERVE_CONTENTS. -EINVAL if kernel and
vendor don't support content preservation
Specifying a flag to choose whether content should be preserved at
conversion-time is the current best suggestion.
What does the rest of the community think of a conversion ioctl flag to
choose whether to preserve memory contents on conversion?
Fuad, I think you also made a related comment on an earlier internal
version we were working on. What do you/pKVM think?
>
> [...snip...]
>
^ permalink raw reply
* [PATCH v1] Documentation/rtla: Add hwnoise to main page
From: Costa Shulyupin @ 2026-02-15 13:12 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar, Jonathan Corbet, linux-trace-kernel,
linux-kernel, linux-doc
Cc: Costa Shulyupin
Add hwnoise to the command list and SEE ALSO section. The command list
is ordered from low level to high level.
Signed-off-by: Costa Shulyupin <costa.shul@redhat.com>
---
Documentation/tools/rtla/rtla.rst | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/Documentation/tools/rtla/rtla.rst b/Documentation/tools/rtla/rtla.rst
index 2a5fb7004ad4..6df1296b8cc1 100644
--- a/Documentation/tools/rtla/rtla.rst
+++ b/Documentation/tools/rtla/rtla.rst
@@ -21,6 +21,10 @@ results.
COMMANDS
========
+**hwnoise**
+
+ Detect and quantify hardware-related noise.
+
**osnoise**
Gives information about the operating system noise (osnoise).
@@ -39,7 +43,7 @@ For other options, see the man page for the corresponding command.
SEE ALSO
========
-**rtla-osnoise**\(1), **rtla-timerlat**\(1)
+**rtla-hwnoise**\(1), **rtla-osnoise**\(1), **rtla-timerlat**\(1)
AUTHOR
======
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2] blk-mq: use NOIO context to prevent deadlock during debugfs creation
From: Nilay Shroff @ 2026-02-15 13:34 UTC (permalink / raw)
To: Yu Kuai, axboe, ming.lei, hch
Cc: yi.zhang, shinichiro.kawasaki, kbusch, rostedt, mhiramat,
mathieu.desnoyers, linux-block, linux-kernel, linux-trace-kernel
In-Reply-To: <20260214054350.2322436-1-yukuai@fnnas.com>
On 2/14/26 11:13 AM, Yu Kuai wrote:
> Creating debugfs entries can trigger fs reclaim, which can enter back
> into the block layer request_queue. This can cause deadlock if the
> queue is frozen.
>
> Previously, a WARN_ON_ONCE check was used in debugfs_create_files()
> to detect this condition, but it was racy since the queue can be frozen
> from another context at any time.
>
> Introduce blk_debugfs_lock()/blk_debugfs_unlock() helpers that combine
> the debugfs_mutex with memalloc_noio_save()/restore() to prevent fs
> reclaim from triggering block I/O. Also add blk_debugfs_lock_nomemsave()
> and blk_debugfs_unlock_nomemrestore() variants for callers that don't
> need NOIO protection (e.g., debugfs removal or read-only operations).
>
> Replace all raw debugfs_mutex lock/unlock pairs with these helpers,
> using the _nomemsave/_nomemrestore variants where appropriate.
>
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Closes: https://lore.kernel.org/all/CAHj4cs9gNKEYAPagD9JADfO5UH+OiCr4P7OO2wjpfOYeM-RV=A@mail.gmail.com/
> Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> Closes: https://lore.kernel.org/all/aYWQR7CtYdk3K39g@shinmob/
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Yu Kuai <yukuai@fnnas.com>
Looks good to me:
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
^ permalink raw reply
* Re: [syzbot] [block?] [trace?] INFO: task hung in blk_trace_startstop
From: syzbot @ 2026-02-15 14:17 UTC (permalink / raw)
To: axboe, linux-block, linux-kernel, linux-trace-kernel,
mathieu.desnoyers, mhiramat, rostedt, syzkaller-bugs
In-Reply-To: <691367ae.a70a0220.22f260.0141.GAE@google.com>
syzbot has found a reproducer for the following issue on:
HEAD commit: 635c467cc14e Add linux-next specific files for 20260213
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=14d8ec02580000
kernel config: https://syzkaller.appspot.com/x/.config?x=f09eec269f9f4746
dashboard link: https://syzkaller.appspot.com/bug?extid=774863666ef5b025c9d0
compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12cc2ffa580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/24870d51cbd0/disk-635c467c.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/49283c738806/vmlinux-635c467c.xz
kernel image: https://storage.googleapis.com/syzbot-assets/8c5f9d1da977/bzImage-635c467c.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/b01c09978893/mount_0.gz
fsck result: OK (log: https://syzkaller.appspot.com/x/fsck.log?x=163e915a580000)
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+774863666ef5b025c9d0@syzkaller.appspotmail.com
INFO: task syz.3.20:6102 blocked for more than 143 seconds.
Not tainted syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz.3.20 state:D stack:28368 pid:6102 tgid:6087 ppid:5957 task_flags:0x400040 flags:0x00080002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5295 [inline]
__schedule+0x1585/0x5340 kernel/sched/core.c:6907
__schedule_loop kernel/sched/core.c:6989 [inline]
schedule+0x164/0x360 kernel/sched/core.c:7004
schedule_preempt_disabled+0x13/0x30 kernel/sched/core.c:7061
__mutex_lock_common kernel/locking/mutex.c:692 [inline]
__mutex_lock+0x7fe/0x1300 kernel/locking/mutex.c:776
blk_trace_startstop+0x8f/0x610 kernel/trace/blktrace.c:901
blk_trace_ioctl+0x315/0x740 kernel/trace/blktrace.c:947
blkdev_common_ioctl+0x13a7/0x3250 block/ioctl.c:724
blkdev_ioctl+0x528/0x740 block/ioctl.c:798
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:597 [inline]
__se_sys_ioctl+0xfc/0x170 fs/ioctl.c:583
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x14d/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f530c39bf79
RSP: 002b:00007f530d2e4028 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007f530c616180 RCX: 00007f530c39bf79
RDX: 0000000000000000 RSI: 0000000000001275 RDI: 0000000000000007
RBP: 00007f530c4327e0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f530c616218 R14: 00007f530c616180 R15: 00007ffdd027c568
</TASK>
INFO: task syz.2.19:6109 blocked for more than 150 seconds.
Not tainted syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz.2.19 state:D stack:26720 pid:6109 tgid:6091 ppid:5955 task_flags:0x400140 flags:0x00080002
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5295 [inline]
__schedule+0x1585/0x5340 kernel/sched/core.c:6907
__schedule_loop kernel/sched/core.c:6989 [inline]
schedule+0x164/0x360 kernel/sched/core.c:7004
schedule_preempt_disabled+0x13/0x30 kernel/sched/core.c:7061
__mutex_lock_common kernel/locking/mutex.c:692 [inline]
__mutex_lock+0x7fe/0x1300 kernel/locking/mutex.c:776
relay_open+0x3b8/0x920 kernel/relay.c:516
blk_trace_setup_prepare+0x425/0x5a0 kernel/trace/blktrace.c:716
blk_trace_setup+0x2c0/0x430 kernel/trace/blktrace.c:789
blk_trace_ioctl+0x380/0x740 kernel/trace/blktrace.c:935
---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
^ permalink raw reply
* [PATCH AUTOSEL 6.19-5.15] tracing: Fix false sharing in hwlat get_sample()
From: Sasha Levin @ 2026-02-15 15:03 UTC (permalink / raw)
To: patches, stable
Cc: Colin Lord, Masami Hiramatsu, Mathieu Desnoyers,
Steven Rostedt (Google), Sasha Levin, linux-kernel,
linux-trace-kernel
In-Reply-To: <20260215150333.2150455-1-sashal@kernel.org>
From: Colin Lord <clord@mykolab.com>
[ Upstream commit f743435f988cb0cf1f521035aee857851b25e06d ]
The get_sample() function in the hwlat tracer assumes the caller holds
hwlat_data.lock, but this is not actually happening. The result is
unprotected data access to hwlat_data, and in per-cpu mode can result in
false sharing which may show up as false positive latency events.
The specific case of false sharing observed was primarily between
hwlat_data.sample_width and hwlat_data.count. These are separated by
just 8B and are therefore likely to share a cache line. When one thread
modifies count, the cache line is in a modified state so when other
threads read sample_width in the main latency detection loop, they fetch
the modified cache line. On some systems, the fetch itself may be slow
enough to count as a latency event, which could set up a self
reinforcing cycle of latency events as each event increments count which
then causes more latency events, continuing the cycle.
The other result of the unprotected data access is that hwlat_data.count
can end up with duplicate or missed values, which was observed on some
systems in testing.
Convert hwlat_data.count to atomic64_t so it can be safely modified
without locking, and prevent false sharing by pulling sample_width into
a local variable.
One system this was tested on was a dual socket server with 32 CPUs on
each numa node. With settings of 1us threshold, 1000us width, and
2000us window, this change reduced the number of latency events from
500 per second down to approximately 1 event per minute. Some machines
tested did not exhibit measurable latency from the false sharing.
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://patch.msgid.link/20260210074810.6328-1-clord@mykolab.com
Signed-off-by: Colin Lord <clord@mykolab.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Analysis of the Commit
### 1. Commit Message Analysis
The commit message is very detailed and clearly describes:
- **The bug**: `get_sample()` assumes `hwlat_data.lock` is held, but
it's not actually held. This leads to unprotected data access and
false sharing in per-cpu mode.
- **The specific false sharing**: `hwlat_data.sample_width` and
`hwlat_data.count` are 8 bytes apart, likely sharing a cache line.
When one thread modifies `count`, other threads reading `sample_width`
in the latency detection loop fetch the modified cache line, causing
measurable latency.
- **The self-reinforcing cycle**: The latency from cache line fetch
triggers a latency event, which increments `count`, which causes more
cache line invalidation, which causes more latency events — a vicious
cycle.
- **The data race**: `hwlat_data.count` can end up with duplicate or
missed values due to unprotected concurrent access.
- **Concrete test results**: On a dual-socket 32-CPUs-per-node server,
latency events dropped from 500/sec to ~1/min.
Keywords: "false sharing", "false positive latency events", "unprotected
data access", "duplicate or missed values."
### 2. Code Change Analysis
The changes are minimal and surgical:
1. **`hwlat_data.count` converted from `u64` to `atomic64_t`**: This
fixes a real data race where multiple threads could concurrently
increment `count` without synchronization, leading to lost updates
(duplicate/missed values). The `atomic64_inc_return()` replaces
`hwlat_data.count++; s.seqnum = hwlat_data.count;` with a single
atomic operation.
2. **`sample_width` pulled into a local variable with `READ_ONCE()`**:
`u64 sample_width = READ_ONCE(hwlat_data.sample_width);` is used
instead of reading `hwlat_data.sample_width` in the hot loop (`do {
... } while (total <= sample_width)`). This:
- Prevents the CPU from repeatedly fetching a cache line that may be
bouncing between cores
- Eliminates the false sharing between `sample_width` and `count`
- Uses `READ_ONCE()` for proper load semantics
3. **Init path updated**: `hwlat_data.count = 0` →
`atomic64_set(&hwlat_data.count, 0)` — consistent with the type
change.
4. **Comment fix**: Removes the incorrect claim that `get_sample()` is
"called with hwlat_data.lock held."
### 3. Bug Classification
This commit fixes **multiple real bugs**:
1. **Data race on `hwlat_data.count`**: Concurrent unsynchronized access
to a shared counter. This is a real correctness bug — sequence
numbers can be duplicated or skipped.
2. **False sharing causing false positive latency detection**: The hwlat
tracer is producing bogus results (500 events/sec vs 1/min). This is
a **functional correctness bug** — the tracer is reporting phantom
hardware latency that doesn't exist. Users relying on hwlat tracer
results would be misled.
3. **Self-reinforcing feedback loop**: The false sharing creates a
pathological cycle that makes the tracer practically unusable on some
multi-socket systems.
### 4. Scope and Risk Assessment
- **Lines changed**: ~15 lines of actual code changes across 4 hunks in
a single file
- **Files touched**: 1 (`kernel/trace/trace_hwlat.c`)
- **Risk**: Very low. The changes are:
- Converting a counter to atomic (well-understood primitive)
- Caching a value in a local variable (safe, the value doesn't need to
be re-read)
- Using `READ_ONCE()` (standard kernel pattern)
- **Subsystem**: Tracing — self-contained, well-maintained by Steven
Rostedt
- **Could it break something?**: Extremely unlikely. The atomic64
operations are well-tested primitives, and caching sample_width is
semantically equivalent (the width doesn't change during a sample).
### 5. User Impact
- **Who is affected**: Anyone using the hwlat tracer in per-cpu mode on
multi-socket systems
- **Severity**: The tracer produces wildly incorrect results (500x false
positives) on affected systems
- **Real-world impact**: Users trying to validate hardware latency
characteristics for real-time workloads would get completely
misleading data
- **The data race on count**: Could cause sequence number issues that
affect trace analysis tooling
### 6. Stability Indicators
- **Signed-off by Steven Rostedt** (tracing subsystem maintainer) — high
confidence
- **Concrete test data** provided showing dramatic improvement
- **Link to mailing list** discussion provided
- The fix uses standard kernel primitives (atomic64_t, READ_ONCE) —
well-understood patterns
### 7. Dependency Check
- No dependencies on other commits
- `atomic64_t` and `READ_ONCE()` are available in all stable kernel
versions
- The hwlat tracer exists in all recent stable trees (introduced in
v4.9)
### Stable Kernel Rules Assessment
1. **Obviously correct and tested**: Yes — tested on real hardware with
measurable improvement
2. **Fixes a real bug**: Yes — data race + false sharing causing
incorrect tracer output
3. **Important issue**: Yes — tracer producing 500x false positive
latency events, plus data race on counter
4. **Small and contained**: Yes — ~15 lines in one file
5. **No new features**: Correct — this is a pure bug fix
6. **Applies cleanly**: Should apply cleanly to recent stable trees
### Risk vs Benefit
- **Risk**: Near zero — atomic counter and local variable caching are
trivially safe changes
- **Benefit**: High — fixes a data race and makes the hwlat tracer
produce correct results on multi-socket systems
**YES**
kernel/trace/trace_hwlat.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index 2f7b94e98317c..3fe274b84f1c2 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -102,9 +102,9 @@ struct hwlat_sample {
/* keep the global state somewhere. */
static struct hwlat_data {
- struct mutex lock; /* protect changes */
+ struct mutex lock; /* protect changes */
- u64 count; /* total since reset */
+ atomic64_t count; /* total since reset */
u64 sample_window; /* total sampling window (on+off) */
u64 sample_width; /* active sampling portion of window */
@@ -193,8 +193,7 @@ void trace_hwlat_callback(bool enter)
* get_sample - sample the CPU TSC and look for likely hardware latencies
*
* Used to repeatedly capture the CPU TSC (or similar), looking for potential
- * hardware-induced latency. Called with interrupts disabled and with
- * hwlat_data.lock held.
+ * hardware-induced latency. Called with interrupts disabled.
*/
static int get_sample(void)
{
@@ -204,6 +203,7 @@ static int get_sample(void)
time_type start, t1, t2, last_t2;
s64 diff, outer_diff, total, last_total = 0;
u64 sample = 0;
+ u64 sample_width = READ_ONCE(hwlat_data.sample_width);
u64 thresh = tracing_thresh;
u64 outer_sample = 0;
int ret = -1;
@@ -267,7 +267,7 @@ static int get_sample(void)
if (diff > sample)
sample = diff; /* only want highest value */
- } while (total <= hwlat_data.sample_width);
+ } while (total <= sample_width);
barrier(); /* finish the above in the view for NMIs */
trace_hwlat_callback_enabled = false;
@@ -285,8 +285,7 @@ static int get_sample(void)
if (kdata->nmi_total_ts)
do_div(kdata->nmi_total_ts, NSEC_PER_USEC);
- hwlat_data.count++;
- s.seqnum = hwlat_data.count;
+ s.seqnum = atomic64_inc_return(&hwlat_data.count);
s.duration = sample;
s.outer_duration = outer_sample;
s.nmi_total_ts = kdata->nmi_total_ts;
@@ -832,7 +831,7 @@ static int hwlat_tracer_init(struct trace_array *tr)
hwlat_trace = tr;
- hwlat_data.count = 0;
+ atomic64_set(&hwlat_data.count, 0);
tr->max_latency = 0;
save_tracing_thresh = tracing_thresh;
--
2.51.0
^ permalink raw reply related
* [PATCH] pstore: fix ftrace dump, when ECC is enabled
From: Andrey Skvortsov @ 2026-02-15 18:51 UTC (permalink / raw)
To: Kees Cook, Tony Luck, Guilherme G. Piccoli, linux-hardening,
linux-kernel, Steven Rostedt, Masami Hiramatsu, Mark Rutland,
linux-trace-kernel
Cc: Andrey Skvortsov
total_size is sum of record->size and record->ecc_notice_size (ECC: No
errors detected). When ECC is not used, then there is no problem.
When ECC is enabled, then ftrace dump is decoded incorrectly after
restart.
First this affects starting offset calculation, that breaks
reading of all ftrace records.
CPU:66 ts:51646260179894273 3818ffff80008002 fe00ffff800080f0 0x3818ffff80008002 <- 0xfe00ffff800080f0
CPU:66 ts:56589664458375169 3818ffff80008002 ff02ffff800080f0 0x3818ffff80008002 <- 0xff02ffff800080f0
CPU:67 ts:13194139533313 afe4ffff80008002 1ffff800080f0 0xafe4ffff80008002 <- 0x1ffff800080f0
CPU:67 ts:13194139533313 b7d0ffff80008001 100ffff80008002 0xb7d0ffff80008001 <- 0x100ffff80008002
CPU:67 ts:51646260179894273 8de0ffff80008001 202ffff80008002 0x8de0ffff80008001 <- 0x202ffff80008002
Second ECC notice message is printed like ftrace record and as a
result couple of last records are completely wrong.
For example, when the starting offset is fixed:
CPU:0 ts:113 ffffffc00879bd04 ffffffc0080dc08c cpuidle_enter <- do_idle+0x20c/0x290
CPU:0 ts:114 ffffffc00879bd04 ffffffc0080dc08c cpuidle_enter <- do_idle+0x20c/0x290
CPU:100 ts:28259048229270629 6f4e203a4343450a 2073726f72726520 0x6f4e203a4343450a <- 0x2073726f72726520
Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
---
fs/pstore/inode.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index b4e55c90f8dc2..9c2d5e5f31d65 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -74,9 +74,9 @@ static void *pstore_ftrace_seq_start(struct seq_file *s, loff_t *pos)
if (!data)
return NULL;
- data->off = ps->total_size % REC_SIZE;
+ data->off = ps->record->size % REC_SIZE;
data->off += *pos * REC_SIZE;
- if (data->off + REC_SIZE > ps->total_size)
+ if (data->off + REC_SIZE > ps->record->size)
return NULL;
return_ptr(data);
@@ -94,7 +94,7 @@ static void *pstore_ftrace_seq_next(struct seq_file *s, void *v, loff_t *pos)
(*pos)++;
data->off += REC_SIZE;
- if (data->off + REC_SIZE > ps->total_size)
+ if (data->off + REC_SIZE > ps->record->size)
return NULL;
return data;
--
2.51.0
^ permalink raw reply related
* [linus:master] [tracing] a46023d561: EIP:do_user_addr_fault
From: kernel test robot @ 2026-02-16 2:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: oe-lkp, lkp, linux-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Sebastian Andrzej Siewior,
Alexei Starovoitov, Paul E. McKenney, linux-trace-kernel,
oliver.sang
Hello,
kernel test robot noticed "EIP:do_user_addr_fault" on:
commit: a46023d5616ed3ed781e56ca93400eb9490e3646 ("tracing: Guard __DECLARE_TRACE() use of __DO_TRACE_CALL() with SRCU-fast")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
[test failed on linus/master ca4ee40bf13dbd3a4be3b40a00c33a1153d487e5]
[test failed on linux-next/master 635c467cc14ebdffab3f77610217c1dacaf88e8c]
in testcase: boot
config: i386-randconfig-001-20260215
compiler: clang-20
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 32G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+---------------------------------------------+------------+------------+
| | a77cb6a867 | a46023d561 |
+---------------------------------------------+------------+------------+
| EIP:do_user_addr_fault | 0 | 18 |
| EIP:do_int80_syscall_32 | 0 | 18 |
| BUG:kernel_NULL_pointer_dereference,address | 0 | 15 |
| Oops | 0 | 18 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 18 |
| BUG:unable_to_handle_page_fault_for_address | 0 | 3 |
+---------------------------------------------+------------+------------+
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202602160941.c62658c7-lkp@intel.com
[ 3.833434][ T59] ------------[ cut here ]------------
[ 3.834088][ T59] WARNING: arch/x86/mm/fault.c:1274 at do_user_addr_fault+0x387/0x480, CPU#1: modprobe/59
[ 3.835108][ T59] Modules linked in:
[ 3.835140][ T59] CPU: 1 UID: 0 PID: 59 Comm: modprobe Tainted: G T 6.19.0-rc7-00020-ga46023d5616e #1 PREEMPT(lazy)
[ 3.835140][ T59] Tainted: [T]=RANDSTRUCT
[ 3.835140][ T59] EIP: do_user_addr_fault (ld-temp.o:?)
[ 3.835140][ T59] Code: ff ff 89 f9 89 da ff 75 f0 e8 15 01 00 00 83 c4 04 e9 03 fe ff ff 89 f9 89 da ff 75 f0 e8 01 49 f6 ff 83 c4 04 e9 f7 fc ff ff <0f> 0b 89 f9 89 da ff 75 f0 e8 eb 03 00 00 83 c4 04 e9 d9 fd ff ff
All code
========
0: ff (bad)
1: ff 89 f9 89 da ff decl -0x257607(%rcx)
7: 75 f0 jne 0xfffffffffffffff9
9: e8 15 01 00 00 call 0x123
e: 83 c4 04 add $0x4,%esp
11: e9 03 fe ff ff jmp 0xfffffffffffffe19
16: 89 f9 mov %edi,%ecx
18: 89 da mov %ebx,%edx
1a: ff 75 f0 push -0x10(%rbp)
1d: e8 01 49 f6 ff call 0xfffffffffff64923
22: 83 c4 04 add $0x4,%esp
25: e9 f7 fc ff ff jmp 0xfffffffffffffd21
2a:* 0f 0b ud2 <-- trapping instruction
2c: 89 f9 mov %edi,%ecx
2e: 89 da mov %ebx,%edx
30: ff 75 f0 push -0x10(%rbp)
33: e8 eb 03 00 00 call 0x423
38: 83 c4 04 add $0x4,%esp
3b: e9 d9 fd ff ff jmp 0xfffffffffffffe19
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: 89 f9 mov %edi,%ecx
4: 89 da mov %ebx,%edx
6: ff 75 f0 push -0x10(%rbp)
9: e8 eb 03 00 00 call 0x3f9
e: 83 c4 04 add $0x4,%esp
11: e9 d9 fd ff ff jmp 0xfffffffffffffdef
[ 3.835140][ T59] EAX: 80000000 EBX: 00000000 ECX: 4324aef7 EDX: 431f7ada
[ 3.835140][ T59] ESI: 52cb2000 EDI: 52cb5f34 EBP: 52cb5f14 ESP: 52cb5ef0
[ 3.835140][ T59] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00210046
[ 3.835140][ T59] CR0: 80050033 CR2: 00000004 CR3: 1275e000 CR4: 000406b0
[ 3.835140][ T59] Call Trace:
[ 3.835140][ T59] ? debug_smp_processor_id (ld-temp.o:?)
[ 3.835140][ T59] ? trace_page_fault_kernel (ld-temp.o:?)
[ 3.835140][ T59] ? exc_page_fault (ld-temp.o:?)
[ 3.835140][ T59] ? pvclock_clocksource_read_nowd (ld-temp.o:?)
[ 3.835140][ T59] ? entry_INT80_32 (arch/x86/entry/entry_32.S:945)
[ 3.835140][ T59] ? handle_exception (arch/x86/entry/entry_32.S:1048)
[ 3.835140][ T59] ? entry_INT80_32 (arch/x86/entry/entry_32.S:945)
[ 3.835140][ T59] ? xas_find_conflict (ld-temp.o:?)
[ 3.835140][ T59] ? pvclock_clocksource_read_nowd (ld-temp.o:?)
[ 3.835140][ T59] ? do_int80_syscall_32 (ld-temp.o:?)
[ 3.835140][ T59] ? pvclock_clocksource_read_nowd (ld-temp.o:?)
[ 3.835140][ T59] ? do_int80_syscall_32 (ld-temp.o:?)
[ 3.835140][ T59] ? entry_INT80_32 (arch/x86/entry/entry_32.S:945)
[ 3.835140][ T59] irq event stamp: 5220
[ 3.835140][ T59] hardirqs last enabled at (5219): free_to_partial_list (ld-temp.o:?)
[ 3.835140][ T59] hardirqs last disabled at (5220): do_int80_syscall_32 (ld-temp.o:?)
[ 3.835140][ T59] softirqs last enabled at (2722): __do_softirq (ld-temp.o:?)
[ 3.835140][ T59] softirqs last disabled at (2713): __do_softirq (ld-temp.o:?)
[ 3.835140][ T59] ---[ end trace 0000000000000000 ]---
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260216/202602160941.c62658c7-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: Enhancing Conditional Filtering for Function Graph Tracer
From: Masami Hiramatsu @ 2026-02-16 7:27 UTC (permalink / raw)
To: Donglin Peng
Cc: Steven Rostedt, linux-trace-kernel, Linux Kernel Mailing List
In-Reply-To: <CAErzpmtMrHWQVAQx=jiq_sWo=+c_JOUoQmPafo-fq+Q3qi+BBw@mail.gmail.com>
On Sat, 14 Feb 2026 18:36:24 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> Hi Steve and Masami,
>
> I recently had an idea while using the function graph tracer.
> Currently, it supports basic
> filtering via set_ftrace_pid(PID-based) and
> set_graph_function(function name-based).
> However, this filtering mechanism feels somewhat limited. Given that
> we already have
> features like funcgraph-args, I wonder if we could enhance filtering by allowing
> conditional tracking based on function parameters — similar to how
> trace events support
> filters/triggers.
Agreed. I think it is a good idea.
>
> To simplify implementation, I propose extending a new trigger type
> (e.g., "funcgraph").
OK, and I think the question is what do we want to filter. As you know,
the kernel function call-graph is a very context depending. I think we
may be better to start with specifying the pid filter via the tringger
instead of enabling/disabling it on all cpus.
echo "funcgraph:setpid:common_pid" >> events/fprobes/func_foo/trigger
Then, we can also specify a specific task's pid too via sched_switch
events etc. Also we can add "clearpid". Maybe we also need a special
PID (e.g. -1) for trace no process instead of using tracing_on.
Thank you,
> In ftrace_graph_ignore_func, we could look up the corresponding trace_fprobe and
> trace_event_file based on trace->func, then decide whether to trace
> the function using
> a helper like the following:
>
> static bool ftrace_graph_filter(struct trace_fprobe *tf, struct
> ftrace_regs *fregs,
> struct trace_event_file *trace_file)
> {
> struct fentry_trace_entry_head *entry;
> struct trace_event_buffer fbuffer;
> struct event_trigger_data *data;
> int dsize;
>
> dsize = __get_data_size(&tf->tp, fregs, NULL);
> entry = trace_event_buffer_reserve(&fbuffer, trace_file,
> sizeof(*entry) + tf->tp.size + dsize);
> if (!entry)
> return false;
>
> entry = ring_buffer_event_data(fbuffer.event);
> store_trace_args(&entry[1], &tf->tp, fregs, NULL, sizeof(*entry), dsize);
>
> list_for_each_entry_rcu(data, &trace_file->triggers, list) {
> if (data->cmd_ops->trigger_type == TRIGGER_TYPE_FUNCGRAPH) {
> struct event_filter *filter = rcu_dereference_sched(data->filter);
> if (filter && filter_match_preds(filter, entry))
> return true; // Allow tracing
> }
> }
> return false; // Skip tracing
> }
>
> Does this approach make sense? Any suggestions or concerns?
>
> Thanks,
> Donglin
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Mikhail Gavrilov @ 2026-02-16 7:47 UTC (permalink / raw)
To: Gabriele Monaco, Daniel Bristot de Oliveira, Steven Rostedt
Cc: Nam Cao, linux-trace-kernel, linux-kernel, Mikhail Gavrilov
The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
minimise macros") replaced per-monitor unique variable names
(da_mon_##name) with a fixed name (da_mon_this).
While this works for 'static' variables (each translation unit gets its
own copy), DEFINE_PER_CPU internally generates a non-static dummy
variable __pcpu_unique_<n> for each per-cpu definition. When multiple
per-cpu monitors (e.g. sco and sts) are built-in simultaneously, they
all produce the same __pcpu_unique_da_mon_this symbol, causing a link
error:
ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
`__pcpu_unique_da_mon_this';
kernel/trace/rv/monitors/sco/sco.o: first defined here
Fix this by introducing a DA_MON_NAME macro that expands to a
per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
CONCATENATE helper. This restores the uniqueness that was present
before the refactoring.
Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
include/rv/da_monitor.h | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index db11d41bb438..f6da07863ed8 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -19,6 +19,7 @@
#include <linux/stringify.h>
#include <linux/bug.h>
#include <linux/sched.h>
+#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
static struct rv_monitor rv_this;
@@ -183,14 +184,14 @@ static inline bool da_event(struct da_monitor *da_mon, struct task_struct *tsk,
/*
* global monitor (a single variable)
*/
-static struct da_monitor da_mon_this;
+static struct da_monitor DA_MON_NAME;
/*
* da_get_monitor - return the global monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return &da_mon_this;
+ return &DA_MON_NAME;
}
/*
@@ -223,14 +224,14 @@ static inline void da_monitor_destroy(void) { }
/*
* per-cpu monitor variables
*/
-static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
+static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
/*
* da_get_monitor - return current CPU monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return this_cpu_ptr(&da_mon_this);
+ return this_cpu_ptr(&DA_MON_NAME);
}
/*
@@ -242,7 +243,7 @@ static void da_monitor_reset_all(void)
int cpu;
for_each_cpu(cpu, cpu_online_mask) {
- da_mon = per_cpu_ptr(&da_mon_this, cpu);
+ da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
da_monitor_reset(da_mon);
}
}
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] ring-buffer: Fix possible dereference of uninitialized pointer
From: Masami Hiramatsu @ 2026-02-16 8:00 UTC (permalink / raw)
To: Daniil Dulov
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
open list:TRACING, open list:TRACING, lvc-project,
kernel test robot, Dan Carpenter
In-Reply-To: <20260213100130.2013839-1-d.dulov@aladdin.ru>
On Fri, 13 Feb 2026 13:01:30 +0300
Daniil Dulov <d.dulov@aladdin.ru> wrote:
> There is a pointer head_page in rb_meta_validate_events() which is not
> initialized at the beginning of a function. This pointer can be dereferenced
> if there is a failure during reader page validation. In this case the control
> is passed to "invalid" label where the pointer is dereferenced in a loop.
>
> To fix the issue initialize orig_head and head_page before calling
> rb_validate_buffer.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202406130130.JtTGRf7W-lkp@intel.com/
> Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
> Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
> ---
> kernel/trace/ring_buffer.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 630221b00838..ad08430347b0 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1918,6 +1918,8 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> if (!meta || !meta->head_buffer)
> return;
>
> + orig_head = head_page = cpu_buffer->head_page;
> +
> /* Do the reader page first */
> ret = rb_validate_buffer(cpu_buffer->reader_page->page, cpu_buffer->cpu);
> if (ret < 0) {
> @@ -1928,7 +1930,6 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> entry_bytes += local_read(&cpu_buffer->reader_page->page->commit);
> local_set(&cpu_buffer->reader_page->entries, ret);
>
> - orig_head = head_page = cpu_buffer->head_page;
> ts = head_page->page->time_stamp;
>
> /*
> --
> 2.34.1
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Gabriele Monaco @ 2026-02-16 8:28 UTC (permalink / raw)
To: Mikhail Gavrilov
Cc: Steven Rostedt, Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <20260216074727.741822-1-mikhail.v.gavrilov@gmail.com>
On Mon, 2026-02-16 at 12:47 +0500, Mikhail Gavrilov wrote:
> The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
> minimise macros") replaced per-monitor unique variable names
> (da_mon_##name) with a fixed name (da_mon_this).
>
> While this works for 'static' variables (each translation unit gets its
> own copy), DEFINE_PER_CPU internally generates a non-static dummy
> variable __pcpu_unique_<n> for each per-cpu definition. When multiple
> per-cpu monitors (e.g. sco and sts) are built-in simultaneously, they
> all produce the same __pcpu_unique_da_mon_this symbol, causing a link
> error:
>
> ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
> `__pcpu_unique_da_mon_this';
> kernel/trace/rv/monitors/sco/sco.o: first defined here
>
> Fix this by introducing a DA_MON_NAME macro that expands to a
> per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
> CONCATENATE helper. This restores the uniqueness that was present
> before the refactoring.
Thanks Mikhail,
I just got a report but you were faster sending the patch!
The __pcpu_unique_ variable (and with it the requirement for per-cpu variables
to be unique although static) exists:
* for modules on specific achitectures (alpha)
* if the kernel is built with CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's
debug kernel)
Could you mention this in the commit message?
Also, it's alright using a unique name also for global monitors although not
required, but please add a comment stating why this is needed at least for CPU
variables, to avoid the next "refactoring" to change it again. Something like:
Per-cpu variables requires a unique name although static in some configurations
Other than that, the change looks good.
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Thanks,
Gabriele
>
> Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---
> include/rv/da_monitor.h | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index db11d41bb438..f6da07863ed8 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -19,6 +19,7 @@
> #include <linux/stringify.h>
> #include <linux/bug.h>
> #include <linux/sched.h>
> +#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
>
> static struct rv_monitor rv_this;
>
> @@ -183,14 +184,14 @@ static inline bool da_event(struct da_monitor *da_mon,
> struct task_struct *tsk,
> /*
> * global monitor (a single variable)
> */
> -static struct da_monitor da_mon_this;
> +static struct da_monitor DA_MON_NAME;
>
> /*
> * da_get_monitor - return the global monitor address
> */
> static struct da_monitor *da_get_monitor(void)
> {
> - return &da_mon_this;
> + return &DA_MON_NAME;
> }
>
> /*
> @@ -223,14 +224,14 @@ static inline void da_monitor_destroy(void) { }
> /*
> * per-cpu monitor variables
> */
> -static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
> +static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
>
> /*
> * da_get_monitor - return current CPU monitor address
> */
> static struct da_monitor *da_get_monitor(void)
> {
> - return this_cpu_ptr(&da_mon_this);
> + return this_cpu_ptr(&DA_MON_NAME);
> }
>
> /*
> @@ -242,7 +243,7 @@ static void da_monitor_reset_all(void)
> int cpu;
>
> for_each_cpu(cpu, cpu_online_mask) {
> - da_mon = per_cpu_ptr(&da_mon_this, cpu);
> + da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
> da_monitor_reset(da_mon);
> }
> }
^ permalink raw reply
* [PATCH v2] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Mikhail Gavrilov @ 2026-02-16 9:01 UTC (permalink / raw)
To: Gabriele Monaco, Daniel Bristot de Oliveira, Steven Rostedt
Cc: Nam Cao, linux-trace-kernel, linux-kernel, Mikhail Gavrilov
In-Reply-To: <628790bb5ee45c8968550a54388862f020d014ff.camel@redhat.com>
The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
minimise macros") replaced per-monitor unique variable names
(da_mon_##name) with a fixed name (da_mon_this).
While this works for 'static' variables (each translation unit gets its
own copy), DEFINE_PER_CPU internally generates a non-static dummy
variable __pcpu_unique_<n> for each per-cpu definition. The requirement
for this variable to be unique although static exists for modules on
specific architectures (alpha) and if the kernel is built with
CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's debug kernel).
When multiple per-cpu monitors (e.g. sco and sts) are built-in
simultaneously, they all produce the same __pcpu_unique_da_mon_this
symbol, causing a link error:
ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
`__pcpu_unique_da_mon_this';
kernel/trace/rv/monitors/sco/sco.o: first defined here
Fix this by introducing a DA_MON_NAME macro that expands to a
per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
CONCATENATE helper. This restores the uniqueness that was present
before the refactoring.
Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
include/rv/da_monitor.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
index db11d41bb438..7511f5464c48 100644
--- a/include/rv/da_monitor.h
+++ b/include/rv/da_monitor.h
@@ -20,6 +20,12 @@
#include <linux/bug.h>
#include <linux/sched.h>
+/*
+ * Per-cpu variables require a unique name although static in some
+ * configurations (e.g. CONFIG_DEBUG_FORCE_WEAK_PER_CPU or alpha modules).
+ */
+#define DA_MON_NAME CONCATENATE(da_mon_, MONITOR_NAME)
+
static struct rv_monitor rv_this;
static void react(enum states curr_state, enum events event)
@@ -183,14 +189,14 @@ static inline bool da_event(struct da_monitor *da_mon, struct task_struct *tsk,
/*
* global monitor (a single variable)
*/
-static struct da_monitor da_mon_this;
+static struct da_monitor DA_MON_NAME;
/*
* da_get_monitor - return the global monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return &da_mon_this;
+ return &DA_MON_NAME;
}
/*
@@ -223,14 +229,14 @@ static inline void da_monitor_destroy(void) { }
/*
* per-cpu monitor variables
*/
-static DEFINE_PER_CPU(struct da_monitor, da_mon_this);
+static DEFINE_PER_CPU(struct da_monitor, DA_MON_NAME);
/*
* da_get_monitor - return current CPU monitor address
*/
static struct da_monitor *da_get_monitor(void)
{
- return this_cpu_ptr(&da_mon_this);
+ return this_cpu_ptr(&DA_MON_NAME);
}
/*
@@ -242,7 +248,7 @@ static void da_monitor_reset_all(void)
int cpu;
for_each_cpu(cpu, cpu_online_mask) {
- da_mon = per_cpu_ptr(&da_mon_this, cpu);
+ da_mon = per_cpu_ptr(&DA_MON_NAME, cpu);
da_monitor_reset(da_mon);
}
}
--
2.53.0
^ permalink raw reply related
* [PATCH] tracing: ring-buffer: Fix to check event length before using
From: Masami Hiramatsu (Google) @ 2026-02-16 9:30 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Check the event length before adding it for accessing next index in
rb_read_data_buffer(). Since this function is used for validating
possibly broken ring buffers, the length of the event could be broken.
In that case, the new event (e + len) can point a wrong address.
To avoid invalid memory access at boot, check whether the length of
each event is in the possible range before using it.
Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
kernel/trace/ring_buffer.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 630221b00838..1ef17d6fd824 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1848,6 +1848,7 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
struct ring_buffer_event *event;
u64 ts, delta;
int events = 0;
+ int len;
int e;
*delta_ptr = 0;
@@ -1855,9 +1856,12 @@ static int rb_read_data_buffer(struct buffer_data_page *dpage, int tail, int cpu
ts = dpage->time_stamp;
- for (e = 0; e < tail; e += rb_event_length(event)) {
+ for (e = 0; e < tail; e += len) {
event = (struct ring_buffer_event *)(dpage->data + e);
+ len = rb_event_length(event);
+ if (len <= 0 || len > tail - e)
+ return -1;
switch (event->type_len) {
^ permalink raw reply related
* [PATCH v2 0/4] Clean up access to trace_event_file from a file struct
From: Petr Pavlu @ 2026-02-16 13:41 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
This series includes several patches related to accessing
trace_event_file from a file struct. The first patch is a fix for an
edge case, the remaining patches are minor cleanups.
Changes since v1 [1]:
* Fix a compilation error when CONFIG_HIST_TRIGGERS is not set.
* Drop a patch that references the trace_event_file data in
event_file_data() and keep the simpler implementation of storing the
id in i_private.
* Inline event_file_data() into event_id_read() to enable adding
additional checks to the former.
Petr Pavlu (4):
tracing: Fix checking of freed trace_event_file for hist files
tracing: Remove unnecessary check for EVENT_FILE_FL_FREED
tracing: Clean up access to trace_event_file from a file pointer
tracing: Free up file->private_data for use by individual events
include/linux/trace_events.h | 5 +++++
kernel/trace/trace.c | 2 --
kernel/trace/trace.h | 17 +++++++++++------
kernel/trace/trace_events.c | 17 ++++++++---------
kernel/trace/trace_events_hist.c | 8 ++------
5 files changed, 26 insertions(+), 23 deletions(-)
base-commit: cee73b1e840c154f64ace682cb477c1ae2e29cc4
--
2.52.0
^ permalink raw reply
* [PATCH v2 1/4] tracing: Fix checking of freed trace_event_file for hist files
From: Petr Pavlu @ 2026-02-16 13:41 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
In-Reply-To: <20260216134247.1311631-1-petr.pavlu@suse.com>
The event_hist_open() and event_hist_poll() functions currently retrieve
a trace_event_file pointer from a file struct by invoking
event_file_data(), which simply returns file->f_inode->i_private. The
functions then check if the pointer is NULL to determine whether the event
is still valid. This approach is flawed because i_private is assigned when
an eventfs inode is allocated and remains set throughout its lifetime.
Instead, the code should call event_file_file(), which checks for
EVENT_FILE_FL_FREED. Using the incorrect access function may result in the
code potentially opening a hist file for an event that is being removed or
becoming stuck while polling on this file.
A related issue is that although event_hist_poll() attempts to verify
whether an event file is being removed, this check may not occur or could
be unnecessarily delayed. This happens because hist_poll_wakeup() is
currently invoked only from event_hist_trigger() when a hist command is
triggered. If the event file is being removed, no associated hist command
will be triggered and a waiter will be woken up only after an unrelated
hist command is triggered.
Fix these issues by changing the access method to event_file_file() and
adding a call to hist_poll_wakeup() in remove_event_file_dir() after
setting the EVENT_FILE_FL_FREED flag. This ensures that a task polling on
a hist file is woken up and receives EPOLLERR.
Fixes: 1bd13edbbed6 ("tracing/hist: Add poll(POLLIN) support on hist file")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
include/linux/trace_events.h | 5 +++++
kernel/trace/trace_events.c | 3 +++
kernel/trace/trace_events_hist.c | 4 ++--
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 3690221ba3d8..f925034e402d 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -683,6 +683,11 @@ static inline void hist_poll_wakeup(void)
#define hist_poll_wait(file, wait) \
poll_wait(file, &hist_poll_wq, wait)
+
+#else
+static inline void hist_poll_wakeup(void)
+{
+}
#endif
#define __TRACE_EVENT_FLAGS(name, value) \
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 137b4d9bb116..e8ed6ba155cf 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1295,6 +1295,9 @@ static void remove_event_file_dir(struct trace_event_file *file)
free_event_filter(file->filter);
file->flags |= EVENT_FILE_FL_FREED;
event_file_put(file);
+
+ /* Wake up hist poll waiters to notice the EVENT_FILE_FL_FREED flag. */
+ hist_poll_wakeup();
}
/*
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index c97bb2fda5c0..744c2aa3d668 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5778,7 +5778,7 @@ static __poll_t event_hist_poll(struct file *file, struct poll_table_struct *wai
guard(mutex)(&event_mutex);
- event_file = event_file_data(file);
+ event_file = event_file_file(file);
if (!event_file)
return EPOLLERR;
@@ -5816,7 +5816,7 @@ static int event_hist_open(struct inode *inode, struct file *file)
guard(mutex)(&event_mutex);
- event_file = event_file_data(file);
+ event_file = event_file_file(file);
if (!event_file) {
ret = -ENODEV;
goto err;
--
2.52.0
^ permalink raw reply related
* [PATCH v2 2/4] tracing: Remove unnecessary check for EVENT_FILE_FL_FREED
From: Petr Pavlu @ 2026-02-16 13:41 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
In-Reply-To: <20260216134247.1311631-1-petr.pavlu@suse.com>
The event_filter_write() function calls event_file_file() to retrieve
a trace_event_file associated with a given file struct. If a non-NULL
pointer is returned, the function then checks whether the trace_event_file
instance has the EVENT_FILE_FL_FREED flag set. This check is redundant
because event_file_file() already performs this validation and returns NULL
if the flag is set. The err value is also already initialized to -ENODEV.
Remove the unnecessary check for EVENT_FILE_FL_FREED in
event_filter_write().
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace_events.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index e8ed6ba155cf..2ca76b048638 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2153,12 +2153,8 @@ event_filter_write(struct file *filp, const char __user *ubuf, size_t cnt,
mutex_lock(&event_mutex);
file = event_file_file(filp);
- if (file) {
- if (file->flags & EVENT_FILE_FL_FREED)
- err = -ENODEV;
- else
- err = apply_event_filter(file, buf);
- }
+ if (file)
+ err = apply_event_filter(file, buf);
mutex_unlock(&event_mutex);
kfree(buf);
--
2.52.0
^ permalink raw reply related
* [PATCH v2 3/4] tracing: Clean up access to trace_event_file from a file pointer
From: Petr Pavlu @ 2026-02-16 13:41 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
In-Reply-To: <20260216134247.1311631-1-petr.pavlu@suse.com>
The tracing code provides two functions event_file_file() and
event_file_data() to obtain a trace_event_file pointer from a file struct.
The primary method to use is event_file_file(), as it checks for the
EVENT_FILE_FL_FREED flag to determine whether the event is being removed.
The second function event_file_data() is an optimization for retrieving the
same data when the event_mutex is still held.
In the past, when removing an event directory in remove_event_file_dir(),
the code set i_private to NULL for all event files and readers were
expected to check for this state to recognize that the event is being
removed. In the case of event_id_read(), the value was read using
event_file_data() without acquiring the event_mutex. This required
event_file_data() to use READ_ONCE() when retrieving the i_private data.
With the introduction of eventfs, i_private is assigned when an eventfs
inode is allocated and remains set throughout its lifetime.
Remove the now unnecessary READ_ONCE() access to i_private in both
event_file_file() and event_file_data(). Inline the access to i_private in
remove_event_file_dir(), which allows event_file_data() to handle i_private
solely as a trace_event_file pointer. Add a check in event_file_data() to
ensure that the event_mutex is held and that file->flags doesn't have the
EVENT_FILE_FL_FREED flag set. Finally, move event_file_data() immediately
after event_file_code() since the latter provides a comment explaining how
both functions should be used together.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace.h | 17 +++++++++++------
kernel/trace/trace_events.c | 6 +++---
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 8428c437cb9d..27fb3fe6a7e0 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1739,11 +1739,6 @@ extern struct trace_event_file *find_event_file(struct trace_array *tr,
const char *system,
const char *event);
-static inline void *event_file_data(struct file *filp)
-{
- return READ_ONCE(file_inode(filp)->i_private);
-}
-
extern struct mutex event_mutex;
extern struct list_head ftrace_events;
@@ -1764,12 +1759,22 @@ static inline struct trace_event_file *event_file_file(struct file *filp)
struct trace_event_file *file;
lockdep_assert_held(&event_mutex);
- file = READ_ONCE(file_inode(filp)->i_private);
+ file = file_inode(filp)->i_private;
if (!file || file->flags & EVENT_FILE_FL_FREED)
return NULL;
return file;
}
+static inline void *event_file_data(struct file *filp)
+{
+ struct trace_event_file *file;
+
+ lockdep_assert_held(&event_mutex);
+ file = file_inode(filp)->i_private;
+ WARN_ON(!file || file->flags & EVENT_FILE_FL_FREED);
+ return file;
+}
+
extern const struct file_operations event_trigger_fops;
extern const struct file_operations event_hist_fops;
extern const struct file_operations event_hist_debug_fops;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 2ca76b048638..d23e7a9d07e6 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2090,12 +2090,12 @@ static int trace_format_open(struct inode *inode, struct file *file)
static ssize_t
event_id_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
{
- int id = (long)event_file_data(filp);
+ /* id is directly in i_private and available for inode's lifetime. */
+ int id = (long)file_inode(filp)->i_private;
char buf[32];
int len;
- if (unlikely(!id))
- return -ENODEV;
+ WARN_ON(!id);
len = sprintf(buf, "%d\n", id);
--
2.52.0
^ permalink raw reply related
* [PATCH v2 4/4] tracing: Free up file->private_data for use by individual events
From: Petr Pavlu @ 2026-02-16 13:42 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, Tom Zanussi, linux-kernel, linux-trace-kernel,
Petr Pavlu
In-Reply-To: <20260216134247.1311631-1-petr.pavlu@suse.com>
The tracing_open_file_tr() function currently copies the trace_event_file
pointer from inode->i_private to file->private_data when the file is
successfully opened. This duplication is not particularly useful, as all
event code should utilize event_file_file() or event_file_data() to
retrieve a trace_event_file pointer from a file struct and these access
functions read file->f_inode->i_private. Moreover, this setup requires the
code for opening hist files to explicitly clear file->private_data before
calling single_open(), since this function expects the private_data member
to be set to NULL and uses it to store a pointer to a seq_file.
Remove the unnecessary setting of file->private_data in
tracing_open_file_tr() and simplify the hist code.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/trace/trace.c | 2 --
kernel/trace/trace_events_hist.c | 4 ----
2 files changed, 6 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index b1cb30a7b83d..01af8d88a468 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4804,8 +4804,6 @@ int tracing_open_file_tr(struct inode *inode, struct file *filp)
event_file_get(file);
}
- filp->private_data = inode->i_private;
-
return 0;
}
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 744c2aa3d668..1ca3e14d7531 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -5831,8 +5831,6 @@ static int event_hist_open(struct inode *inode, struct file *file)
hist_file->file = file;
hist_file->last_act = get_hist_hit_count(event_file);
- /* Clear private_data to avoid warning in single_open() */
- file->private_data = NULL;
ret = single_open(file, hist_show, hist_file);
if (ret) {
kfree(hist_file);
@@ -6114,8 +6112,6 @@ static int event_hist_debug_open(struct inode *inode, struct file *file)
if (ret)
return ret;
- /* Clear private_data to avoid warning in single_open() */
- file->private_data = NULL;
ret = single_open(file, hist_debug_show, file);
if (ret)
tracing_release_file_tr(inode, file);
--
2.52.0
^ permalink raw reply related
* Re: Enhancing Conditional Filtering for Function Graph Tracer
From: Steven Rostedt @ 2026-02-16 15:09 UTC (permalink / raw)
To: Donglin Peng
Cc: Masami Hiramatsu, linux-trace-kernel, Linux Kernel Mailing List
In-Reply-To: <CAErzpmtMrHWQVAQx=jiq_sWo=+c_JOUoQmPafo-fq+Q3qi+BBw@mail.gmail.com>
On Sat, 14 Feb 2026 18:36:24 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> To simplify implementation, I propose extending a new trigger type
> (e.g., "funcgraph").
> In ftrace_graph_ignore_func, we could look up the corresponding trace_fprobe and
> trace_event_file based on trace->func, then decide whether to trace
> the function using
> a helper like the following:
>
> static bool ftrace_graph_filter(struct trace_fprobe *tf, struct
> ftrace_regs *fregs,
> struct trace_event_file *trace_file)
> {
> struct fentry_trace_entry_head *entry;
> struct trace_event_buffer fbuffer;
> struct event_trigger_data *data;
> int dsize;
>
> dsize = __get_data_size(&tf->tp, fregs, NULL);
> entry = trace_event_buffer_reserve(&fbuffer, trace_file,
> sizeof(*entry) + tf->tp.size + dsize);
> if (!entry)
> return false;
>
> entry = ring_buffer_event_data(fbuffer.event);
> store_trace_args(&entry[1], &tf->tp, fregs, NULL, sizeof(*entry), dsize);
>
> list_for_each_entry_rcu(data, &trace_file->triggers, list) {
> if (data->cmd_ops->trigger_type == TRIGGER_TYPE_FUNCGRAPH) {
> struct event_filter *filter = rcu_dereference_sched(data->filter);
> if (filter && filter_match_preds(filter, entry))
> return true; // Allow tracing
> }
> }
> return false; // Skip tracing
> }
>
> Does this approach make sense? Any suggestions or concerns?
My biggest concern is with performance. You want to run this against all
functions being traced?
How is this different than just using fprobes?
-- Steve
^ permalink raw reply
* Re: [PATCH v2] rv: Fix multiple definition of __pcpu_unique_da_mon_this
From: Steven Rostedt @ 2026-02-16 15:14 UTC (permalink / raw)
To: Mikhail Gavrilov
Cc: Gabriele Monaco, Daniel Bristot de Oliveira, Nam Cao,
linux-trace-kernel, linux-kernel
In-Reply-To: <20260216090141.757726-1-mikhail.v.gavrilov@gmail.com>
FYI, please send new versions of a patch as a separate thread. Do not send
it as a reply to the first version. That makes it much more difficult for
maintainers to keep track of.
You should also add what changed since the first version:
> The refactoring in commit 30984ccf31b7 ("rv: Refactor da_monitor to
> minimise macros") replaced per-monitor unique variable names
> (da_mon_##name) with a fixed name (da_mon_this).
>
> While this works for 'static' variables (each translation unit gets its
> own copy), DEFINE_PER_CPU internally generates a non-static dummy
> variable __pcpu_unique_<n> for each per-cpu definition. The requirement
> for this variable to be unique although static exists for modules on
> specific architectures (alpha) and if the kernel is built with
> CONFIG_DEBUG_FORCE_WEAK_PER_CPU (e.g. Fedora's debug kernel).
>
> When multiple per-cpu monitors (e.g. sco and sts) are built-in
> simultaneously, they all produce the same __pcpu_unique_da_mon_this
> symbol, causing a link error:
>
> ld: kernel/trace/rv/monitors/sts/sts.o: multiple definition of
> `__pcpu_unique_da_mon_this';
> kernel/trace/rv/monitors/sco/sco.o: first defined here
>
> Fix this by introducing a DA_MON_NAME macro that expands to a
> per-monitor unique name (da_mon_<MONITOR_NAME>) via the existing
> CONCATENATE helper. This restores the uniqueness that was present
> before the refactoring.
>
> Fixes: 30984ccf31b7 ("rv: Refactor da_monitor to minimise macros")
> Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> ---
Changes since v1: https://lore.kernel.org/all/20260216074727.741822-1-mikhail.v.gavrilov@gmail.com/
- <list changes here>
> include/rv/da_monitor.h | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index db11d41bb438..7511f5464c48 10064
This way this patch has a link to the previous version. Then when this
patch gets committed, it the commit has a link to this patch, and that
patch has a link to a previous version and the chain of updates is archived
nicely.
-- Steve
^ permalink raw reply
* Re: [PATCH mm-unstable v14 07/16] khugepaged: introduce collapse_max_ptes_none helper function
From: Lorenzo Stoakes @ 2026-02-16 15:16 UTC (permalink / raw)
To: Nico Pache
Cc: linux-mm, linux-doc, linux-kernel, linux-trace-kernel, akpm,
david, ziy, baolin.wang, Liam.Howlett, ryan.roberts, dev.jain,
baohua, lance.yang, vbabka, rppt, surenb, mhocko, corbet, rostedt,
mhiramat, mathieu.desnoyers, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, ying.huang, apopple, jannh,
pfalcato, jackmanb, hannes, willy, peterx, wangkefeng.wang,
usamaarif642, sunnanyong, vishal.moola, thomas.hellstrom, yang,
kas, aarcange, raquini, anshuman.khandual, catalin.marinas, tiwai,
will, dave.hansen, jack, cl, jglisse, zokeefe, rientjes, rdunlap,
hughd, richard.weiyang
In-Reply-To: <CAA1CXcBQWPD=AxX9mCOdAOv85LTk+FPJQeEudQur-ymg4vbp5g@mail.gmail.com>
On Fri, Feb 06, 2026 at 10:44:03AM -0700, Nico Pache wrote:
> On Tue, Feb 3, 2026 at 5:09 AM Lorenzo Stoakes
> > > ---
> > > mm/khugepaged.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 42 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > > index 0f68902edd9a..9b7e05827749 100644
> > > --- a/mm/khugepaged.c
> > > +++ b/mm/khugepaged.c
> > > @@ -460,6 +460,44 @@ void __khugepaged_enter(struct mm_struct *mm)
> > > wake_up_interruptible(&khugepaged_wait);
> > > }
> > >
> > > +/**
> > > + * collapse_max_ptes_none - Calculate maximum allowed empty PTEs for collapse
> > > + * @order: The folio order being collapsed to
> > > + * @full_scan: Whether this is a full scan (ignore limits)
> > > + *
> > > + * For madvise-triggered collapses (full_scan=true), all limits are bypassed
> > > + * and allow up to HPAGE_PMD_NR - 1 empty PTEs.
> > > + *
> > > + * For PMD-sized collapses (order == HPAGE_PMD_ORDER), use the configured
> > > + * khugepaged_max_ptes_none value.
> > > + *
> > > + * For mTHP collapses, we currently only support khugepaged_max_pte_none values
> > > + * of 0 or (HPAGE_PMD_NR - 1). Any other value will emit a warning and no mTHP
> > > + * collapse will be attempted
> > > + *
> > > + * Return: Maximum number of empty PTEs allowed for the collapse operation
> > > + */
> > > +static unsigned int collapse_max_ptes_none(unsigned int order, bool full_scan)
> > > +{
> > > + /* ignore max_ptes_none limits */
> > > + if (full_scan)
> > > + return HPAGE_PMD_NR - 1;
> >
> > I wonder if, given we are effectively doing:
> >
> > const unsigned int nr_pages = collapse_max_ptes_none(order, /*full_scan=*/true);
> >
> > ...
> >
> > foo(nr_pages);
> >
> > In places where we ignore limits, whether we would be better off putting
> > HPAGE_PMD_NR - 1 into a define and just using that in these cases, like:
> >
> > #define COLLAPSE_MAX_PTES_LIM (HPAGE_PMD_NR - 1)
>
> Would a shorter name be appropriate? COLLAPSE_MAX_PTES_LIM(IT) is
> quite long. Can we call it MAX_PTES_LIMIT or KHUGE_MAX_PTES_LIM?
Yeah sure re: shorter/better name :) to be fair my suggestion is pretty
terrible, kinda just getting at the notion of there being _some_ define.
But MAX_PTES_LIMIT or KHUGE_MAX_PTES_LIM I think are unclear.
MAX_COLLAPSE_PTES?
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH mm-unstable v14 08/16] khugepaged: generalize collapse_huge_page for mTHP collapse
From: Lorenzo Stoakes @ 2026-02-16 15:20 UTC (permalink / raw)
To: Nico Pache
Cc: linux-mm, linux-doc, linux-kernel, linux-trace-kernel, akpm,
david, ziy, baolin.wang, Liam.Howlett, ryan.roberts, dev.jain,
baohua, lance.yang, vbabka, rppt, surenb, mhocko, corbet, rostedt,
mhiramat, mathieu.desnoyers, matthew.brost, joshua.hahnjy,
rakie.kim, byungchul, gourry, ying.huang, apopple, jannh,
pfalcato, jackmanb, hannes, willy, peterx, wangkefeng.wang,
usamaarif642, sunnanyong, vishal.moola, thomas.hellstrom, yang,
kas, aarcange, raquini, anshuman.khandual, catalin.marinas, tiwai,
will, dave.hansen, jack, cl, jglisse, zokeefe, rientjes, rdunlap,
hughd, richard.weiyang
In-Reply-To: <CAA1CXcCCbiV9j+_SVNJrkfVRqKPXjGg+Lt3YnyNUhDHWkRjHGQ@mail.gmail.com>
On Wed, Feb 04, 2026 at 03:00:57PM -0700, Nico Pache wrote:
> On Tue, Feb 3, 2026 at 6:13 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Thu, Jan 22, 2026 at 12:28:33PM -0700, Nico Pache wrote:
> > > Pass an order and offset to collapse_huge_page to support collapsing anon
> > > memory to arbitrary orders within a PMD. order indicates what mTHP size we
> > > are attempting to collapse to, and offset indicates were in the PMD to
> > > start the collapse attempt.
> > >
> > > For non-PMD collapse we must leave the anon VMA write locked until after
> > > we collapse the mTHP-- in the PMD case all the pages are isolated, but in
> > > the mTHP case this is not true, and we must keep the lock to prevent
> > > changes to the VMA from occurring.
> > >
> > > Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > > Tested-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > > Signed-off-by: Nico Pache <npache@redhat.com>
> > > ---
> > > mm/khugepaged.c | 111 +++++++++++++++++++++++++++++++-----------------
> > > 1 file changed, 71 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > > index 9b7e05827749..76cb17243793 100644
> > > --- a/mm/khugepaged.c
> > > +++ b/mm/khugepaged.c
> > > @@ -1151,44 +1151,54 @@ static enum scan_result alloc_charge_folio(struct folio **foliop, struct mm_stru
> > > return SCAN_SUCCEED;
> > > }
> > >
> > > -static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long address,
> > > - int referenced, int unmapped, struct collapse_control *cc)
> > > +static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long start_addr,
> > > + int referenced, int unmapped, struct collapse_control *cc,
> > > + bool *mmap_locked, unsigned int order)
> > > {
> > > LIST_HEAD(compound_pagelist);
> > > pmd_t *pmd, _pmd;
> > > - pte_t *pte;
> > > + pte_t *pte = NULL;
> > > pgtable_t pgtable;
> > > struct folio *folio;
> > > spinlock_t *pmd_ptl, *pte_ptl;
> > > enum scan_result result = SCAN_FAIL;
> > > struct vm_area_struct *vma;
> > > struct mmu_notifier_range range;
> > > + bool anon_vma_locked = false;
> > > + const unsigned long nr_pages = 1UL << order;
> > > + const unsigned long pmd_address = start_addr & HPAGE_PMD_MASK;
> > >
> > > - VM_BUG_ON(address & ~HPAGE_PMD_MASK);
> > > + VM_WARN_ON_ONCE(pmd_address & ~HPAGE_PMD_MASK);
> > >
> > > /*
> > > * Before allocating the hugepage, release the mmap_lock read lock.
> > > * The allocation can take potentially a long time if it involves
> > > * sync compaction, and we do not need to hold the mmap_lock during
> > > * that. We will recheck the vma after taking it again in write mode.
> > > + * If collapsing mTHPs we may have already released the read_lock.
> > > */
> > > - mmap_read_unlock(mm);
> > > + if (*mmap_locked) {
> > > + mmap_read_unlock(mm);
> > > + *mmap_locked = false;
> > > + }
> > >
> > > - result = alloc_charge_folio(&folio, mm, cc, HPAGE_PMD_ORDER);
> > > + result = alloc_charge_folio(&folio, mm, cc, order);
> > > if (result != SCAN_SUCCEED)
> > > goto out_nolock;
> > >
> > > mmap_read_lock(mm);
> > > - result = hugepage_vma_revalidate(mm, address, true, &vma, cc,
> > > - HPAGE_PMD_ORDER);
> > > + *mmap_locked = true;
> > > + result = hugepage_vma_revalidate(mm, pmd_address, true, &vma, cc, order);
> >
> > Why would we use the PMD address here rather than the actual start address?
>
> The revalidation relies on the pmd_addr not the start_addr. It (only)
> uses this to make sure the VMA is still at least PMD sized, and it
> uses the order to validate that the target order is allowed. I left a
> small comment about this in the revalidate function.
Yeah having these different addresses is a bit icky, easy to make mistakes here.
Oh how we need to refactor all of these...
>
> >
> > Also please add /*expect_anon=*/ before the 'true' because it's hard to
> > understand what that references.
>
> ack
>
> >
> > > if (result != SCAN_SUCCEED) {
> > > mmap_read_unlock(mm);
> > > + *mmap_locked = false;
> > > goto out_nolock;
> > > }
> > >
> > > - result = find_pmd_or_thp_or_none(mm, address, &pmd);
> > > + result = find_pmd_or_thp_or_none(mm, pmd_address, &pmd);
> > > if (result != SCAN_SUCCEED) {
> > > mmap_read_unlock(mm);
> > > + *mmap_locked = false;
> > > goto out_nolock;
> > > }
> > >
> > > @@ -1198,13 +1208,16 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > * released when it fails. So we jump out_nolock directly in
> > > * that case. Continuing to collapse causes inconsistency.
> > > */
> > > - result = __collapse_huge_page_swapin(mm, vma, address, pmd,
> > > - referenced, HPAGE_PMD_ORDER);
> > > - if (result != SCAN_SUCCEED)
> > > + result = __collapse_huge_page_swapin(mm, vma, start_addr, pmd,
> > > + referenced, order);
> > > + if (result != SCAN_SUCCEED) {
> > > + *mmap_locked = false;
> > > goto out_nolock;
> > > + }
> > > }
> > >
> > > mmap_read_unlock(mm);
> > > + *mmap_locked = false;
> > > /*
> > > * Prevent all access to pagetables with the exception of
> > > * gup_fast later handled by the ptep_clear_flush and the VM
> > > @@ -1214,20 +1227,20 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > * mmap_lock.
> > > */
> > > mmap_write_lock(mm);
> > > - result = hugepage_vma_revalidate(mm, address, true, &vma, cc,
> > > - HPAGE_PMD_ORDER);
> > > + result = hugepage_vma_revalidate(mm, pmd_address, true, &vma, cc, order);
> > > if (result != SCAN_SUCCEED)
> > > goto out_up_write;
> > > /* check if the pmd is still valid */
> > > vma_start_write(vma);
> > > - result = check_pmd_still_valid(mm, address, pmd);
> > > + result = check_pmd_still_valid(mm, pmd_address, pmd);
> > > if (result != SCAN_SUCCEED)
> > > goto out_up_write;
> > >
> > > anon_vma_lock_write(vma->anon_vma);
> > > + anon_vma_locked = true;
> > >
> > > - mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm, address,
> > > - address + HPAGE_PMD_SIZE);
> > > + mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm, start_addr,
> > > + start_addr + (PAGE_SIZE << order));
> > > mmu_notifier_invalidate_range_start(&range);
> > >
> > > pmd_ptl = pmd_lock(mm, pmd); /* probably unnecessary */
> > > @@ -1239,24 +1252,21 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > * Parallel GUP-fast is fine since GUP-fast will back off when
> > > * it detects PMD is changed.
> > > */
> > > - _pmd = pmdp_collapse_flush(vma, address, pmd);
> > > + _pmd = pmdp_collapse_flush(vma, pmd_address, pmd);
> > > spin_unlock(pmd_ptl);
> > > mmu_notifier_invalidate_range_end(&range);
> > > tlb_remove_table_sync_one();
> > >
> > > - pte = pte_offset_map_lock(mm, &_pmd, address, &pte_ptl);
> > > + pte = pte_offset_map_lock(mm, &_pmd, start_addr, &pte_ptl);
> > > if (pte) {
> > > - result = __collapse_huge_page_isolate(vma, address, pte, cc,
> > > - HPAGE_PMD_ORDER,
> > > - &compound_pagelist);
> > > + result = __collapse_huge_page_isolate(vma, start_addr, pte, cc,
> > > + order, &compound_pagelist);
> > > spin_unlock(pte_ptl);
> > > } else {
> > > result = SCAN_NO_PTE_TABLE;
> > > }
> > >
> > > if (unlikely(result != SCAN_SUCCEED)) {
> > > - if (pte)
> > > - pte_unmap(pte);
> > > spin_lock(pmd_ptl);
> > > BUG_ON(!pmd_none(*pmd));
> > > /*
> > > @@ -1266,21 +1276,21 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > */
> > > pmd_populate(mm, pmd, pmd_pgtable(_pmd));
> > > spin_unlock(pmd_ptl);
> > > - anon_vma_unlock_write(vma->anon_vma);
> > > goto out_up_write;
> > > }
> > >
> > > /*
> > > - * All pages are isolated and locked so anon_vma rmap
> > > - * can't run anymore.
> > > + * For PMD collapse all pages are isolated and locked so anon_vma
> > > + * rmap can't run anymore. For mTHP collapse we must hold the lock
> > > */
> > > - anon_vma_unlock_write(vma->anon_vma);
> > > + if (is_pmd_order(order)) {
> > > + anon_vma_unlock_write(vma->anon_vma);
> > > + anon_vma_locked = false;
> > > + }
> > >
> > > result = __collapse_huge_page_copy(pte, folio, pmd, _pmd,
> > > - vma, address, pte_ptl,
> > > - HPAGE_PMD_ORDER,
> > > - &compound_pagelist);
> > > - pte_unmap(pte);
> > > + vma, start_addr, pte_ptl,
> > > + order, &compound_pagelist);
> > > if (unlikely(result != SCAN_SUCCEED))
> > > goto out_up_write;
> > >
> > > @@ -1290,20 +1300,42 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long a
> > > * write.
> > > */
> > > __folio_mark_uptodate(folio);
> > > - pgtable = pmd_pgtable(_pmd);
> > > + if (is_pmd_order(order)) { /* PMD collapse */
> > > + pgtable = pmd_pgtable(_pmd);
> > >
> > > - spin_lock(pmd_ptl);
> > > - BUG_ON(!pmd_none(*pmd));
> > > - pgtable_trans_huge_deposit(mm, pmd, pgtable);
> > > - map_anon_folio_pmd_nopf(folio, pmd, vma, address);
> > > + spin_lock(pmd_ptl);
> > > + WARN_ON_ONCE(!pmd_none(*pmd));
> > > + pgtable_trans_huge_deposit(mm, pmd, pgtable);
> > > + map_anon_folio_pmd_nopf(folio, pmd, vma, pmd_address);
> > > + } else { /* mTHP collapse */
> > > + pte_t mthp_pte = mk_pte(folio_page(folio, 0), vma->vm_page_prot);
> > > +
> > > + mthp_pte = maybe_mkwrite(pte_mkdirty(mthp_pte), vma);
> > > + spin_lock(pmd_ptl);
> > > + WARN_ON_ONCE(!pmd_none(*pmd));
> > > + folio_ref_add(folio, nr_pages - 1);
> > > + folio_add_new_anon_rmap(folio, vma, start_addr, RMAP_EXCLUSIVE);
> > > + folio_add_lru_vma(folio, vma);
> > > + set_ptes(vma->vm_mm, start_addr, pte, mthp_pte, nr_pages);
> > > + update_mmu_cache_range(NULL, vma, start_addr, pte, nr_pages);
> > > +
> > > + smp_wmb(); /* make PTEs visible before PMD. See pmd_install() */
> > > + pmd_populate(mm, pmd, pmd_pgtable(_pmd));
> >
> > I seriously hate this being open-coded, can we separate it out into another
> > function?
>
> Yeah I think we've discussed this before. I started to generalize
> this, and apply it to other parts of the kernel that maintain a
> similar pattern, but each potential user of the helper was slightly
> different in its approach and I was unable to find a quick solution to
> make it apply to all. I think it will require a lot more thought to
> cleanly refactor this. I figured I could leave this to the later
> cleanup work, or I could just create a static function just for
> khugepaged for now?
Yeah let's at least separate it out for this logic anyway.
Really we should have done the refactoring in advance of these changes, but that
ship has sailed :)
>
> >
> > > + }
> > > spin_unlock(pmd_ptl);
> > >
> > > folio = NULL;
> > >
> > > result = SCAN_SUCCEED;
> > > out_up_write:
> > > + if (anon_vma_locked)
> > > + anon_vma_unlock_write(vma->anon_vma);
> >
> > Thanks it's much better tracking this specifically.
> >
> > The whole damn thing needs refactoring (by this I mean - khugepaged and really
> > THP in general to be clear :) but it's not your fault.
>
> Yeah it has not been the prettiest code to try and understand/work on!
:)
>
> >
> > Could I ask though whether you might help out with some cleanups after this
> > lands :)
> >
> > I feel like we all need to do our bit to pay down some technical debt!
>
>
> Yes ofc! I had already planned on doing so. I have some in mind, and I
> believe others have already tackled some. After this land, let's
> discuss further plans (discussion thread or THP meeting).
Yeah, I'll get that TODO list discussed in meeting shared soon...
>
> Cheers,
> -- Nico
>
> >
> > > + if (pte)
> > > + pte_unmap(pte);
> > > mmap_write_unlock(mm);
> > > + *mmap_locked = false;
> > > out_nolock:
> > > + WARN_ON_ONCE(*mmap_locked);
> > > if (folio)
> > > folio_put(folio);
> > > trace_mm_collapse_huge_page(mm, result == SCAN_SUCCEED, result);
> > > @@ -1471,9 +1503,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > > pte_unmap_unlock(pte, ptl);
> > > if (result == SCAN_SUCCEED) {
> > > result = collapse_huge_page(mm, start_addr, referenced,
> > > - unmapped, cc);
> > > - /* collapse_huge_page will return with the mmap_lock released */
> > > - *mmap_locked = false;
> > > + unmapped, cc, mmap_locked,
> > > + HPAGE_PMD_ORDER);
> > > }
> > > out:
> > > trace_mm_khugepaged_scan_pmd(mm, folio, referenced,
> > > --
> > > 2.52.0
> > >
> >
> > Cheers, Lorenzo
> >
>
Cheers, Lorenzo
^ permalink raw reply
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