* Re: [PATCH 3/3] rv/reactors: add KUnit tests for reactor_panic
From: Gabriele Monaco @ 2026-07-10 8:27 UTC (permalink / raw)
To: Wen Yang; +Cc: Nam Cao, linux-trace-kernel, linux-kernel
In-Reply-To: <18dcc24d-329e-43f4-bc69-e368c1f1d066@linux.dev>
On Fri, 2026-07-10 at 00:46 +0800, Wen Yang wrote:
> Since rv_panic_reaction calls vpanic() which is __noreturn, direct
> testing is not feasible in KUnit.
>
> A preparatory v2 drops reactor_printk_kunit.c and reactor_panic_kunit.c
> entirely, replacing them with a single rv_reactors_kunit.c containing
> two suites:
>
> rv_reactor_registration: register/unregister lifecycle, duplicate
> rejection (-EINVAL), name-too-long rejection, and safe unregister
> of a never-registered reactor.
>
> rv_react_dispatch: null-callback guard, callback invocation check,
> and the mdelay lockdep stress test.
>
>
> v2 also adds EXPORT_SYMBOL_GPL for rv_react(), rv_register_reactor(),
> and rv_unregister_reactor(), and the Kconfig entry is tristate.
Alright, sounds good.
> Additional fix: rv_unregister_reactor()
>
> Testing exposed a real bug: rv_unregister_reactor() called list_del()
> unconditionally. On a never-registered reactor the list_head is
> zero-initialised, so list_del() dereferences NULL->prev and crashes.
> v2 adds a patch that iterates rv_reactors_list first and only calls
> list_del() when the reactor is found. test_unregister_nonexistent
> documents and guards this behaviour.
Mmh, besides unit testing, when can we even see a reactor unregistration before
it's ever registered? Do we really need to actively guard against this?
What is probably an issue is to call unregistration also if registration failed
in reactors (which would cause the /bug/ you see, but shouldn't happen on
current reactors anyway), we can fix that.
I wouldn't guard against an issue that can only be triggered by broken code.
What am I missing?
Thanks,
Gabriele
^ permalink raw reply
* Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test
From: Gabriele Monaco @ 2026-07-10 8:31 UTC (permalink / raw)
To: Wen Yang, linux-trace-kernel, linux-kernel
Cc: Nam Cao, Thomas Weissschuh, Tomas Glozar, John Kacur,
Steven Rostedt, Shuah Khan, linux-kselftest
In-Reply-To: <cd0a678b-1eec-42d8-b027-0d27b9c804a3@linux.dev>
On Fri, 2026-07-10 at 01:08 +0800, Wen Yang wrote:
> > load() { # returns true if there was a reaction
> > - local lines_before num
> > + local lines_before num load_pid ret
> > num=$((($(nproc) + 1) / 2))
> > lines_before=$(dmesg | wc -l)
> > - stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q
> > - dmesg | tail -n $((lines_before + 1)) | grep -q "rv: monitor wwnr
> > does not allow event"
> > + stress-ng --cpu-sched "$num" --timer "$num" -t 5 -q &
> > + load_pid=$!
> > + timeout 5 dmesg -w | tail -n +$((lines_before + 1)) | grep -m 1 -q
> > "rv: monitor wwnr does not allow event"
>
>
> Minor nit: could we add a small delay (e.g., sleep 0.1) before dmesg?
I don't get what the benefit would be.
The intent of that line is to run a continuous dmesg (-w) for up to 5s (timeout)
but stopping if an occurrence is found (grep -m 1 would close the pipe).
Do you see a case in which this wouldn't happen?
>
> Reviewed-by: Wen Yang <wen.yang@linux.dev>
>
> > + ret=$?
> > + kill "$load_pid"
> > + wait "$load_pid"
> > + return $ret
> > }
> >
> > echo 1 > monitors/wwnr/enable
> > echo printk > monitors/wwnr/reactors
> >
> > -load
> > -
> > echo 0 > monitoring_on
> > ! load || false
> > echo 1 > monitoring_on
> >
> > -load
> > -
> > echo 0 > reacting_on
> > ! load || false
> > echo 1 > reacting_on
> >
> > +load
> > +
> > echo nop > monitors/wwnr/reactors
> > echo 0 > monitors/wwnr/enable
^ permalink raw reply
* Re: [PATCH v3 16/17] selftests/verification: Rearrange the wwnr_printk test
From: Gabriele Monaco @ 2026-07-10 8:41 UTC (permalink / raw)
To: Nam Cao, linux-trace-kernel, linux-kernel
Cc: Thomas Weissschuh, Tomas Glozar, John Kacur, Wen Yang,
Steven Rostedt, Shuah Khan, linux-kselftest
In-Reply-To: <87zezzxt9a.fsf@yellow.woof>
On Fri, 2026-07-10 at 08:18 +0200, Nam Cao wrote:
> Gabriele Monaco <gmonaco@redhat.com> writes:
> > Simplify the test and run the steps expecting no reaction before the one
> > expecting reactions.
>
> This requirement is not clear from the code. I fear a new developer who
> wants to introduce a new test case will easily trip over this, or even
> us a few months from now.
>
> Instead of this, how about each test case discards or ignores all
> previous reactions?
Fair points, the issue is that we are hammering wwnr, on some systems we may
still see printks for long after the monitor was disabled, I tried that grep -m1
trick to mitigate that.
To ignore all previous reactions reliably we need to wait for the buffers to
flush, something like waiting for a full second for nothing new on the
ringbuffer perhaps. I could think of that or at least make this more clear with
comments.
Thanks,
Gabriele
^ permalink raw reply
* Re: [RFC v3 2/2] arm64: kprobes: Allow reentering kprobes while single-stepping
From: Masami Hiramatsu @ 2026-07-10 9:38 UTC (permalink / raw)
To: Pu Hu
Cc: ada.coupriediaz@arm.com, catalin.marinas@arm.com,
davem@davemloft.net, Hongyan Xia, Jiazi Li,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
naveen@kernel.org, will@kernel.org, yang@os.amperecomputing.com
In-Reply-To: <20260710063242.228714-3-hupu@transsion.com>
On Fri, 10 Jul 2026 06:32:55 +0000
Pu Hu <hupu@transsion.com> wrote:
> From: Pu Hu <hupu@transsion.com>
>
> A kprobe can be hit while another kprobe is in KPROBE_HIT_SS state. This
> can happen when tracing or perf code runs from the debug exception path
> while the first kprobe is preparing or executing its out-of-line
> single-step instruction.
>
> Currently arm64 treats a kprobe hit in KPROBE_HIT_SS as unrecoverable,
> the same as a hit in KPROBE_REENTER. This is too strict. A hit in
> KPROBE_HIT_SS is still a one-level reentry and can be handled by saving
> the current kprobe state and setting up single-step for the new probe,
> just like reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
>
> The truly unrecoverable case is hitting another kprobe while already in
> KPROBE_REENTER, because the reentry save area has already been consumed.
>
> Move KPROBE_HIT_SS to the recoverable reentry cases and leave
> KPROBE_REENTER as the unrecoverable nested reentry case.
>
> This change also requires saving saved_irqflag in struct prev_kprobe.
> When a nested kprobe calls kprobes_save_local_irqflag(), it overwrites
> kcb->saved_irqflag with the currently masked DAIF value, losing the
> outer kprobe's original DAIF state. Without this fix, when the outer
> kprobe's single-step finishes, kprobes_restore_local_irqflag() applies
> the wrong DAIF mask and leaves interrupts permanently disabled.
>
> Extend struct prev_kprobe with a saved_irqflag field and save/restore it
> alongside kp and status. This ensures the outer kprobe's original
> interrupt state is preserved across reentry.
>
> This mirrors the x86 fix in commit 6a5022a56ac3
> ("kprobes/x86: Allow to handle reentered kprobe on single-stepping").
>
OK, this looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
for this series.
Will, Catalin, can you pick this series?
Thanks!
> Signed-off-by: Pu Hu <hupu@transsion.com>
> Signed-off-by: Hongyan Xia <hongyan.xia@transsion.com>
> ---
> arch/arm64/include/asm/kprobes.h | 6 ++++++
> arch/arm64/kernel/probes/kprobes.c | 23 ++++++++++++++++++++++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kprobes.h b/arch/arm64/include/asm/kprobes.h
> index f2782560647b..35ce2c94040e 100644
> --- a/arch/arm64/include/asm/kprobes.h
> +++ b/arch/arm64/include/asm/kprobes.h
> @@ -26,6 +26,12 @@
> struct prev_kprobe {
> struct kprobe *kp;
> unsigned int status;
> +
> + /*
> + * The original DAIF state of the outer kprobe, saved here before
> + * a nested kprobe overwrites kcb->saved_irqflag during reentry.
> + */
> + unsigned long saved_irqflag;
> };
>
> /* per-cpu kprobe control block */
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index 798e4b091d1a..4e0efad5caf2 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -174,12 +174,27 @@ static void __kprobes save_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> kcb->prev_kprobe.kp = kprobe_running();
> kcb->prev_kprobe.status = kcb->kprobe_status;
> +
> + /*
> + * Save the outer kprobe's original DAIF flags before the nested
> + * kprobe calls kprobes_save_local_irqflag() and overwrites
> + * kcb->saved_irqflag. Without this, the outer kprobe will restore
> + * the wrong DAIF state and leave interrupts permanently masked.
> + */
> + kcb->prev_kprobe.saved_irqflag = kcb->saved_irqflag;
> }
>
> static void __kprobes restore_previous_kprobe(struct kprobe_ctlblk *kcb)
> {
> __this_cpu_write(current_kprobe, kcb->prev_kprobe.kp);
> kcb->kprobe_status = kcb->prev_kprobe.status;
> +
> + /*
> + * Restore the outer kprobe's saved_irqflag so that when its
> + * single-step completes, kprobes_restore_local_irqflag() uses
> + * the correct original DAIF value.
> + */
> + kcb->saved_irqflag = kcb->prev_kprobe.saved_irqflag;
> }
>
> static void __kprobes set_current_kprobe(struct kprobe *p)
> @@ -240,10 +255,16 @@ static int __kprobes reenter_kprobe(struct kprobe *p,
> switch (kcb->kprobe_status) {
> case KPROBE_HIT_SSDONE:
> case KPROBE_HIT_ACTIVE:
> + case KPROBE_HIT_SS:
> + /*
> + * A probe can be hit while another kprobe is preparing or
> + * executing its XOL single-step instruction. This is still a
> + * recoverable one-level reentry, so handle it in the same way as
> + * reentry from KPROBE_HIT_ACTIVE or KPROBE_HIT_SSDONE.
> + */
> kprobes_inc_nmissed_count(p);
> setup_singlestep(p, regs, kcb, 1);
> break;
> - case KPROBE_HIT_SS:
> case KPROBE_REENTER:
> pr_warn("Failed to recover from reentered kprobes.\n");
> dump_kprobe(p);
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [BUG] tracing: Too many tries to read user space
From: Steven Rostedt @ 2026-07-10 11:46 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Jeongho Choi, linux-trace-kernel, linux-kernel, ji2yoon.jo,
minki.jang, hajun.sung
In-Reply-To: <20260710122231.9bc9fae3dcfc72215f4a2dcd@kernel.org>
On Fri, 10 Jul 2026 12:22:31 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> However, this seems a bit strange that we only checks the CPU-wide context
> switching in the loop. Instead, can we introduce a per-cpu sequence counter
> to per-cpu buffer, and check it?
I originally tried this but found a situation that it fails:
tbuf->sequence = 0;
Task 1 Task 2
------ ------
tbuf->sequence++;
seq = tbuf->sequence; (seq = 1)
preempt_enable();
[schedule] ---------------------->
tbuf->sequence++;
seq = tbuf->sequence; (seq = 2);
preempt_enable();
copy_from_user(buffer);
<--------------------[schedule]
copy_from_user(buffer);
*** BUFFER NOW CORRUPTED ***
[schedule] ---------------------->
preempt_disable();
} while (tubf->sequence != seq); // tbuf->sequence == seq !!!!
This is why we use a CPU wide counter.
-- Steve
^ permalink raw reply
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Vincent Donnefort @ 2026-07-10 11:56 UTC (permalink / raw)
To: Steven Rostedt
Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team,
linux-kernel, Sashiko
In-Reply-To: <alCm5fq3UC2JtzZV@google.com>
On Fri, Jul 10, 2026 at 09:01:41AM +0100, Vincent Donnefort wrote:
> On Fri, Jul 10, 2026 at 08:58:51AM +0100, Vincent Donnefort wrote:
> > On Thu, Jul 09, 2026 at 07:15:16PM -0400, Steven Rostedt wrote:
> > > On Thu, 9 Jul 2026 17:00:15 +0100
> > > Vincent Donnefort <vdonnefort@google.com> wrote:
> > >
> > > > If page allocation fails in trace_remote_alloc_buffer(), desc->nr_cpus
> > > > is not yet incremented for the current CPU. As a consequence, on error,
> > > > half-allocated rb_desc will not be freed in trace_remote_free_buffer().
> > > >
> > > > Increment desc->nr_cpus as soon as the first allocation for the current
> > > > CPU has succeeded.
> > > >
> > > > Fixes: 96e43537af54 ("tracing: Introduce trace remotes")
> > > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> > >
> > > This patch makes Sashiko find other possible issues with the code :-p
> > >
> > > https://sashiko.dev/#/patchset/20260709160017.1729517-2-vdonnefort%40google.com
> >
> > So one of them is fixed as part of "tracing/remotes: Add printk, dump_on_panic
> > and boot parameters" [1]. I can pull those fixes into this series instead.
> >
> > I'll look at the rest.
>
> Ha yes this other one is real too. However this is in arch/arm64/kvm/ so I'll
> post it separately to kvmarm.
Posted here: https://lore.kernel.org/all/20260710114819.2689386-3-vdonnefort@google.com/
Let me know if I should fold [1] into this series.
>
> >
> > [1] https://lore.kernel.org/all/20260605163825.1762953-2-vdonnefort@google.com/
> >
> > >
> > > -- Steve
^ permalink raw reply
* Re: [BUG] tracing: Too many tries to read user space
From: Steven Rostedt @ 2026-07-10 12:33 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Jeongho Choi, linux-trace-kernel, linux-kernel, ji2yoon.jo,
minki.jang, hajun.sung
In-Reply-To: <20260710122231.9bc9fae3dcfc72215f4a2dcd@kernel.org>
On Fri, 10 Jul 2026 12:22:31 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> Hm, in my view, this warning indicates that the circuit breaker has
> triggered correctly, so that is not a bug. Under the heavy memory
> pressure and low-memory situation, the page can be reclaimed soon
> after it is copied.
So you are saying that every time the copy_from_user() is executed, the
page is reclaimed? And this causes a schedule?
Now, I did have a version that used sched_switch and only incremented the
counter when a non-kernel thread was scheduled in. Then the test would
check if the counter increased by 2 or more. As an increase by 1 meant that
only kernel threads scheduled in which would not corrupt the buffer. The 1
increment was the current task scheduling back.
This is based on that work (I'm glad I save old versions in my git tree :-)
Funny, the comments were from the original change I did back in August of
2025, which mentions kernel threads scheduling in to handle the fault.
I also kept this around in case it was needed. Looks like it may be needed.
-- Steve
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 18710c190c92..19354fe2fca1 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -53,6 +53,8 @@
#include <linux/io.h> /* vmap_page_range() */
#include <linux/fs_context.h>
+#include <trace/events/sched.h>
+
#include <asm/setup.h> /* COMMAND_LINE_SIZE */
#include "trace.h"
@@ -5984,6 +5986,32 @@ struct trace_user_buf {
static DEFINE_MUTEX(trace_user_buffer_mutex);
static struct trace_user_buf_info *trace_user_buffer;
+static DEFINE_PER_CPU(unsigned long, sched_switch_cnt);
+
+/*
+ * The per CPU buffer trace_user_buffer is written to optimstically.
+ * The counter sched_switch_cnt is taken, preemption is enabled,
+ * the copying of the user space memory is placed into the trace_user_buffer,
+ * Preeption is re-enabled and the count is read again. If the count is greater
+ * than one from its previous reading, it means that another user space
+ * task scheduled in and the buffer is unreliable for use.
+ */
+static void
+probe_sched_switch(void *ignore, bool preempt,
+ struct task_struct *prev, struct task_struct *next,
+ unsigned int prev_state)
+{
+ /*
+ * The buffer can only be corrupted by another user space task.
+ * Ignore kernel tasks that may be scheduled in order to process
+ * the faulting memory.
+ */
+ if (!is_user_task(next))
+ return;
+
+ this_cpu_inc(sched_switch_cnt);
+}
+
/**
* trace_user_fault_destroy - free up allocated memory of a trace user buffer
* @tinfo: The descriptor to free up
@@ -6003,6 +6031,8 @@ void trace_user_fault_destroy(struct trace_user_buf_info *tinfo)
kfree(buf);
}
free_percpu(tinfo->tbuf);
+
+ unregister_trace_sched_switch(probe_sched_switch, NULL);
}
static int user_fault_buffer_enable(struct trace_user_buf_info *tinfo, size_t size)
@@ -6053,11 +6083,17 @@ static int user_buffer_init(struct trace_user_buf_info **tinfo, size_t size)
lockdep_assert_held(&trace_user_buffer_mutex);
+ ret = register_trace_sched_switch(probe_sched_switch, NULL);
+ if (ret < 0)
+ return ret;
+
if (!*tinfo) {
alloc = true;
*tinfo = kzalloc_obj(**tinfo);
- if (!*tinfo)
+ if (!*tinfo) {
+ unregister_trace_sched_switch(probe_sched_switch, NULL);
return -ENOMEM;
+ }
}
ret = user_fault_buffer_enable(*tinfo, size);
@@ -6241,7 +6277,7 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
return NULL;
/* Read the current CPU context switch counter */
- cnt = nr_context_switches_cpu(cpu);
+ cnt = this_cpu_read(sched_switch_cnt);
/*
* Preemption is going to be enabled, but this task must
@@ -6272,12 +6308,19 @@ char *trace_user_fault_read(struct trace_user_buf_info *tinfo,
return NULL;
/*
- * Preemption is disabled again, now check the per CPU context
- * switch counter. If it doesn't match, then another user space
- * process may have schedule in and corrupted our buffer. In that
- * case the copying must be retried.
+ * Preemption is disabled again, now check the sched_switch_cnt.
+ * If it increased by two or more, then another user space process
+ * may have schedule in and corrupted our buffer. In that case
+ * the copying must be retried.
+ *
+ * Note, if this task was scheduled out and only kernel threads
+ * were scheduled in (maybe to process the fault), then the
+ * counter would increment again when this task scheduled in.
+ * If this task scheduled out and another user task scheduled
+ * in, this task would still need to be scheduled back in and
+ * the counter would increment by at least two.
*/
- } while (nr_context_switches_cpu(cpu) != cnt);
+ } while (this_cpu_read(sched_switch_cnt) > cnt + 1);
return buffer;
}
^ permalink raw reply related
* Re: [PATCH v2 1/2] module/kallsyms: fix nextval for data symbol lookup
From: Petr Pavlu @ 2026-07-10 12:36 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-modules, Sami Tolvanen, Luis Chamberlain, linux-kernel,
linux-trace-kernel, live-patching, Daniel Gomez, Aaron Tomlin,
Steven Rostedt, Masami Hiramatsu, Jordan Rome, Viktor Malik
In-Reply-To: <20260327110005.16499-1-stf_xl@wp.pl>
On 3/27/26 12:00 PM, Stanislaw Gruszka wrote:
> The symbol lookup code assumes the queried address resides in either
> MOD_TEXT or MOD_INIT_TEXT. This breaks for addresses in other module
> memory regions (e.g. rodata or data), resulting in incorrect upper
> bounds and wrong symbol size.
>
> Select the module memory region the address belongs to instead of
> hardcoding text sections. Also initialize the lower bound to the start
> of that region, as searching from address 0 is unnecessary.
>
> Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
I've queued this first patch on modules-fixes.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v2 1/3] tracing/remotes: Fix leak in trace_remote_alloc_buffer() error path
From: Steven Rostedt @ 2026-07-10 12:37 UTC (permalink / raw)
To: Vincent Donnefort
Cc: mhiramat, linux-trace-kernel, mathieu.desnoyers, kernel-team,
linux-kernel, Sashiko
In-Reply-To: <alDd8OrrcI8vcPDL@google.com>
On Fri, 10 Jul 2026 12:56:32 +0100
Vincent Donnefort <vdonnefort@google.com> wrote:
> > Ha yes this other one is real too. However this is in arch/arm64/kvm/ so I'll
> > post it separately to kvmarm.
>
> Posted here: https://lore.kernel.org/all/20260710114819.2689386-3-vdonnefort@google.com/
>
> Let me know if I should fold [1] into this series.
If they are separate, I'll pull this series in now and start testing it.
I just wanted to know if these would cause you to rethink this series.
-- Steve
^ permalink raw reply
* Re: [PATCH v5 2/2] serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
From: Greg Kroah-Hartman @ 2026-07-10 12:41 UTC (permalink / raw)
To: Praveen Talari
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Jiri Slaby,
konrad.dybcio, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
In-Reply-To: <20260615-add-tracepoints-for-qcom-geni-serial-v5-2-2efa4c97e0e2@oss.qualcomm.com>
On Mon, Jun 15, 2026 at 07:46:53PM +0530, Praveen Talari wrote:
> Add tracing to the Qualcomm GENI serial driver to improve runtime
> observability.
>
> Trace hooks are added at key points including termios and clock
> configuration, manual control get/set, interrupt handling, and data
> TX/RX paths.
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
> v2->v3:
> - Updated commit text(removed example as it was available on cover
> letter).
> ---
> drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
> 1 file changed, 23 insertions(+), 4 deletions(-)
Does not apply to my tty-next branch :(
^ permalink raw reply
* [PATCH] rtla/cli: Unify and improve range validation logic
From: Tomas Glozar @ 2026-07-10 13:15 UTC (permalink / raw)
To: Steven Rostedt, Tomas Glozar
Cc: John Kacur, Luis Goncalves, Crystal Wood, Costa Shulyupin,
Wander Lairson Costa, LKML, linux-trace-kernel
Several RTLA options do range validation inside the CLI parser layer
(e.g. -p/--period). When RTLA migrated CLI parsing to libsubcmd, this
logic was moved unchanged inside opt_*() callbacks.
Unify range validation so that all options use two newly added
functions, check_llong_range() and check_int_range(), to validate the
range.
The new range validation returns -1 from opt_*() callbacks rather than
hard-exit with fatal(), allowing the help message for the specific
option to be automatically displayed by libsubcmd logic.
Many options no longer need a custom callback, as they use the unified
range validation of opt_llong_callback() and opt_int_callback().
Validation for several other options is improved:
- timerlat -p/--period: lower bound raised from 1 to 100 us to match
the kernel's timerlat_min_period in trace_osnoise.c.
- timerlat -A/--aligned: reject negative values.
- timerlat --deepest-idle-state: add range [-1, INT_MAX]; previously,
values <= -2 were read as "option not set".
- timerlat -p/--period, -A/--aligned, -b/--bucket-size: properly reject
negative values instead of passing them to the tracer.
Remove unit tests for removed callbacks and test the new range
validation functionality of opt_llong_callback() and opt_int_callback().
Update runtime tests for histogram options to account for the new error
messages and exit value.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Tomas Glozar <tglozar@redhat.com>
---
Dependencies:
- "rtla: Allow unsetting non-list custom-callback CLI options" patchset
Link: https://lore.kernel.org/linux-trace-kernel/20260629083654.1548925-1-tglozar@redhat.com/T/#u
tools/tracing/rtla/src/cli_p.h | 234 +++++-------
tools/tracing/rtla/tests/osnoise.t | 2 +-
tools/tracing/rtla/tests/timerlat.t | 2 +-
.../rtla/tests/unit/cli_opt_callback.c | 353 ++++++------------
4 files changed, 215 insertions(+), 376 deletions(-)
diff --git a/tools/tracing/rtla/src/cli_p.h b/tools/tracing/rtla/src/cli_p.h
index 3a93dba60215b..b7688559ca53d 100644
--- a/tools/tracing/rtla/src/cli_p.h
+++ b/tools/tracing/rtla/src/cli_p.h
@@ -5,6 +5,7 @@
#error "Private header file included outside of cli.c module"
#endif
+#include <limits.h>
#include <linux/kernel.h>
#include <subcmd/parse-options.h>
@@ -32,9 +33,73 @@ static const int default_bucket_size = 1;
static const int default_entries = 256;
static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE;
+/*
+ * Range checking for long long and int option callbacks.
+ *
+ * Pass a pointer to a const struct as opt->data to enable range checking.
+ * If opt->data is NULL, no range check is performed.
+ */
+struct llong_range {
+ long long min;
+ long long max;
+};
+
+struct int_range {
+ int min;
+ int max;
+};
+
+#define LLONG_RANGE(lo, hi) \
+ (&(const struct llong_range){ .min = (lo), .max = (hi) })
+
+#define INT_RANGE(lo, hi) \
+ (&(const struct int_range){ .min = (lo), .max = (hi) })
+
+static int check_llong_range(const struct option *opt, long long value)
+{
+ const struct llong_range *range = opt->data;
+
+ if (!range)
+ return 0;
+ if (value < range->min || value > range->max) {
+ fprintf(stderr, " Error: --%s value %lld is out of range [%lld, %lld]\n",
+ opt->long_name, value, range->min, range->max);
+ return -1;
+ }
+ return 0;
+}
+
+static int check_int_range(const struct option *opt, int value)
+{
+ const struct int_range *range = opt->data;
+
+ if (!range)
+ return 0;
+ if (value < range->min || value > range->max) {
+ fprintf(stderr, " Error: --%s value %d is out of range [%d, %d]\n",
+ opt->long_name, value, range->min, range->max);
+ return -1;
+ }
+ return 0;
+}
+
+/*
+ * OPT_CALLBACK variant that populates .data (for range checking).
+ */
+#define RTLA_OPT_CALLBACK_DATA(s, l, v, a, h, f, d) \
+ { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), \
+ .value = (v), .argh = (a), .help = (h), .callback = (f), \
+ .data = (void *)(d) }
+
+#define RTLA_OPT_CALLBACK_DATA_DEFVAL(s, l, v, a, h, f, d, dv) \
+ { .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), \
+ .value = (v), .argh = (a), .help = (h), .callback = (f), \
+ .data = (void *)(d), .defval = (intptr_t)(dv) }
+
/*
* Shorthand macros for integer/long long command line options using
- * opt_int_callback/opt_llong_callback, with variants that set defval.
+ * opt_int_callback/opt_llong_callback, with variants that set defval
+ * and/or data (for range checking).
*
* Note: defval's type is intptr_t. opt_int_callback interprets it directly as
* an int, opt_llong_callback interprets it as a pointer to a long long, as
@@ -47,6 +112,10 @@ static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE;
.short_name = (s), .long_name = (l), .value = (v), .argh = (a), \
.help = (h), .callback = opt_llong_callback, .defval = (intptr_t)(d) }
+#define RTLA_OPT_LLONG_DATA(s, l, v, a, h, d) { .type = OPTION_CALLBACK, \
+ .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \
+ .help = (h), .callback = opt_llong_callback, .data = (void *)(d) }
+
#define RTLA_OPT_INT(s, l, v, a, h) \
OPT_CALLBACK(s, l, v, a, h, opt_int_callback)
@@ -54,6 +123,11 @@ static const enum stack_format default_stack_format = STACK_FORMAT_TRUNCATE;
.short_name = (s), .long_name = (l), .value = (v), .argh = (a), \
.help = (h), .callback = opt_int_callback, .defval = (intptr_t)(d) }
+#define RTLA_OPT_INT_DATA_DEFVAL(s, l, v, a, h, d, dv) { .type = OPTION_CALLBACK, \
+ .short_name = (s), .long_name = (l), .value = (v), .argh = (a), \
+ .help = (h), .callback = opt_int_callback, \
+ .data = (void *)(d), .defval = (intptr_t)(dv) }
+
/*
* Macros for command line options common to all tools
*
@@ -182,6 +256,8 @@ static int opt_llong_callback(const struct option *opt, const char *arg, int uns
return -1;
*value = get_llong_from_str((char *)arg);
+ if (check_llong_range(opt, *value))
+ return -1;
return 0;
}
@@ -199,6 +275,8 @@ static int opt_int_callback(const struct option *opt, const char *arg, int unset
if (strtoi(arg, value))
return -1;
+ if (check_int_range(opt, *value))
+ return -1;
return 0;
}
@@ -359,13 +437,13 @@ static int opt_filter_cb(const struct option *opt, const char *arg, int unset)
/*
* Macros for command line options specific to osnoise
*/
-#define OSNOISE_OPT_PERIOD OPT_CALLBACK('p', "period", ¶ms->period, "us", \
+#define OSNOISE_OPT_PERIOD RTLA_OPT_LLONG_DATA('p', "period", ¶ms->period, "us", \
"osnoise period in us", \
- opt_osnoise_period_cb)
+ LLONG_RANGE(1, 10000000))
-#define OSNOISE_OPT_RUNTIME OPT_CALLBACK('r', "runtime", ¶ms->runtime, "us", \
+#define OSNOISE_OPT_RUNTIME RTLA_OPT_LLONG_DATA('r', "runtime", ¶ms->runtime, "us", \
"osnoise runtime in us", \
- opt_osnoise_runtime_cb)
+ LLONG_RANGE(100, LLONG_MAX))
#define OSNOISE_OPT_THRESHOLD RTLA_OPT_LLONG('T', "threshold", ¶ms->threshold, "us", \
"the minimum delta to be considered a noise")
@@ -400,44 +478,6 @@ static int opt_osnoise_auto_cb(const struct option *opt, const char *arg, int un
return 0;
}
-static int opt_osnoise_period_cb(const struct option *opt, const char *arg, int unset)
-{
- unsigned long long *period = opt->value;
-
- if (unset) {
- *period = 0;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *period = get_llong_from_str((char *)arg);
- if (*period > 10000000)
- fatal("Period longer than 10 s");
-
- return 0;
-}
-
-static int opt_osnoise_runtime_cb(const struct option *opt, const char *arg, int unset)
-{
- unsigned long long *runtime = opt->value;
-
- if (unset) {
- *runtime = 0;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *runtime = get_llong_from_str((char *)arg);
- if (*runtime < 100)
- fatal("Runtime shorter than 100 us");
-
- return 0;
-}
-
static int opt_osnoise_trace_output_cb(const struct option *opt, const char *arg, int unset)
{
const char **trace_output = opt->value;
@@ -492,9 +532,9 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int
/*
* Macros for command line options specific to timerlat
*/
-#define TIMERLAT_OPT_PERIOD OPT_CALLBACK('p', "period", ¶ms->timerlat_period_us, "us", \
+#define TIMERLAT_OPT_PERIOD RTLA_OPT_LLONG_DATA('p', "period", ¶ms->timerlat_period_us, "us", \
"timerlat period in us", \
- opt_timerlat_period_cb)
+ LLONG_RANGE(100, 1000000))
#define TIMERLAT_OPT_STACK RTLA_OPT_LLONG('s', "stack", ¶ms->print_stack, "us", \
"save the stack trace at the IRQ if a thread latency is higher than the argument in us")
@@ -503,14 +543,15 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int
"display data in nanoseconds", \
opt_nano_cb)
-#define TIMERLAT_OPT_DMA_LATENCY OPT_CALLBACK(0, "dma-latency", ¶ms->dma_latency, "us", \
+#define TIMERLAT_OPT_DMA_LATENCY RTLA_OPT_INT_DATA_DEFVAL(0, "dma-latency", \
+ ¶ms->dma_latency, "us", \
"set /dev/cpu_dma_latency latency <us> to reduce exit from idle latency", \
- opt_dma_latency_cb)
+ INT_RANGE(0, 10000), default_dma_latency)
-#define TIMERLAT_OPT_DEEPEST_IDLE_STATE RTLA_OPT_INT_DEFVAL(0, "deepest-idle-state", \
+#define TIMERLAT_OPT_DEEPEST_IDLE_STATE RTLA_OPT_INT_DATA_DEFVAL(0, "deepest-idle-state", \
¶ms->deepest_idle_state, "n", \
"only go down to idle state n on cpus used by timerlat to reduce exit from idle latency", \
- default_deepest_idle_state)
+ INT_RANGE(-1, INT_MAX), default_deepest_idle_state)
#define TIMERLAT_OPT_AA_ONLY OPT_CALLBACK(0, "aa-only", params, "us", \
"stop if <us> latency is hit, only printing the auto analysis (reduces CPU usage)", \
@@ -530,33 +571,14 @@ static int opt_osnoise_on_end_cb(const struct option *opt, const char *arg, int
"set the stack format (truncate, skip, full)", \
opt_stack_format_cb)
-#define TIMERLAT_OPT_ALIGNED OPT_CALLBACK('A', "aligned", params, "us", \
+#define TIMERLAT_OPT_ALIGNED RTLA_OPT_CALLBACK_DATA('A', "aligned", params, "us", \
"align thread wakeups to a specific offset", \
- opt_timerlat_align_cb)
+ opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX))
/*
* Callback functions for command line options for timerlat tools
*/
-static int opt_timerlat_period_cb(const struct option *opt, const char *arg, int unset)
-{
- long long *period = opt->value;
-
- if (unset) {
- *period = 0;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *period = get_llong_from_str((char *)arg);
- if (*period > 1000000)
- fatal("Period longer than 1 s");
-
- return 0;
-}
-
static int opt_timerlat_auto_cb(const struct option *opt, const char *arg, int unset)
{
struct timerlat_cb_data *cb_data = opt->value;
@@ -585,28 +607,6 @@ static int opt_timerlat_auto_cb(const struct option *opt, const char *arg, int u
return 0;
}
-static int opt_dma_latency_cb(const struct option *opt, const char *arg, int unset)
-{
- int *dma_latency = opt->value;
- int retval;
-
- if (unset) {
- *dma_latency = default_dma_latency;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- retval = strtoi((char *)arg, dma_latency);
- if (retval)
- fatal("Invalid -dma-latency %s", arg);
- if (*dma_latency < 0 || *dma_latency > 10000)
- fatal("--dma-latency needs to be >= 0 and <= 10000");
-
- return 0;
-}
-
static int opt_aa_only_cb(const struct option *opt, const char *arg, int unset)
{
struct timerlat_params *params = opt->value;
@@ -748,6 +748,8 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int
params->timerlat_align = true;
params->timerlat_align_us = get_llong_from_str((char *)arg);
+ if (check_llong_range(opt, params->timerlat_align_us))
+ return -1;
return 0;
}
@@ -756,14 +758,15 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int
* Macros for command line options specific to histogram-based tools
*/
-#define HIST_OPT_BUCKET_SIZE OPT_CALLBACK('b', "bucket-size", \
+#define HIST_OPT_BUCKET_SIZE RTLA_OPT_INT_DATA_DEFVAL('b', "bucket-size", \
¶ms->common.hist.bucket_size, "N", \
"set the histogram bucket size (default 1)", \
- opt_bucket_size_cb)
+ INT_RANGE(1, 999999), default_bucket_size)
-#define HIST_OPT_ENTRIES OPT_CALLBACK('E', "entries", ¶ms->common.hist.entries, "N", \
+#define HIST_OPT_ENTRIES RTLA_OPT_INT_DATA_DEFVAL('E', "entries", \
+ ¶ms->common.hist.entries, "N", \
"set the number of entries of the histogram (default 256)", \
- opt_entries_cb)
+ INT_RANGE(10, 9999999), default_entries)
#define HIST_OPT_NO_IRQ OPT_BOOLEAN_FLAG(0, "no-irq", ¶ms->common.hist.no_irq, \
"ignore IRQ latencies", PARSE_OPT_NOAUTONEG)
@@ -783,42 +786,3 @@ static int opt_timerlat_align_cb(const struct option *opt, const char *arg, int
#define HIST_OPT_WITH_ZEROS OPT_BOOLEAN(0, "with-zeros", ¶ms->common.hist.with_zeros, \
"print zero only entries")
-/* Histogram-specific callbacks */
-
-static int opt_bucket_size_cb(const struct option *opt, const char *arg, int unset)
-{
- int *bucket_size = opt->value;
-
- if (unset) {
- *bucket_size = default_bucket_size;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *bucket_size = get_llong_from_str((char *)arg);
- if (*bucket_size == 0 || *bucket_size >= 1000000)
- fatal("Bucket size needs to be > 0 and <= 1000000");
-
- return 0;
-}
-
-static int opt_entries_cb(const struct option *opt, const char *arg, int unset)
-{
- int *entries = opt->value;
-
- if (unset) {
- *entries = default_entries;
- return 0;
- }
-
- if (!arg)
- return -1;
-
- *entries = get_llong_from_str((char *)arg);
- if (*entries < 10 || *entries > 9999999)
- fatal("Entries must be > 10 and < 10000000");
-
- return 0;
-}
diff --git a/tools/tracing/rtla/tests/osnoise.t b/tools/tracing/rtla/tests/osnoise.t
index 346a14a860c82..cd9769bb5a9f7 100644
--- a/tools/tracing/rtla/tests/osnoise.t
+++ b/tools/tracing/rtla/tests/osnoise.t
@@ -32,7 +32,7 @@ check "hist with -b/--bucket-size" \
check "hist with -E/--entries" \
"osnoise hist -E 10 -d 1s"
check "hist with -E/--entries out of range" \
- "osnoise hist -E 1 -d 1s" 1 "^Entries must be > 10 and < 10000000$"
+ "osnoise hist -E 1 -d 1s" 129 "out of range \[10, 9999999\]"
check "hist with --no-header" \
"osnoise hist --no-header -d 1s" 0 "" "RTLA osnoise histogram"
check "hist with --with-zeros" \
diff --git a/tools/tracing/rtla/tests/timerlat.t b/tools/tracing/rtla/tests/timerlat.t
index 8193048e8c8ce..57bada5b20631 100644
--- a/tools/tracing/rtla/tests/timerlat.t
+++ b/tools/tracing/rtla/tests/timerlat.t
@@ -61,7 +61,7 @@ check "hist with -b/--bucket-size" \
check "hist with -E/--entries" \
"timerlat hist -E 10 -d 1s"
check "hist with -E/--entries out of range" \
- "timerlat hist -E 1 -d 1s" 1 "^Entries must be > 10 and < 10000000$"
+ "timerlat hist -E 1 -d 1s" 129 "out of range \[10, 9999999\]"
check "hist with --no-header" \
"timerlat hist --no-header -d 1s" 0 "" "RTLA timerlat histogram"
check "hist with --with-zeros" \
diff --git a/tools/tracing/rtla/tests/unit/cli_opt_callback.c b/tools/tracing/rtla/tests/unit/cli_opt_callback.c
index 413a04f898fbb..8439fb5c6f0b3 100644
--- a/tools/tracing/rtla/tests/unit/cli_opt_callback.c
+++ b/tools/tracing/rtla/tests/unit/cli_opt_callback.c
@@ -9,6 +9,12 @@
#include "cli_params_assert.h"
#define TEST_CALLBACK(value, cb) OPT_CALLBACK('t', "test", value, "test value", "test help", cb)
+#define TEST_LLONG_RANGE(value, lo, hi) \
+ RTLA_OPT_CALLBACK_DATA('t', "test", value, "test value", "test help", \
+ opt_llong_callback, LLONG_RANGE(lo, hi))
+#define TEST_INT_RANGE(value, lo, hi) \
+ RTLA_OPT_CALLBACK_DATA('t', "test", value, "test value", "test help", \
+ opt_int_callback, INT_RANGE(lo, hi))
START_TEST(test_opt_llong_callback_simple)
{
@@ -137,6 +143,90 @@ START_TEST(test_opt_int_callback_unset_defval)
}
END_TEST
+START_TEST(test_opt_llong_callback_range_in)
+{
+ long long test_value = 0;
+ const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100);
+
+ ck_assert_int_eq(opt_llong_callback(&opt, "50", 0), 0);
+ ck_assert_int_eq(test_value, 50);
+}
+END_TEST
+
+START_TEST(test_opt_llong_callback_range_below)
+{
+ long long test_value = 0;
+ const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100);
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_llong_callback(&opt, "9", 0), -1);
+}
+END_TEST
+
+START_TEST(test_opt_llong_callback_range_above)
+{
+ long long test_value = 0;
+ const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100);
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_llong_callback(&opt, "101", 0), -1);
+}
+END_TEST
+
+START_TEST(test_opt_llong_callback_range_boundary)
+{
+ long long test_value = 0;
+ const struct option opt = TEST_LLONG_RANGE(&test_value, 10, 100);
+
+ ck_assert_int_eq(opt_llong_callback(&opt, "10", 0), 0);
+ ck_assert_int_eq(test_value, 10);
+ ck_assert_int_eq(opt_llong_callback(&opt, "100", 0), 0);
+ ck_assert_int_eq(test_value, 100);
+}
+END_TEST
+
+START_TEST(test_opt_int_callback_range_in)
+{
+ int test_value = 0;
+ const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000);
+
+ ck_assert_int_eq(opt_int_callback(&opt, "5000", 0), 0);
+ ck_assert_int_eq(test_value, 5000);
+}
+END_TEST
+
+START_TEST(test_opt_int_callback_range_below)
+{
+ int test_value = 0;
+ const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000);
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_int_callback(&opt, "-1", 0), -1);
+}
+END_TEST
+
+START_TEST(test_opt_int_callback_range_above)
+{
+ int test_value = 0;
+ const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000);
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_int_callback(&opt, "10001", 0), -1);
+}
+END_TEST
+
+START_TEST(test_opt_int_callback_range_boundary)
+{
+ int test_value = 0;
+ const struct option opt = TEST_INT_RANGE(&test_value, 0, 10000);
+
+ ck_assert_int_eq(opt_int_callback(&opt, "0", 0), 0);
+ ck_assert_int_eq(test_value, 0);
+ ck_assert_int_eq(opt_int_callback(&opt, "10000", 0), 0);
+ ck_assert_int_eq(test_value, 10000);
+}
+END_TEST
+
START_TEST(test_opt_cpus_cb)
{
struct common_params params = {0};
@@ -388,67 +478,6 @@ START_TEST(test_opt_osnoise_auto_cb_unset)
}
END_TEST
-START_TEST(test_opt_osnoise_period_cb)
-{
- unsigned long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb);
-
- ck_assert_int_eq(opt_osnoise_period_cb(&opt, "1000000", 0), 0);
- ck_assert_int_eq(period, 1000000);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_period_cb_invalid)
-{
- unsigned long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_osnoise_period_cb(&opt, "10000001", 0);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_period_cb_unset)
-{
- unsigned long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_osnoise_period_cb);
-
- ck_assert_int_eq(opt_osnoise_period_cb(&opt, "1000000", 0), 0);
- ck_assert_int_eq(opt_osnoise_period_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(period, 0);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_runtime_cb)
-{
- unsigned long long runtime = 0;
- const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb);
-
- ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, "900000", 0), 0);
- ck_assert_int_eq(runtime, 900000);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_runtime_cb_invalid)
-{
- unsigned long long runtime = 0;
- const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_osnoise_runtime_cb(&opt, "99", 0);
-}
-END_TEST
-
-START_TEST(test_opt_osnoise_runtime_cb_unset)
-{
- unsigned long long runtime = 0;
- const struct option opt = TEST_CALLBACK(&runtime, opt_osnoise_runtime_cb);
-
- ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, "900000", 0), 0);
- ck_assert_int_eq(opt_osnoise_runtime_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(runtime, 0);
-}
-END_TEST
START_TEST(test_opt_osnoise_trace_output_cb)
{
@@ -525,37 +554,6 @@ START_TEST(test_opt_osnoise_on_end_cb_invalid)
}
END_TEST
-START_TEST(test_opt_timerlat_period_cb)
-{
- long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb);
-
- ck_assert_int_eq(opt_timerlat_period_cb(&opt, "1000", 0), 0);
- ck_assert_int_eq(period, 1000);
-}
-END_TEST
-
-START_TEST(test_opt_timerlat_period_cb_invalid)
-{
- long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_timerlat_period_cb(&opt, "1000001", 0);
-}
-END_TEST
-
-START_TEST(test_opt_timerlat_period_cb_unset)
-{
- long long period = 0;
- const struct option opt = TEST_CALLBACK(&period, opt_timerlat_period_cb);
-
- ck_assert_int_eq(opt_timerlat_period_cb(&opt, "1000", 0), 0);
- ck_assert_int_eq(opt_timerlat_period_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(period, 0);
-}
-END_TEST
-
START_TEST(test_opt_timerlat_auto_cb)
{
struct timerlat_params params = {0};
@@ -585,46 +583,6 @@ START_TEST(test_opt_timerlat_auto_cb_unset)
}
END_TEST
-START_TEST(test_opt_dma_latency_cb)
-{
- int dma_latency = 0;
- const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb);
-
- ck_assert_int_eq(opt_dma_latency_cb(&opt, "1000", 0), 0);
- ck_assert_int_eq(dma_latency, 1000);
-}
-END_TEST
-
-START_TEST(test_opt_dma_latency_cb_min)
-{
- int dma_latency = 0;
- const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_dma_latency_cb(&opt, "-1", 0);
-}
-END_TEST
-
-START_TEST(test_opt_dma_latency_cb_max)
-{
- int dma_latency = 0;
- const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_dma_latency_cb(&opt, "10001", 0);
-}
-END_TEST
-
-START_TEST(test_opt_dma_latency_cb_unset)
-{
- int dma_latency = 0;
- const struct option opt = TEST_CALLBACK(&dma_latency, opt_dma_latency_cb);
-
- ck_assert_int_eq(opt_dma_latency_cb(&opt, "1000", 0), 0);
- ck_assert_int_eq(opt_dma_latency_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(dma_latency, default_dma_latency);
-}
-END_TEST
START_TEST(test_opt_aa_only_cb)
{
@@ -775,7 +733,8 @@ END_TEST
START_TEST(test_opt_timerlat_align_cb)
{
struct timerlat_params params = {0};
- const struct option opt = TEST_CALLBACK(¶ms, opt_timerlat_align_cb);
+ const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us",
+ "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX));
ck_assert_int_eq(opt_timerlat_align_cb(&opt, "500", 0), 0);
ck_assert(params.timerlat_align);
@@ -783,10 +742,22 @@ START_TEST(test_opt_timerlat_align_cb)
}
END_TEST
+START_TEST(test_opt_timerlat_align_cb_invalid)
+{
+ struct timerlat_params params = {0};
+ const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us",
+ "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX));
+
+ assert(freopen("/dev/null", "w", stderr));
+ ck_assert_int_eq(opt_timerlat_align_cb(&opt, "-1", 0), -1);
+}
+END_TEST
+
START_TEST(test_opt_timerlat_align_cb_unset)
{
struct timerlat_params params = {0};
- const struct option opt = TEST_CALLBACK(¶ms, opt_timerlat_align_cb);
+ const struct option opt = RTLA_OPT_CALLBACK_DATA('A', "aligned", ¶ms, "us",
+ "test", opt_timerlat_align_cb, LLONG_RANGE(0, LLONG_MAX));
ck_assert_int_eq(opt_timerlat_align_cb(&opt, "500", 0), 0);
ck_assert_int_eq(opt_timerlat_align_cb(&opt, NULL, 1), 0);
@@ -826,87 +797,6 @@ START_TEST(test_opt_stack_format_cb_unset)
}
END_TEST
-START_TEST(test_opt_bucket_size_cb)
-{
- int bucket_size = 0;
- const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb);
-
- ck_assert_int_eq(opt_bucket_size_cb(&opt, "100", 0), 0);
- ck_assert_int_eq(bucket_size, 100);
-}
-END_TEST
-
-START_TEST(test_opt_bucket_size_min)
-{
- int bucket_size = 0;
- const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_bucket_size_cb(&opt, "0", 0);
-}
-END_TEST
-
-START_TEST(test_opt_bucket_size_max)
-{
- int bucket_size = 0;
- const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_bucket_size_cb(&opt, "1000001", 0);
-}
-END_TEST
-
-START_TEST(test_opt_bucket_size_cb_unset)
-{
- int bucket_size = 0;
- const struct option opt = TEST_CALLBACK(&bucket_size, opt_bucket_size_cb);
-
- ck_assert_int_eq(opt_bucket_size_cb(&opt, "100", 0), 0);
- ck_assert_int_eq(opt_bucket_size_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(bucket_size, default_bucket_size);
-}
-END_TEST
-
-START_TEST(test_opt_entries_cb)
-{
- int entries = 0;
- const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb);
-
- ck_assert_int_eq(opt_entries_cb(&opt, "100", 0), 0);
- ck_assert_int_eq(entries, 100);
-}
-END_TEST
-
-START_TEST(test_opt_entries_min)
-{
- int entries = 0;
- const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_entries_cb(&opt, "9", 0);
-}
-END_TEST
-
-START_TEST(test_opt_entries_max)
-{
- int entries = 0;
- const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb);
-
- assert(freopen("/dev/null", "w", stderr));
- opt_entries_cb(&opt, "10000000", 0);
-}
-END_TEST
-
-START_TEST(test_opt_entries_cb_unset)
-{
- int entries = 0;
- const struct option opt = TEST_CALLBACK(&entries, opt_entries_cb);
-
- ck_assert_int_eq(opt_entries_cb(&opt, "100", 0), 0);
- ck_assert_int_eq(opt_entries_cb(&opt, NULL, 1), 0);
- ck_assert_int_eq(entries, default_entries);
-}
-END_TEST
Suite *cli_opt_callback_suite(void)
{
@@ -919,6 +809,10 @@ Suite *cli_opt_callback_suite(void)
tcase_add_test(tc, test_opt_llong_callback_min);
tcase_add_test(tc, test_opt_llong_callback_unset);
tcase_add_test(tc, test_opt_llong_callback_unset_defval);
+ tcase_add_test(tc, test_opt_llong_callback_range_in);
+ tcase_add_test(tc, test_opt_llong_callback_range_below);
+ tcase_add_test(tc, test_opt_llong_callback_range_above);
+ tcase_add_test(tc, test_opt_llong_callback_range_boundary);
tcase_add_test(tc, test_opt_int_callback_simple);
tcase_add_test(tc, test_opt_int_callback_max);
tcase_add_test(tc, test_opt_int_callback_min);
@@ -926,6 +820,10 @@ Suite *cli_opt_callback_suite(void)
tcase_add_test(tc, test_opt_int_callback_non_numeric_suffix);
tcase_add_test(tc, test_opt_int_callback_unset);
tcase_add_test(tc, test_opt_int_callback_unset_defval);
+ tcase_add_test(tc, test_opt_int_callback_range_in);
+ tcase_add_test(tc, test_opt_int_callback_range_below);
+ tcase_add_test(tc, test_opt_int_callback_range_above);
+ tcase_add_test(tc, test_opt_int_callback_range_boundary);
tcase_add_test(tc, test_opt_cpus_cb);
tcase_add_exit_test(tc, test_opt_cpus_cb_invalid, EXIT_FAILURE);
tcase_add_test(tc, test_opt_cgroup_cb);
@@ -951,12 +849,6 @@ Suite *cli_opt_callback_suite(void)
tc = tcase_create("osnoise");
tcase_add_test(tc, test_opt_osnoise_auto_cb);
tcase_add_test(tc, test_opt_osnoise_auto_cb_unset);
- tcase_add_test(tc, test_opt_osnoise_period_cb);
- tcase_add_test(tc, test_opt_osnoise_period_cb_unset);
- tcase_add_exit_test(tc, test_opt_osnoise_period_cb_invalid, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_osnoise_runtime_cb);
- tcase_add_exit_test(tc, test_opt_osnoise_runtime_cb_invalid, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_osnoise_runtime_cb_unset);
tcase_add_test(tc, test_opt_osnoise_trace_output_cb);
tcase_add_test(tc, test_opt_osnoise_trace_output_cb_noarg);
tcase_add_test(tc, test_opt_osnoise_trace_output_cb_unset);
@@ -967,15 +859,8 @@ Suite *cli_opt_callback_suite(void)
suite_add_tcase(s, tc);
tc = tcase_create("timerlat");
- tcase_add_test(tc, test_opt_timerlat_period_cb);
- tcase_add_exit_test(tc, test_opt_timerlat_period_cb_invalid, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_timerlat_period_cb_unset);
tcase_add_test(tc, test_opt_timerlat_auto_cb);
tcase_add_test(tc, test_opt_timerlat_auto_cb_unset);
- tcase_add_test(tc, test_opt_dma_latency_cb);
- tcase_add_exit_test(tc, test_opt_dma_latency_cb_min, EXIT_FAILURE);
- tcase_add_exit_test(tc, test_opt_dma_latency_cb_max, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_dma_latency_cb_unset);
tcase_add_test(tc, test_opt_aa_only_cb);
tcase_add_test(tc, test_opt_aa_only_cb_unset);
tcase_add_test(tc, test_opt_timerlat_trace_output_cb);
@@ -993,19 +878,9 @@ Suite *cli_opt_callback_suite(void)
tcase_add_exit_test(tc, test_opt_stack_format_cb_invalid, EXIT_FAILURE);
tcase_add_test(tc, test_opt_stack_format_cb_unset);
tcase_add_test(tc, test_opt_timerlat_align_cb);
+ tcase_add_test(tc, test_opt_timerlat_align_cb_invalid);
tcase_add_test(tc, test_opt_timerlat_align_cb_unset);
suite_add_tcase(s, tc);
- tc = tcase_create("histogram");
- tcase_add_test(tc, test_opt_bucket_size_cb);
- tcase_add_exit_test(tc, test_opt_bucket_size_min, EXIT_FAILURE);
- tcase_add_exit_test(tc, test_opt_bucket_size_max, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_bucket_size_cb_unset);
- tcase_add_test(tc, test_opt_entries_cb);
- tcase_add_exit_test(tc, test_opt_entries_min, EXIT_FAILURE);
- tcase_add_exit_test(tc, test_opt_entries_max, EXIT_FAILURE);
- tcase_add_test(tc, test_opt_entries_cb_unset);
- suite_add_tcase(s, tc);
-
return s;
}
--
2.55.0
^ permalink raw reply related
* Re: [PATCH 02/30] mm: add kdoc comments for vma_start/last_pgoff()
From: Lorenzo Stoakes @ 2026-07-10 13:38 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Marek Szyprowski, Peter Zijlstra,
Arnaldo Carvalho de Melo, Namhyung Kim, Masami Hiramatsu,
Oleg Nesterov, Steven Rostedt, SeongJae Park, Miaohe Lin,
Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <03762de9-c695-4cdd-8c25-4fc31335ea15@kernel.org>
On Tue, Jul 07, 2026 at 04:46:13PM +0200, David Hildenbrand (Arm) wrote:
> On 6/29/26 14:23, Lorenzo Stoakes wrote:
> > Describe what vma_start_pgoff() and vma_last_pgoff() actually provide in
> > detail.
> >
> > This is in order that we can differentiate this between functions that will
> > be added in a subsequent patch which provide a different page offset.
> >
> > No functional change intended.
> >
> > Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
> > ---
> > include/linux/mm.h | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 059144435729..2f00c75e66bd 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -4278,11 +4278,37 @@ static inline unsigned long vma_pages(const struct vm_area_struct *vma)
> > return (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> > }
> >
> > +/**
> > + * vma_start_pgoff() - Get the page offset of the start of @vma
> > + * @vma: The VMA whose page offset is required.
> > + *
> > + * If the VMA is file-backed, this is the page offset into the file.
> > + *
> > + * If the VMA is anonymous, this is the virtual page offset of the start of the
> > + * VMA - if unfaulted, then vma->vm_start >> PAGE_SHIFT, if faulted then the
> > + * virtual page offset at the time of first fault.
> > + *
> > + * Note that if @vma is a MAP_PRIVATE file-backed mapping, then this returns the
> > + * file offset.
>
> There is the ugly case of @vma being a MAP_PRIVATE PFNMAP mapping, where it
> returns something different.
Yup, the pfn I will update doc to mention this abomination too.
The virt pgoff stuff in the RFC fixes this also.
>
> (remap_pfn_range_prepare_vma() -> vma->vm_pgoff set to PFN)
>
> --
> Cheers,
>
> David
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v1 06/11] rcu: Enable RCU callbacks to benefit from expedited grace periods
From: Frederic Weisbecker @ 2026-07-10 13:58 UTC (permalink / raw)
To: Puranjay Mohan
Cc: rcu, linux-kernel, linux-trace-kernel, Paul E. McKenney,
Neeraj Upadhyay, Joel Fernandes, Josh Triplett, Boqun Feng,
Uladzislau Rezki, Steven Rostedt, Mathieu Desnoyers,
Lai Jiangshan, Zqiang, Masami Hiramatsu, Davidlohr Bueso,
Breno Leitao
In-Reply-To: <20260624132356.516959-7-puranjay@kernel.org>
Le Wed, Jun 24, 2026 at 06:23:48AM -0700, Puranjay Mohan a écrit :
> Currently, RCU callbacks only track normal grace-period sequence
> numbers. This means callbacks must wait for normal grace periods to
> complete even when expedited grace periods have already elapsed.
>
> Use the full struct rcu_gp_seq (which tracks both the normal and
> expedited grace-period sequences) throughout the callback
> infrastructure.
>
> rcu_segcblist_advance() now checks both normal and expedited GP
> completion via poll_state_synchronize_rcu_full(), and becomes
> parameterless since it reads the grace-period state internally.
> rcu_segcblist_accelerate() stores the full state (both sequences)
> instead of just the normal one. rcu_accelerate_cbs() and
> rcu_accelerate_cbs_unlocked() use get_state_synchronize_rcu_full() to
> capture both sequences, and the NOCB advance checks use
> poll_state_synchronize_rcu_full() instead of comparing only the normal
> sequence.
>
> srcu_segcblist_advance() becomes a standalone implementation because it
> compares SRCU sequences directly and cannot use
> poll_state_synchronize_rcu_full(), which reads RCU-specific globals.
> srcu_segcblist_accelerate() sets the ->exp field to
> RCU_GET_STATE_NOT_TRACKED so that poll_state_synchronize_rcu_full()
> compares only ->norm and ignores ->exp.
>
> Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
> ---
> kernel/rcu/rcu_segcblist.c | 30 +++++++++++++++++++++++-------
> kernel/rcu/rcu_segcblist.h | 2 +-
> kernel/rcu/tree.c | 9 +++------
> kernel/rcu/tree_nocb.h | 33 +++++++++++++++++++++++----------
> 4 files changed, 50 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/rcu/rcu_segcblist.c b/kernel/rcu/rcu_segcblist.c
> index 4e3dfe42bc097..cf8951d33e767 100644
> --- a/kernel/rcu/rcu_segcblist.c
> +++ b/kernel/rcu/rcu_segcblist.c
> @@ -12,6 +12,7 @@
> #include <linux/kernel.h>
> #include <linux/types.h>
>
> +#include "rcu.h"
> #include "rcu_segcblist.h"
>
> /* Initialize simple callback list. */
> @@ -494,9 +495,9 @@ static void rcu_segcblist_advance_compact(struct rcu_segcblist *rsclp, int i)
>
> /*
> * Advance the callbacks in the specified rcu_segcblist structure based
> - * on the current value passed in for the grace-period counter.
> + * on the current value of the grace-period counter.
> */
> -void rcu_segcblist_advance(struct rcu_segcblist *rsclp, struct rcu_gp_seq *gsp)
> +void rcu_segcblist_advance(struct rcu_segcblist *rsclp)
> {
> int i;
>
> @@ -509,7 +510,7 @@ void rcu_segcblist_advance(struct rcu_segcblist *rsclp, struct rcu_gp_seq *gsp)
> * are ready to invoke, and put them into the RCU_DONE_TAIL segment.
> */
> for (i = RCU_WAIT_TAIL; i < RCU_NEXT_TAIL; i++) {
> - if (ULONG_CMP_LT(gsp->norm, rsclp->gp_seq[i].norm))
> + if (!poll_state_synchronize_rcu_full(&rsclp->gp_seq[i]))
So after more careful review, the smp_mb() at the end of a successful
poll_state_synchronize_rcu_full() is necessary here because the current locking
is not enough to make sure we synchronize against the end of the grace period.
But what about the smp_mb() at the beginning? Paul what is the point of this one
already? It advertizes to pair with the smp_mb() on root cleanup but what
exactly is to be ordered here? Why does gp cleanup need to synchronize with
failing poll_state_synchronize_rcu_full() ? The smp_mb() before rcu_seq_snap()
in get_state_synchronize_rcu_full() should already synchronize the accesses
before that call against the beginning of the grace period.
If we keep all these barriers around and both RCU_WAIT_TAIL and RCU_NEXT_READY
need to be advanced, that makes 4 smp_mb() calls.
> @@ -637,14 +638,29 @@ void rcu_segcblist_merge(struct rcu_segcblist *dst_rsclp,
>
> void srcu_segcblist_advance(struct rcu_segcblist *rsclp, unsigned long seq)
> {
> - struct rcu_gp_seq gs = { .norm = seq };
> + int i;
> +
> + WARN_ON_ONCE(!rcu_segcblist_is_enabled(rsclp));
> + if (rcu_segcblist_restempty(rsclp, RCU_DONE_TAIL))
> + return;
> +
> + for (i = RCU_WAIT_TAIL; i < RCU_NEXT_TAIL; i++) {
> + if (ULONG_CMP_LT(seq, rsclp->gp_seq[i].norm))
> + break;
Why not use the same API here and consolidate the code? ->exp is RCU_GET_STATE_NOT_TRACKED so it's
harmless?
> @@ -1164,7 +1164,7 @@ static bool rcu_accelerate_cbs(struct rcu_node *rnp, struct rcu_data *rdp)
> * accelerating callback invocation to an earlier grace-period
> * number.
> */
> - gs.norm = rcu_seq_snap(&rcu_state.gp_seq);
> + get_state_synchronize_rcu_full(&gs);
I have similar concerns about the three smp_mb() in
get_state_synchronize_rcu_full(). It could be just two (rcu_seq_snap()
has a barrier that could be just one). Not sure if that matters but,
just wanted to point that.
Thanks.
--
Frederic Weisbecker
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/2] arm64: ftrace: support DIRECT_CALLS without CALL_OPS
From: Jose Fernandez (Anthropic) @ 2026-07-10 14:07 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Will Deacon, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, Puranjay Mohan, Clayton Craft, Xu Kuohai
Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, llvm, bpf,
Florent Revest, Xu Kuohai, Ben Cressey
In-Reply-To: <20260609-arm64-ftrace-direct-calls-v1-0-4a46f266697f@linux.dev>
Hi all,
Just a gentle ping on this series. It has Reviewed-by from Puranjay
Mohan, Acked-by from Xu Kuohai on both patches, and Tested-by from
Clayton Craft and Nathan Chancellor, with no outstanding comments since
Jun 11.
Could you please consider picking this up for the next cycle?
Thanks,
Jose
^ permalink raw reply
* Re: [PATCH 13/30] mm/vma: refactor vmg_adjust_set_range() for clarity
From: Lorenzo Stoakes @ 2026-07-10 14:50 UTC (permalink / raw)
To: Gregory Price
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <ak-70CHGujkI5jJI@gourry-fedora-PF4VCD3F>
On Thu, Jul 09, 2026 at 11:18:40AM -0400, Gregory Price wrote:
> On Mon, Jun 29, 2026 at 01:23:24PM +0100, Lorenzo Stoakes wrote:
> > Add comments with ASCII diagrams to describe what we're doing, avoid
> > dubious use of PHYS_PFN(), and use vma_start_pgoff().
> >
> > The most complicated scenario represented here is vmg->__adjust_next_start
> > - when this is set, vmg->[start, end] actually indicate the range to be
> > retained, so take special care to describe this accurately.
> >
> > No functional change intended.
> >
> > Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>
> Reviewed-by: Gregory Price <gourry@gourry.net>
>
> > + /*
> > + * vmg->start vmg->end
> > + * | |
> > + * v merge v
> > + * <------------->
> > + * delta
> > + * <------>
> > + * |------|----------------|
> > + * | prev | middle |
> > + * |------|----------------|
> > + * ^
> > + * |
> > + * middle->vm_start
> > + */
>
> Even with these diagrams, it's a bit difficult to understand what the
> actual intent/result of this chunk is (but that may be a limitation of
> me not spending enough time reading the surrounding code, not a comment
> of your work here).
Yeah I know, I do hope to refactor all this at some point to make it simpler,
but for the time being tried to make the confusing thing at least _slightly_
less confusing :)
>
> ~Gregory
Cheers, Lorenzo
>
> > + /*
> > + * Originally:
> > + *
> > + * vmg->start vmg->end
> > + * | |
> > + * v merge v
> > + * <------------>
> > + * . .
> > + * merge_existing_range() updates to:
> > + * . .
> > + * vmg->start vmg->end .
> > + * | | .
> > + * v retain v .
> > + * <----------> .
> > + * delta .
> > + * <-----> .
> > + * |----------------|------|
> > + * | middle | next |
> > + * |----------------|------|
> > + * ^
> > + * |
> > + * next->vm_start
> > + */
^ permalink raw reply
* Re: [PATCH 16/30] mm/vma: use vma_start_pgoff(), linear_page_index() in mm code
From: Lorenzo Stoakes @ 2026-07-10 14:52 UTC (permalink / raw)
To: Gregory Price
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <ak--doS80hnAeQ9s@gourry-fedora-PF4VCD3F>
On Thu, Jul 09, 2026 at 11:29:58AM -0400, Gregory Price wrote:
> On Mon, Jun 29, 2026 at 01:23:27PM +0100, Lorenzo Stoakes wrote:
> > There are many instances in which linear_page_index() (as well as
> > linear_page_delta()) is open-coded, which is confusing and inconsistent.
> >
> > Additionally, vma->vm_pgoff doesn't necessarily make it clear that this is
> > the page offset of the start of the VMA range.
> >
> > Doing so also aids greppability.
> >
> > So use vma_start_pgoff() in favour of directly accessing vma->vm_pgoff, and
> > linear_page_index() where we can.
> >
> > This also lays the ground for future changes which will add an anonymous
> > page offset in order to be able to index MAP_PRIVATE-file backed anon
> > folios in terms of their virtual page offset.
> >
> > No functional change intended.
> >
> > Signed-off-by: Lorenzo Stoakes <ljs@kernel.org>
>
> such a nice readability improvement, thank you.
>
> This made me wonder though whether we can make it painful for people to
> open-code this in the future, otherwise I fear another one of these
> patches creeping in 5-10 years down the line.
Yeah for sure, I wonder if we could somehow make some VMA fields private
honestly which would address a lot of this. I think there clever ways of making
certain things a gap in the struct outwardly that can do it.
But one for the future :)
>
> Not sure it's feasible though
>
> Reviewed-by: Gregory Price <gourry@gourry.net>
Thanks! :)
Cheers, Lorenzo
>
> > ---
> > include/linux/huge_mm.h | 1 +
> > include/linux/hugetlb.h | 3 +--
> > include/linux/pagemap.h | 2 +-
> > mm/damon/vaddr.c | 5 +++--
> > mm/debug.c | 2 +-
> > mm/filemap.c | 7 ++++---
> > mm/huge_memory.c | 2 +-
> > mm/hugetlb.c | 11 ++++-------
> > mm/internal.h | 24 ++++++++++++++----------
> > mm/khugepaged.c | 3 ++-
> > mm/madvise.c | 6 +++---
> > mm/mapping_dirty_helpers.c | 2 +-
> > mm/memory.c | 25 +++++++++++++------------
> > mm/mempolicy.c | 13 +++++++------
> > mm/mremap.c | 12 ++++--------
> > mm/msync.c | 4 ++--
> > mm/nommu.c | 7 ++++---
> > mm/pagewalk.c | 2 +-
> > mm/shmem.c | 9 +++++----
> > mm/userfaultfd.c | 4 ++--
> > mm/util.c | 4 ++--
> > mm/vma.c | 15 +++++++--------
> > mm/vma_exec.c | 4 ++--
> > mm/vma_init.c | 2 +-
> > 24 files changed, 86 insertions(+), 83 deletions(-)
> >
> > diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> > index ad20f7f8c179..653b81d08fe7 100644
> > --- a/include/linux/huge_mm.h
> > +++ b/include/linux/huge_mm.h
> > @@ -230,6 +230,7 @@ static inline bool thp_vma_suitable_order(struct vm_area_struct *vma,
> >
> > /* Don't have to check pgoff for anonymous vma */
> > if (!vma_is_anonymous(vma)) {
> > + /* vma_start_pgoff() in mm.h so not available. */
> > if (!IS_ALIGNED((vma->vm_start >> PAGE_SHIFT) - vma->vm_pgoff,
> > hpage_size >> PAGE_SHIFT))
> > return false;
> > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> > index 2abaf99321e9..8390f50604d6 100644
> > --- a/include/linux/hugetlb.h
> > +++ b/include/linux/hugetlb.h
> > @@ -792,8 +792,7 @@ static inline pgoff_t hugetlb_linear_page_index(struct vm_area_struct *vma,
> > {
> > struct hstate *h = hstate_vma(vma);
> >
> > - return ((address - vma->vm_start) >> huge_page_shift(h)) +
> > - (vma->vm_pgoff >> huge_page_order(h));
> > + return linear_page_index(vma, address) >> huge_page_order(h);
> > }
> >
> > static inline bool order_is_gigantic(unsigned int order)
> > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> > index 644c0f25ae73..68a88d34a468 100644
> > --- a/include/linux/pagemap.h
> > +++ b/include/linux/pagemap.h
> > @@ -1101,7 +1101,7 @@ static inline pgoff_t linear_page_index(const struct vm_area_struct *vma,
> > pgoff_t pgoff;
> >
> > pgoff = linear_page_delta(vma, address);
> > - pgoff += vma->vm_pgoff;
> > + pgoff += vma_start_pgoff(vma);
> > return pgoff;
> > }
> >
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index d27147603564..faa44aa3219b 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> > @@ -12,6 +12,7 @@
> > #include <linux/mman.h>
> > #include <linux/mmu_notifier.h>
> > #include <linux/page_idle.h>
> > +#include <linux/pagemap.h>
> > #include <linux/pagewalk.h>
> > #include <linux/sched/mm.h>
> >
> > @@ -627,8 +628,8 @@ static void damos_va_migrate_dests_add(struct folio *folio,
> > }
> >
> > order = folio_order(folio);
> > - ilx = vma->vm_pgoff >> order;
> > - ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> > + ilx = vma_start_pgoff(vma) >> order;
> > + ilx += linear_page_delta(vma, addr) >> order;
> >
> > for (i = 0; i < dests->nr_dests; i++)
> > weight_total += dests->weight_arr[i];
> > diff --git a/mm/debug.c b/mm/debug.c
> > index 77fa8fe1d641..497654b36f1a 100644
> > --- a/mm/debug.c
> > +++ b/mm/debug.c
> > @@ -163,7 +163,7 @@ void dump_vma(const struct vm_area_struct *vma)
> > "flags: %#lx(%pGv)\n",
> > vma, (void *)vma->vm_start, (void *)vma->vm_end, vma->vm_mm,
> > (unsigned long)pgprot_val(vma->vm_page_prot),
> > - vma->anon_vma, vma->vm_ops, vma->vm_pgoff,
> > + vma->anon_vma, vma->vm_ops, vma_start_pgoff(vma),
> > vma->vm_file, vma->vm_private_data,
> > #ifdef CONFIG_PER_VMA_LOCK
> > refcount_read(&vma->vm_refcnt),
> > diff --git a/mm/filemap.c b/mm/filemap.c
> > index 5af62e6abca5..bcb07b21a685 100644
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -3402,8 +3402,8 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
> > * of memory.
> > */
> > struct vm_area_struct *vma = vmf->vma;
> > - unsigned long start = vma->vm_pgoff;
> > - unsigned long end = start + vma_pages(vma);
> > + const unsigned long start = vma_start_pgoff(vma);
> > + const unsigned long end = vma_end_pgoff(vma);
> > unsigned long ra_end;
> >
> > ra->order = exec_folio_order();
> > @@ -3921,7 +3921,8 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf,
> > goto out;
> > }
> >
> > - addr = vma->vm_start + ((start_pgoff - vma->vm_pgoff) << PAGE_SHIFT);
> > + addr = vma->vm_start +
> > + ((start_pgoff - vma_start_pgoff(vma)) << PAGE_SHIFT);
> > vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, addr, &vmf->ptl);
> > if (!vmf->pte) {
> > folio_unlock(folio);
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 2bccb0a53a0a..e94f56487225 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -180,7 +180,7 @@ unsigned long __thp_vma_allowable_orders(struct vm_area_struct *vma,
> > */
> > if (!in_pf && shmem_file(vma->vm_file))
> > return orders & shmem_allowable_huge_orders(file_inode(vma->vm_file),
> > - vma, vma->vm_pgoff, 0,
> > + vma, vma_start_pgoff(vma), 0,
> > forced_collapse);
> >
> > if (!vma_is_anonymous(vma)) {
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index f45000149a78..d44a3ac5ee0a 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -1011,8 +1011,7 @@ static long region_count(struct resv_map *resv, long f, long t)
> > static pgoff_t vma_hugecache_offset(struct hstate *h,
> > struct vm_area_struct *vma, unsigned long address)
> > {
> > - return ((address - vma->vm_start) >> huge_page_shift(h)) +
> > - (vma->vm_pgoff >> huge_page_order(h));
> > + return linear_page_index(vma, address) >> huge_page_order(h);
> > }
> >
> > /*
> > @@ -5372,8 +5371,7 @@ static void unmap_ref_private(struct mm_struct *mm, struct vm_area_struct *vma,
> > * from page cache lookup which is in HPAGE_SIZE units.
> > */
> > address = address & huge_page_mask(h);
> > - pgoff = ((address - vma->vm_start) >> PAGE_SHIFT) +
> > - vma->vm_pgoff;
> > + pgoff = linear_page_index(vma, address);
> > mapping = vma->vm_file->f_mapping;
> >
> > /*
> > @@ -6771,7 +6769,7 @@ static unsigned long page_table_shareable(struct vm_area_struct *svma,
> > struct vm_area_struct *vma,
> > unsigned long addr, pgoff_t idx)
> > {
> > - unsigned long saddr = ((idx - svma->vm_pgoff) << PAGE_SHIFT) +
> > + unsigned long saddr = ((idx - vma_start_pgoff(svma)) << PAGE_SHIFT) +
> > svma->vm_start;
> > unsigned long sbase = saddr & PUD_MASK;
> > unsigned long s_end = sbase + PUD_SIZE;
> > @@ -6856,8 +6854,7 @@ pte_t *huge_pmd_share(struct mm_struct *mm, struct vm_area_struct *vma,
> > unsigned long addr, pud_t *pud)
> > {
> > struct address_space *mapping = vma->vm_file->f_mapping;
> > - pgoff_t idx = ((addr - vma->vm_start) >> PAGE_SHIFT) +
> > - vma->vm_pgoff;
> > + const pgoff_t idx = linear_page_index(vma, addr);
> > struct vm_area_struct *svma;
> > unsigned long saddr;
> > pte_t *spte = NULL;
> > diff --git a/mm/internal.h b/mm/internal.h
> > index 181e79f1d6a2..89e5b7efe256 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -1143,26 +1143,28 @@ static inline bool
> > folio_within_range(struct folio *folio, struct vm_area_struct *vma,
> > unsigned long start, unsigned long end)
> > {
> > - pgoff_t pgoff, addr;
> > - unsigned long vma_pglen = vma_pages(vma);
> > + const unsigned long vma_pglen = vma_pages(vma);
> > + pgoff_t pgoff_folio, pgoff_vma_start;
> > + unsigned long addr;
> >
> > VM_WARN_ON_FOLIO(folio_test_ksm(folio), folio);
> > if (start > end)
> > return false;
> >
> > + pgoff_folio = folio_pgoff(folio);
> > + pgoff_vma_start = vma_start_pgoff(vma);
> > +
> > if (start < vma->vm_start)
> > start = vma->vm_start;
> >
> > if (end > vma->vm_end)
> > end = vma->vm_end;
> >
> > - pgoff = folio_pgoff(folio);
> > -
> > /* if folio start address is not in vma range */
> > - if (!in_range(pgoff, vma->vm_pgoff, vma_pglen))
> > + if (!in_range(pgoff_folio, pgoff_vma_start, vma_pglen))
> > return false;
> >
> > - addr = vma->vm_start + ((pgoff - vma->vm_pgoff) << PAGE_SHIFT);
> > + addr = vma->vm_start + ((pgoff_folio - pgoff_vma_start) << PAGE_SHIFT);
> >
> > return !(addr < start || end - addr < folio_size(folio));
> > }
> > @@ -1234,15 +1236,16 @@ extern pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma);
> > static inline unsigned long vma_address(const struct vm_area_struct *vma,
> > pgoff_t pgoff, unsigned long nr_pages)
> > {
> > + const pgoff_t pgoff_start = vma_start_pgoff(vma);
> > unsigned long address;
> >
> > - if (pgoff >= vma->vm_pgoff) {
> > + if (pgoff >= pgoff_start) {
> > address = vma->vm_start +
> > - ((pgoff - vma->vm_pgoff) << PAGE_SHIFT);
> > + ((pgoff - pgoff_start) << PAGE_SHIFT);
> > /* Check for address beyond vma (or wrapped through 0?) */
> > if (address < vma->vm_start || address >= vma->vm_end)
> > address = -EFAULT;
> > - } else if (pgoff + nr_pages - 1 >= vma->vm_pgoff) {
> > + } else if (pgoff + nr_pages - 1 >= pgoff_start) {
> > /* Test above avoids possibility of wrap to 0 on 32-bit */
> > address = vma->vm_start;
> > } else {
> > @@ -1266,7 +1269,8 @@ static inline unsigned long vma_address_end(struct page_vma_mapped_walk *pvmw)
> > return pvmw->address + PAGE_SIZE;
> >
> > pgoff = pvmw->pgoff + pvmw->nr_pages;
> > - address = vma->vm_start + ((pgoff - vma->vm_pgoff) << PAGE_SHIFT);
> > + address = vma->vm_start +
> > + ((pgoff - vma_start_pgoff(vma)) << PAGE_SHIFT);
> > /* Check for address beyond vma (or wrapped through 0?) */
> > if (address < vma->vm_start || address > vma->vm_end)
> > address = vma->vm_end;
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index bd5f86cf4bd8..ffef738d826c 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -2145,7 +2145,8 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
> > spinlock_t *ptl;
> > bool success = false;
> >
> > - addr = vma->vm_start + ((pgoff - vma->vm_pgoff) << PAGE_SHIFT);
> > + addr = vma->vm_start +
> > + ((pgoff - vma_start_pgoff(vma)) << PAGE_SHIFT);
> > if (addr & ~HPAGE_PMD_MASK ||
> > vma->vm_end < addr + HPAGE_PMD_SIZE)
> > continue;
> > diff --git a/mm/madvise.c b/mm/madvise.c
> > index cd9bb077072c..6730c4200a93 100644
> > --- a/mm/madvise.c
> > +++ b/mm/madvise.c
> > @@ -253,7 +253,7 @@ static void shmem_swapin_range(struct vm_area_struct *vma,
> > continue;
> >
> > addr = vma->vm_start +
> > - ((xas.xa_index - vma->vm_pgoff) << PAGE_SHIFT);
> > + ((xas.xa_index - vma_start_pgoff(vma)) << PAGE_SHIFT);
> > xas_pause(&xas);
> > rcu_read_unlock();
> >
> > @@ -318,7 +318,7 @@ static long madvise_willneed(struct madvise_behavior *madv_behavior)
> > mark_mmap_lock_dropped(madv_behavior);
> > get_file(file);
> > offset = (loff_t)(start - vma->vm_start)
> > - + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
> > + + ((loff_t)vma_start_pgoff(vma) << PAGE_SHIFT);
> > mmap_read_unlock(mm);
> > vfs_fadvise(file, offset, end - start, POSIX_FADV_WILLNEED);
> > fput(file);
> > @@ -1023,7 +1023,7 @@ static long madvise_remove(struct madvise_behavior *madv_behavior)
> > return -EACCES;
> >
> > offset = (loff_t)(start - vma->vm_start)
> > - + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
> > + + ((loff_t)vma_start_pgoff(vma) << PAGE_SHIFT);
> >
> > /*
> > * Filesystem's fallocate may need to take i_rwsem. We need to
> > diff --git a/mm/mapping_dirty_helpers.c b/mm/mapping_dirty_helpers.c
> > index 737c407f4081..e0efa36e0a07 100644
> > --- a/mm/mapping_dirty_helpers.c
> > +++ b/mm/mapping_dirty_helpers.c
> > @@ -95,7 +95,7 @@ static int clean_record_pte(pte_t *pte, unsigned long addr,
> >
> > if (pte_dirty(ptent)) {
> > pgoff_t pgoff = ((addr - walk->vma->vm_start) >> PAGE_SHIFT) +
> > - walk->vma->vm_pgoff - cwalk->bitmap_pgoff;
> > + vma_start_pgoff(walk->vma) - cwalk->bitmap_pgoff;
> > pte_t old_pte = ptep_modify_prot_start(walk->vma, addr, pte);
> >
> > ptent = pte_mkclean(old_pte);
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 98c1a245f45a..f5eb06544ba4 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -725,10 +725,10 @@ static inline struct page *__vm_normal_page(struct vm_area_struct *vma,
> > if (!pfn_valid(pfn))
> > return NULL;
> > } else {
> > - unsigned long off = (addr - vma->vm_start) >> PAGE_SHIFT;
> > + const pgoff_t index = linear_page_index(vma, addr);
> >
> > /* Only CoW'ed anon folios are "normal". */
> > - if (pfn == vma->vm_pgoff + off)
> > + if (pfn == index)
> > return NULL;
> > if (!is_cow_mapping(vma->vm_flags))
> > return NULL;
> > @@ -2643,7 +2643,7 @@ static int __vm_map_pages(struct vm_area_struct *vma, struct page **pages,
> > int vm_map_pages(struct vm_area_struct *vma, struct page **pages,
> > unsigned long num)
> > {
> > - return __vm_map_pages(vma, pages, num, vma->vm_pgoff);
> > + return __vm_map_pages(vma, pages, num, vma_start_pgoff(vma));
> > }
> > EXPORT_SYMBOL(vm_map_pages);
> >
> > @@ -3298,7 +3298,8 @@ int vm_iomap_memory(struct vm_area_struct *vma, phys_addr_t start, unsigned long
> > unsigned long pfn;
> > int err;
> >
> > - err = __simple_ioremap_prep(vm_len, vma->vm_pgoff, start, len, &pfn);
> > + err = __simple_ioremap_prep(vm_len, vma_start_pgoff(vma), start, len,
> > + &pfn);
> > if (err)
> > return err;
> >
> > @@ -4342,15 +4343,15 @@ static inline void unmap_mapping_range_tree(struct address_space *mapping,
> > struct zap_details *details)
> > {
> > struct vm_area_struct *vma;
> > - unsigned long start, size;
> > struct mmu_gather tlb;
> >
> > mapping_interval_tree_foreach(vma, mapping, first_index, last_index) {
> > - const pgoff_t start_idx = max(first_index, vma->vm_pgoff);
> > + const pgoff_t start_idx = max(first_index, vma_start_pgoff(vma));
> > const pgoff_t end_idx = min(last_index, vma_last_pgoff(vma)) + 1;
> > -
> > - start = vma->vm_start + ((start_idx - vma->vm_pgoff) << PAGE_SHIFT);
> > - size = (end_idx - start_idx) << PAGE_SHIFT;
> > + const pgoff_t offset = start_idx - vma_start_pgoff(vma);
> > + const unsigned long offset_bytes = offset << PAGE_SHIFT;
> > + const unsigned long start = vma->vm_start + offset_bytes;
> > + const unsigned long size = (end_idx - start_idx) << PAGE_SHIFT;
> >
> > tlb_gather_mmu(&tlb, vma->vm_mm);
> > zap_vma_range_batched(&tlb, vma, start, size, details);
> > @@ -5684,7 +5685,7 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
> > } else if (nr_pages > 1) {
> > pgoff_t idx = folio_page_idx(folio, page);
> > /* The page offset of vmf->address within the VMA. */
> > - pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
> > + pgoff_t vma_off = vmf->pgoff - vma_start_pgoff(vmf->vma);
> > /* The index of the entry in the pagetable for fault page. */
> > pgoff_t pte_off = pte_index(vmf->address);
> >
> > @@ -5796,7 +5797,7 @@ static vm_fault_t do_fault_around(struct vm_fault *vmf)
> > pgoff_t nr_pages = READ_ONCE(fault_around_pages);
> > pgoff_t pte_off = pte_index(vmf->address);
> > /* The page offset of vmf->address within the VMA. */
> > - pgoff_t vma_off = vmf->pgoff - vmf->vma->vm_pgoff;
> > + pgoff_t vma_off = vmf->pgoff - vma_start_pgoff(vmf->vma);
> > pgoff_t from_pte, to_pte;
> > vm_fault_t ret;
> >
> > @@ -7274,7 +7275,7 @@ void print_vma_addr(char *prefix, unsigned long ip)
> > if (vma && vma->vm_file) {
> > struct file *f = vma->vm_file;
> > ip -= vma->vm_start;
> > - ip += vma->vm_pgoff << PAGE_SHIFT;
> > + ip += vma_start_pgoff(vma) << PAGE_SHIFT;
> > printk("%s%pD[%lx,%lx+%lx]", prefix, f, ip,
> > vma->vm_start,
> > vma->vm_end - vma->vm_start);
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index 36699fabd3c2..650cdb23354a 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -2048,8 +2048,8 @@ struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
> > pol = get_task_policy(current);
> > if (pol->mode == MPOL_INTERLEAVE ||
> > pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> > - *ilx += vma->vm_pgoff >> order;
> > - *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> > + *ilx += vma_start_pgoff(vma) >> order;
> > + *ilx += linear_page_delta(vma, addr) >> order;
> > }
> > return pol;
> > }
> > @@ -3250,16 +3250,17 @@ EXPORT_SYMBOL_FOR_MODULES(mpol_shared_policy_init, "kvm");
> > int mpol_set_shared_policy(struct shared_policy *sp,
> > struct vm_area_struct *vma, struct mempolicy *pol)
> > {
> > - int err;
> > + const pgoff_t pgoff = vma_start_pgoff(vma);
> > + const pgoff_t pgoff_end = vma_end_pgoff(vma);
> > struct sp_node *new = NULL;
> > - unsigned long sz = vma_pages(vma);
> > + int err;
> >
> > if (pol) {
> > - new = sp_alloc(vma->vm_pgoff, vma->vm_pgoff + sz, pol);
> > + new = sp_alloc(pgoff, pgoff_end, pol);
> > if (!new)
> > return -ENOMEM;
> > }
> > - err = shared_policy_replace(sp, vma->vm_pgoff, vma->vm_pgoff + sz, new);
> > + err = shared_policy_replace(sp, pgoff, pgoff_end, new);
> > if (err && new)
> > sp_free(new);
> > return err;
> > diff --git a/mm/mremap.c b/mm/mremap.c
> > index e9c8b1d05832..079a0ba0c4a7 100644
> > --- a/mm/mremap.c
> > +++ b/mm/mremap.c
> > @@ -948,8 +948,7 @@ static unsigned long vrm_set_new_addr(struct vma_remap_struct *vrm)
> > struct vm_area_struct *vma = vrm->vma;
> > unsigned long map_flags = 0;
> > /* Page Offset _into_ the VMA. */
> > - pgoff_t internal_pgoff = (vrm->addr - vma->vm_start) >> PAGE_SHIFT;
> > - pgoff_t pgoff = vma->vm_pgoff + internal_pgoff;
> > + const pgoff_t pgoff = linear_page_index(vma, vrm->addr);
> > unsigned long new_addr = vrm_implies_new_addr(vrm) ? vrm->new_addr : 0;
> > unsigned long res;
> >
> > @@ -1255,12 +1254,10 @@ static void unmap_source_vma(struct vma_remap_struct *vrm)
> > static int copy_vma_and_data(struct vma_remap_struct *vrm,
> > struct vm_area_struct **new_vma_ptr)
> > {
> > - unsigned long internal_offset = vrm->addr - vrm->vma->vm_start;
> > - unsigned long internal_pgoff = internal_offset >> PAGE_SHIFT;
> > - unsigned long new_pgoff = vrm->vma->vm_pgoff + internal_pgoff;
> > - unsigned long moved_len;
> > + const unsigned long new_pgoff = linear_page_index(vrm->vma, vrm->addr);
> > struct vm_area_struct *vma = vrm->vma;
> > struct vm_area_struct *new_vma;
> > + unsigned long moved_len;
> > int err = 0;
> > PAGETABLE_MOVE(pmc, NULL, NULL, vrm->addr, vrm->new_addr, vrm->old_len);
> >
> > @@ -1802,8 +1799,7 @@ static int check_prep_vma(struct vma_remap_struct *vrm)
> > vrm->populate_expand = true;
> >
> > /* Need to be careful about a growing mapping */
> > - pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
> > - pgoff += vma->vm_pgoff;
> > + pgoff = linear_page_index(vma, addr);
> > if (pgoff + (new_len >> PAGE_SHIFT) < pgoff)
> > return -EINVAL;
> >
> > diff --git a/mm/msync.c b/mm/msync.c
> > index ac4c9bfea2e7..90b491a27a14 100644
> > --- a/mm/msync.c
> > +++ b/mm/msync.c
> > @@ -12,6 +12,7 @@
> > #include <linux/mm.h>
> > #include <linux/mman.h>
> > #include <linux/file.h>
> > +#include <linux/pagemap.h>
> > #include <linux/syscalls.h>
> > #include <linux/sched.h>
> >
> > @@ -85,8 +86,7 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len, int, flags)
> > goto out_unlock;
> > }
> > file = vma->vm_file;
> > - fstart = (start - vma->vm_start) +
> > - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
> > + fstart = (loff_t)linear_page_index(vma, start) << PAGE_SHIFT;
> > fend = fstart + (min(end, vma->vm_end) - start) - 1;
> > start = vma->vm_end;
> > if ((flags & MS_SYNC) && file &&
> > diff --git a/mm/nommu.c b/mm/nommu.c
> > index 6d168f69763f..60560b2c457e 100644
> > --- a/mm/nommu.c
> > +++ b/mm/nommu.c
> > @@ -975,7 +975,7 @@ static int do_mmap_private(struct vm_area_struct *vma,
> > /* read the contents of a file into the copy */
> > loff_t fpos;
> >
> > - fpos = vma->vm_pgoff;
> > + fpos = vma_start_pgoff(vma);
> > fpos <<= PAGE_SHIFT;
> >
> > ret = kernel_read(vma->vm_file, base, len, &fpos);
> > @@ -1355,7 +1355,8 @@ static int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
> > delete_nommu_region(vma->vm_region);
> > if (new_below) {
> > vma->vm_region->vm_start = vma->vm_start = addr;
> > - vma->vm_region->vm_pgoff = vma->vm_pgoff += npages;
> > + vma->vm_pgoff += npages;
> > + vma->vm_region->vm_pgoff = vma_start_pgoff(vma);
> > } else {
> > vma->vm_region->vm_end = vma->vm_end = addr;
> > vma->vm_region->vm_top = addr;
> > @@ -1603,7 +1604,7 @@ int vm_iomap_memory(struct vm_area_struct *vma, phys_addr_t start, unsigned long
> > unsigned long pfn = start >> PAGE_SHIFT;
> > unsigned long vm_len = vma->vm_end - vma->vm_start;
> >
> > - pfn += vma->vm_pgoff;
> > + pfn += vma_start_pgoff(vma);
> > return io_remap_pfn_range(vma, vma->vm_start, pfn, vm_len, vma->vm_page_prot);
> > }
> > EXPORT_SYMBOL(vm_iomap_memory);
> > diff --git a/mm/pagewalk.c b/mm/pagewalk.c
> > index 98d090ede077..0a3bbff57d46 100644
> > --- a/mm/pagewalk.c
> > +++ b/mm/pagewalk.c
> > @@ -813,7 +813,7 @@ int walk_page_mapping(struct address_space *mapping, pgoff_t first_index,
> > mapping_interval_tree_foreach(vma, mapping, first_index,
> > first_index + nr - 1) {
> > /* Clip to the vma */
> > - vba = vma->vm_pgoff;
> > + vba = vma_start_pgoff(vma);
> > vea = vba + vma_pages(vma);
> > cba = first_index;
> > cba = max(cba, vba);
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index b51f83c970bb..4e7f6bc7a389 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -1032,6 +1032,8 @@ unsigned long shmem_swap_usage(struct vm_area_struct *vma)
> > struct inode *inode = file_inode(vma->vm_file);
> > struct shmem_inode_info *info = SHMEM_I(inode);
> > struct address_space *mapping = inode->i_mapping;
> > + const pgoff_t pgoff = vma_start_pgoff(vma);
> > + const pgoff_t pgoff_end = vma_end_pgoff(vma);
> > unsigned long swapped;
> >
> > /* Be careful as we don't hold info->lock */
> > @@ -1045,12 +1047,11 @@ unsigned long shmem_swap_usage(struct vm_area_struct *vma)
> > if (!swapped)
> > return 0;
> >
> > - if (!vma->vm_pgoff && vma->vm_end - vma->vm_start >= inode->i_size)
> > + if (!pgoff && vma->vm_end - vma->vm_start >= inode->i_size)
> > return swapped << PAGE_SHIFT;
> >
> > /* Here comes the more involved part */
> > - return shmem_partial_swap_usage(mapping, vma->vm_pgoff,
> > - vma->vm_pgoff + vma_pages(vma));
> > + return shmem_partial_swap_usage(mapping, pgoff, pgoff_end);
> > }
> >
> > /*
> > @@ -2839,7 +2840,7 @@ static struct mempolicy *shmem_get_policy(struct vm_area_struct *vma,
> > * by page order, as in shmem_get_pgoff_policy() and get_vma_policy()).
> > */
> > *ilx = inode->i_ino;
> > - index = ((addr - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
> > + index = linear_page_index(vma, addr);
> > return mpol_shared_policy_lookup(&SHMEM_I(inode)->policy, index);
> > }
> >
> > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> > index 246af12bf801..bf4518f4449d 100644
> > --- a/mm/userfaultfd.c
> > +++ b/mm/userfaultfd.c
> > @@ -481,7 +481,7 @@ static void mfill_retry_state_save(struct mfill_retry_state *s,
> > {
> > s->flags = vma_flags_and_mask(&vma->flags, MFILL_RETRY_STATE_VMA_FLAGS);
> > s->ops = vma_uffd_ops(vma);
> > - s->pgoff = vma->vm_pgoff;
> > + s->pgoff = vma_start_pgoff(vma);
> >
> > if (vma->vm_file)
> > s->file = get_file(vma->vm_file);
> > @@ -507,7 +507,7 @@ static bool mfill_retry_state_changed(struct mfill_retry_state *state,
> >
> > /* VMA was file backed, but file, inode or offset has changed */
> > if (!vma->vm_file || vma->vm_file->f_inode != state->file->f_inode ||
> > - state->file != vma->vm_file || vma->vm_pgoff != state->pgoff)
> > + state->file != vma->vm_file || vma_start_pgoff(vma) != state->pgoff)
> > return true;
> >
> > return false;
> > diff --git a/mm/util.c b/mm/util.c
> > index af2c2103f0d9..61e6d32b2c16 100644
> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -1188,7 +1188,7 @@ void compat_set_desc_from_vma(struct vm_area_desc *desc,
> > desc->start = vma->vm_start;
> > desc->end = vma->vm_end;
> >
> > - desc->pgoff = vma->vm_pgoff;
> > + desc->pgoff = vma_start_pgoff(vma);
> > desc->vm_file = vma->vm_file;
> > desc->vma_flags = vma->flags;
> > desc->page_prot = vma->vm_page_prot;
> > @@ -1379,7 +1379,7 @@ static int call_vma_mapped(struct vm_area_struct *vma)
> > if (!vm_ops || !vm_ops->mapped)
> > return 0;
> >
> > - err = vm_ops->mapped(vma->vm_start, vma->vm_end, vma->vm_pgoff,
> > + err = vm_ops->mapped(vma->vm_start, vma->vm_end, vma_start_pgoff(vma),
> > vma->vm_file, &vm_private_data);
> > if (err)
> > return err;
> > diff --git a/mm/vma.c b/mm/vma.c
> > index dc4c2c1077f4..ee3a8ca13d07 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -967,10 +967,9 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
> > * prev middle next
> > * extend delete delete
> > */
> > -
> > vmg->start = prev->vm_start;
> > vmg->end = next->vm_end;
> > - vmg->pgoff = prev->vm_pgoff;
> > + vmg->pgoff = vma_start_pgoff(prev);
> >
> > /*
> > * We already ensured anon_vma compatibility above, so now it's
> > @@ -987,9 +986,8 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
> > * prev middle
> > * extend shrink/delete
> > */
> > -
> > vmg->start = prev->vm_start;
> > - vmg->pgoff = prev->vm_pgoff;
> > + vmg->pgoff = vma_start_pgoff(prev);
> >
> > if (!vmg->__remove_middle)
> > vmg->__adjust_middle_start = true;
> > @@ -1011,13 +1009,13 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
> >
> > if (vmg->__remove_middle) {
> > vmg->end = next->vm_end;
> > - vmg->pgoff = next->vm_pgoff - pglen;
> > + vmg->pgoff = vma_start_pgoff(next) - pglen;
> > } else {
> > /* We shrink middle and expand next. */
> > vmg->__adjust_next_start = true;
> > vmg->start = middle->vm_start;
> > vmg->end = start;
> > - vmg->pgoff = middle->vm_pgoff;
> > + vmg->pgoff = vma_start_pgoff(middle);
> > }
> >
> > err = dup_anon_vma(next, middle, &anon_dup);
> > @@ -1126,7 +1124,7 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
> > if (can_merge_left) {
> > vmg->start = prev->vm_start;
> > vmg->target = prev;
> > - vmg->pgoff = prev->vm_pgoff;
> > + vmg->pgoff = vma_start_pgoff(prev);
> >
> > /*
> > * If this merge would result in removal of the next VMA but we
> > @@ -1957,7 +1955,8 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
> > VM_BUG_ON_VMA(faulted_in_anon_vma, new_vma);
> > *vmap = vma = new_vma;
> > }
> > - *need_rmap_locks = (new_vma->vm_pgoff <= vma->vm_pgoff);
> > + *need_rmap_locks =
> > + (vma_start_pgoff(new_vma) <= vma_start_pgoff(vma));
> > } else {
> > new_vma = vm_area_dup(vma);
> > if (!new_vma)
> > diff --git a/mm/vma_exec.c b/mm/vma_exec.c
> > index 5cee8b7efa0f..e3644a3042e2 100644
> > --- a/mm/vma_exec.c
> > +++ b/mm/vma_exec.c
> > @@ -37,7 +37,7 @@ int relocate_vma_down(struct vm_area_struct *vma, unsigned long shift)
> > unsigned long new_end = old_end - shift;
> > VMA_ITERATOR(vmi, mm, new_start);
> > VMG_STATE(vmg, mm, &vmi, new_start, old_end, EMPTY_VMA_FLAGS,
> > - vma->vm_pgoff);
> > + vma_start_pgoff(vma));
> > struct vm_area_struct *next;
> > struct mmu_gather tlb;
> > PAGETABLE_MOVE(pmc, vma, vma, old_start, new_start, length);
> > @@ -89,7 +89,7 @@ int relocate_vma_down(struct vm_area_struct *vma, unsigned long shift)
> >
> > vma_prev(&vmi);
> > /* Shrink the vma to just the new range */
> > - return vma_shrink(&vmi, vma, new_start, new_end, vma->vm_pgoff);
> > + return vma_shrink(&vmi, vma, new_start, new_end, vma_start_pgoff(vma));
> > }
> >
> > /*
> > diff --git a/mm/vma_init.c b/mm/vma_init.c
> > index 3c0b65950510..a459669a1654 100644
> > --- a/mm/vma_init.c
> > +++ b/mm/vma_init.c
> > @@ -46,7 +46,7 @@ static void vm_area_init_from(const struct vm_area_struct *src,
> > dest->vm_start = src->vm_start;
> > dest->vm_end = src->vm_end;
> > dest->anon_vma = src->anon_vma;
> > - dest->vm_pgoff = src->vm_pgoff;
> > + dest->vm_pgoff = vma_start_pgoff(src);
> > dest->vm_file = src->vm_file;
> > dest->vm_private_data = src->vm_private_data;
> > vm_flags_init(dest, src->vm_flags);
> > --
> > 2.54.0
> >
^ permalink raw reply
* Re: [PATCH 17/30] mm: prefer vma_[start,end]_pgoff() to vma->vm_pgoff in kernel/
From: Lorenzo Stoakes @ 2026-07-10 14:56 UTC (permalink / raw)
To: Gregory Price
Cc: Pedro Falcato, Andrew Morton, Russell King, Dinh Nguyen,
Simon Schuster, James E . J . Bottomley, Helge Deller,
Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Ian Abbott, H Hartley Sweeten, Lucas Stach,
David Airlie, Simona Vetter, Patrik Jakobsson, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Clark, Dmitry Baryshkov,
Tomi Valkeinen, Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Rik van Riel, Harry Yoo,
Jann Horn
In-Reply-To: <ak_C_o2ehS17Q5HV@gourry-fedora-PF4VCD3F>
On Thu, Jul 09, 2026 at 11:49:18AM -0400, Gregory Price wrote:
> On Thu, Jul 02, 2026 at 12:30:59PM +0100, Lorenzo Stoakes wrote:
> >
> ...
> > static inline unsigned long vma_offset(const struct vm_area_struct *vma,
> > const unsigned long address)
> > {
> > /* Retains page offset and tags. */
> > return address - vma->vm_start;
> > }
> >
> ...
> > And I'm not sure it's really all that useful. Perhaps retaining vma_offset()
> > would be though.
> >
>
> Silly question:
There's no such thing :)
>
> What's the purpose of retaining tags in a non-address value?
Well if you want to reconstitute an address from it later then that'd be the
intent, but I'm being hand wavey here for sure.
Main thing is to retain stuff under the page mask
>
> That sounds like there's fragility just waiting to be broken.
>
> (I presume you are talking about things like ARM MTE and such, right?)
>
> > This is one that I think makes more sense.
> >
> > But in general, I'd rather hold off from yet more churn here.
> >
> > I'm making these changes to establish a basis for virtual page offsets
> > introduced in [0], rather than just cleaning up in general.
> >
>
> I agree with this. If the refactors here suddenly have to think about
> corner cases on things like tags, that's better resolved separately.
Yeah this series is huge enough as it is :P
>
> ~Gregory
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 19/30] mm: use linear_page_[index, delta]() consistently
From: Lorenzo Stoakes @ 2026-07-10 15:01 UTC (permalink / raw)
To: Gregory Price
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Pedro Falcato, Rik van Riel,
Harry Yoo, Jann Horn
In-Reply-To: <ak_E1Y-D2Twykusr@gourry-fedora-PF4VCD3F>
On Thu, Jul 09, 2026 at 11:57:09AM -0400, Gregory Price wrote:
> On Thu, Jul 09, 2026 at 11:55:54AM -0400, Gregory Price wrote:
> > > @@ -2462,7 +2463,7 @@ static int comedi_vm_access(struct vm_area_struct *vma, unsigned long addr,
> > > {
> > > struct comedi_buf_map *bm = vma->vm_private_data;
> > > unsigned long offset =
> > > - addr - vma->vm_start + (vma->vm_pgoff << PAGE_SHIFT);
> > > + addr - vma->vm_start + (vma_start_pgoff(vma) << PAGE_SHIFT);
> > >
> >
> > Obviously correct, but was this intended for a different patch?
> >
> > ~Gregory
>
> bleh already caught, sorry for the noise :]
Haha no thanks for picking it up, many eyes bugs etc. etc. ;)
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 17/30] mm: prefer vma_[start,end]_pgoff() to vma->vm_pgoff in kernel/
From: Gregory Price @ 2026-07-10 15:09 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Pedro Falcato, Andrew Morton, Russell King, Dinh Nguyen,
Simon Schuster, James E . J . Bottomley, Helge Deller,
Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Ian Abbott, H Hartley Sweeten, Lucas Stach,
David Airlie, Simona Vetter, Patrik Jakobsson, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Clark, Dmitry Baryshkov,
Tomi Valkeinen, Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Rik van Riel, Harry Yoo,
Jann Horn
In-Reply-To: <alEHy1raICek7imv@lucifer>
On Fri, Jul 10, 2026 at 03:56:33PM +0100, Lorenzo Stoakes wrote:
> On Thu, Jul 09, 2026 at 11:49:18AM -0400, Gregory Price wrote:
> >
> > Silly question:
>
> There's no such thing :)
>
All my questions are silly. That's my secret :]
> >
> > What's the purpose of retaining tags in a non-address value?
>
> Well if you want to reconstitute an address from it later then that'd be the
> intent, but I'm being hand wavey here for sure.
>
> Main thing is to retain stuff under the page mask
>
Right, which ...
vvvvvv
> > That sounds like there's fragility just waiting to be broken.
If that's an actual issue, then our relationship with tags is painful :[
~Gregory
^ permalink raw reply
* Re: [PATCH 28/30] mm/vma: use guard clauses in can_vma_merge_[before, after]()
From: Lorenzo Stoakes @ 2026-07-10 15:14 UTC (permalink / raw)
To: Pedro Falcato
Cc: Andrew Morton, Russell King, Dinh Nguyen, Simon Schuster,
James E . J . Bottomley, Helge Deller, Jarkko Sakkinen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Ian Abbott, H Hartley Sweeten, Lucas Stach, David Airlie,
Simona Vetter, Patrik Jakobsson, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Clark, Dmitry Baryshkov, Tomi Valkeinen,
Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Rik van Riel, Harry Yoo,
Jann Horn
In-Reply-To: <akZOPG6fxr98Ycl_@pedro-suse.lan>
On Thu, Jul 02, 2026 at 12:41:51PM +0100, Pedro Falcato wrote:
> Looks nicer, thanks.
Thanks!
>
> Reviewed-by: Pedro Falcato <pfalcato@suse.de>
Also thanks! :)
>
> --
> Pedro
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 17/30] mm: prefer vma_[start,end]_pgoff() to vma->vm_pgoff in kernel/
From: Lorenzo Stoakes @ 2026-07-10 15:15 UTC (permalink / raw)
To: Gregory Price
Cc: Pedro Falcato, Andrew Morton, Russell King, Dinh Nguyen,
Simon Schuster, James E . J . Bottomley, Helge Deller,
Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, Ian Abbott, H Hartley Sweeten, Lucas Stach,
David Airlie, Simona Vetter, Patrik Jakobsson, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Rob Clark, Dmitry Baryshkov,
Tomi Valkeinen, Thierry Reding, Mikko Perttunen, Jonathan Hunter,
Christian Koenig, Huang Rui, Ankit Agrawal, Alex Williamson,
Alexander Viro, Christian Brauner, Dan Williams, Muchun Song,
Oscar Salvador, David Hildenbrand, Suren Baghdasaryan,
Liam R . Howlett, Matthew Wilcox, Marek Szyprowski,
Peter Zijlstra, Arnaldo Carvalho de Melo, Namhyung Kim,
Masami Hiramatsu, Oleg Nesterov, Steven Rostedt, SeongJae Park,
Miaohe Lin, Hugh Dickins, Mike Rapoport, Kees Cook, Paolo Bonzini,
linux-kernel, linux-arm-kernel, linux-parisc, linux-sgx, etnaviv,
dri-devel, linux-arm-msm, freedreno, linux-tegra, kvm,
linux-fsdevel, nvdimm, linux-mm, iommu, linux-perf-users,
linux-trace-kernel, kasan-dev, damon, Rik van Riel, Harry Yoo,
Jann Horn
In-Reply-To: <alELDgm258nhjk3b@gourry-fedora-PF4VCD3F>
On Fri, Jul 10, 2026 at 11:09:02AM -0400, Gregory Price wrote:
> On Fri, Jul 10, 2026 at 03:56:33PM +0100, Lorenzo Stoakes wrote:
> > On Thu, Jul 09, 2026 at 11:49:18AM -0400, Gregory Price wrote:
> > >
> > > Silly question:
> >
> > There's no such thing :)
> >
>
> All my questions are silly. That's my secret :]
Mine too ;)
>
> > >
> > > What's the purpose of retaining tags in a non-address value?
> >
> > Well if you want to reconstitute an address from it later then that'd be the
> > intent, but I'm being hand wavey here for sure.
> >
> > Main thing is to retain stuff under the page mask
> >
>
> Right, which ...
>
> vvvvvv
> > > That sounds like there's fragility just waiting to be broken.
>
> If that's an actual issue, then our relationship with tags is painful :[
Yeah, I'm just not sure tbh.
But all that is stuf for future masochism^Wfun anyway ;) focusing on the 'respin
the monster series' fun^Wmasochism atm...
>
> ~Gregory
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 0/2] arm64: ftrace: support DIRECT_CALLS without CALL_OPS
From: Steven Rostedt @ 2026-07-10 15:55 UTC (permalink / raw)
To: Jose Fernandez (Anthropic)
Cc: Masami Hiramatsu, Mark Rutland, Catalin Marinas, Will Deacon,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
Puranjay Mohan, Clayton Craft, Xu Kuohai, linux-kernel,
linux-trace-kernel, linux-arm-kernel, llvm, bpf, Florent Revest,
Xu Kuohai, Ben Cressey
In-Reply-To: <alD7Rr659mGVznQp@linux.dev>
On Fri, 10 Jul 2026 14:07:01 +0000
"Jose Fernandez (Anthropic)" <jose.fernandez@linux.dev> wrote:
> Hi all,
>
> Just a gentle ping on this series. It has Reviewed-by from Puranjay
> Mohan, Acked-by from Xu Kuohai on both patches, and Tested-by from
> Clayton Craft and Nathan Chancellor, with no outstanding comments since
> Jun 11.
>
> Could you please consider picking this up for the next cycle?
>
As this is arm64 specific, the decision lies with the arm64 maintainers.
-- Steve
^ permalink raw reply
* Re: [RFC PATCH 1/1] psi: Introduce in-kernel PSI auto monitor feature
From: Pintu Kumar Agarwal @ 2026-07-10 16:01 UTC (permalink / raw)
To: K Prateek Nayak
Cc: linux-kernel, linux-trace-kernel, hannes, surenb, rostedt,
mhiramat, peterz, mathieu.desnoyers, mingo, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
pintu.ping, nathan, ojeda, nsc, gary, tglx, thomas.weissschuh,
aliceryhl, dianders, linux.amoon, rdunlap, akpm, shuah
In-Reply-To: <CANqwyPjEAvxtupQK1YyhYE-2h=0==+FumCj48F4FDKCpCvWZWw@mail.gmail.com>
Hi All,
On Fri, Jul 3, 2026 at 9:02 PM Pintu Kumar Agarwal
<pintu.k@oss.qualcomm.com> wrote:
>
> Hi Prateek,
>
> Thank you so much for your review feedback and comments.
> Please find my response below.
>
> On Fri, Jul 3, 2026 at 1:21 AM K Prateek Nayak <kprateek.nayak@amd.com> wrote:
> >
> > Hello Pintu,
> >
> > On 7/2/2026 10:46 PM, Pintu Kumar Agarwal wrote:
> > > diff --git a/kernel/sched/build_utility.c b/kernel/sched/build_utility.c
> > > index e2cf3b08d4e9..30e9800ce947 100644
> > > --- a/kernel/sched/build_utility.c
> > > +++ b/kernel/sched/build_utility.c
> > > @@ -104,3 +104,7 @@
> > > #ifdef CONFIG_SCHED_AUTOGROUP
> > > # include "autogroup.c"
> > > #endif
> > > +
> > > +#ifdef CONFIG_PSI_AUTO_MONITOR
> > > +# include "psi_monitor.c"
> > > +#endif
> >
> > Isn't this a module? Why is this being included as a scheduler file?
> > Based on a quick glance, nothing in this module needs scheduler internal
> > APIs (and nor it should) so tools/sched/ would probabaly be a better
> > place to put it in if there is interest for this feature.
> >
> The scheduler placement was chosen because the feature currently
> operates on PSI internals
> and was developed as an extension to kernel/sched/psi.c.
> I am open to alternative placement if another location is more appropriate.
>
> > > diff --git a/kernel/sched/psi_monitor.c b/kernel/sched/psi_monitor.c
> > > new file mode 100644
> > > index 000000000000..e929a0c05494
> > > --- /dev/null
> > > +++ b/kernel/sched/psi_monitor.c
> > > @@ -0,0 +1,307 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * PSI Automatic Monitor with Weighted Task Ranking + Tracepoints
> > > + *
> > > + * Periodically samples system PSI (CPU, memory, IO) and, when any
> > > + * configured threshold is exceeded, ranks tasks using a composite
> > > + * score based on RSS, I/O activity and CPU time, then logs the
> > > + * top-N tasks via printk and a tracepoint.
> > > + *
> > > + * Sysfs interface:
> > > + * /sys/kernel/psi_monitor/cpu_thresh (percentage)
> > > + * /sys/kernel/psi_monitor/mem_thresh (percentage)
> > > + * /sys/kernel/psi_monitor/io_thresh (percentage)
> > > + * /sys/kernel/psi_monitor/monitor_interval_ms (milliseconds)
> > > + * /sys/kernel/psi_monitor/rss_weight
> > > + * /sys/kernel/psi_monitor/io_weight
> > > + * /sys/kernel/psi_monitor/cpu_weight
> > > + *
> > > + * Author: Pintu Kumar Agarwal
> > > + */
> > > +
> > > +#include <linux/init.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/sched.h>
> > > +#include <linux/sched/signal.h>
> > > +#include <linux/sched/loadavg.h>
> > > +#include <linux/mm.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/workqueue.h>
> > > +#include <linux/psi_types.h>
> > > +#include <linux/kobject.h>
> > > +#include <linux/sort.h>
> > > +#include <linux/jiffies.h>
> > > +#include <linux/time64.h>
> > > +#include <linux/sched/cputime.h>
> > > +
> > > +/* Create tracepoints defined in include/trace/events/psi_monitor.h */
> > > +#define CREATE_TRACE_POINTS
> > > +#include <linux/psi.h>
> > > +#include <trace/events/psi_monitor.h>
> > > +
> > > +
> > > +/* Sysfs tunables */
> > > +static unsigned int cpu_thresh = 80; /* in percent */
> > > +static unsigned int mem_thresh = 80; /* in percent */
> > > +static unsigned int io_thresh = 80; /* in percent */
> > > +static unsigned int monitor_interval_ms = 10000;
> > > +
> > > +/* scoring weights */
> > > +static unsigned int rss_weight = 2;
> > > +static unsigned int io_weight = 1;
> > > +static unsigned int cpu_weight = 5;
> >
> > Insanely configurable but what makes it easy for developers to know
> > the right configurations under severe pressure as you put it?
> >
> This is one of the goal of RFC to decide what parameters should be
> kept configurable
> and to what default values. Currently it is at experimental stage and
> gathering feedback.
> As per my experiments (2 core, < 1GB RAM), 80% number is good enough as default.
> This triggers only during extreme pressure, otherwise kept silent.
> Based on the scenario and workload, users can configure it.
> Run scenario => check workload => configure => rerun
> Scoring weight values are optional and it depends on which load we
> want to give priority.
> These are open for discussions and only needed for sorting logic.
>
> > > +
> > > +static struct delayed_work psi_work;
> > > +static struct kobject *psi_kobj;
> > > +
> > > +#define TOP_N 20
> > > +
> > > +struct task_info {
> > > + struct task_struct *task;
> > > + unsigned long rss; /* pages */
> > > + unsigned long io_kb; /* kB */
> > > + unsigned long cpu_ms; /* ms */
> >
> > Isn't the suffix selfexplanatory? Do you really need the comments?
> >
> oh yes, this can be removed if not needed.
>
> > > + u64 score;
> > > +};
> > > +
> > > +/*
> > > + * psi_avg10_percent() - derive a rough integer percentage from avg10
> > > + * for a given PSI state (e.g. PSI_CPU_SOME, PSI_MEM_SOME, PSI_IO_SOME).
> > > + *
> > > + * psi_group.avg[state][0] is the avg10 window in fixed-point notation.
> > > + * The conversion here is approximate but monotonic, which is sufficient
> > > + * for thresholding and ranking in this internal monitor.
> > > + */
> > > +static unsigned long psi_avg10_percent(int state)
> > > +{
> > > + u64 avg10;
> > > +
> > > + if (state < 0 || state >= NR_PSI_STATES)
> > > + return 0;
> > > +
> > > + avg10 = READ_ONCE(psi_system.avg[state][0]);
> > > + if (!avg10)
> > > + return 0;
> > > +
> > > + /* Convert back from loadavg-style fixed-point to an approximate % */
> > > + /* Just consider the integer value and ignore fraction */
> >
> > Why two single line comments?
> >
> ok I will merge it in the next version.
>
> > > + return LOAD_INT(avg10);
> > > +}
> > > +
> > > +static int compare_score_desc(const void *a, const void *b)
> > > +{
> > > + const struct task_info *ta = a;
> > > + const struct task_info *tb = b;
> > > +
> > > + if (tb->score > ta->score)
> > > + return 1;
> > > + if (tb->score < ta->score)
> > > + return -1;
> > > + return 0;
> > > +}
> > > +
> > > +static void log_top_tasks(void)
> > > +{
> > > + struct task_info tasks[TOP_N];
> > > + struct task_struct *p, *t;
> > > + int count = 0;
> > > + int i;
> > > +
> > > + rcu_read_lock();
> > > + for_each_process_thread(p, t) {
> >
> > Thats a ton of work every 10s.
> >
> This happens only when the threshold is breached and the system is
> already under pressure.
> Based on the feedback we can rate-limit this.
>
> > > + struct mm_struct *mm;
> > > + unsigned long rss = 0;
> > > + unsigned long io_kb = 0;
> > > + unsigned long cpu_ms = 0;
> > > + u64 score;
> > > +
> > > + /* Ignore tasks that are not on run queue or idle */
> > > + if (!t->on_rq && !is_idle_task(t))
> >
> > Condition doesn't match the comment. Tasks off rq that aren't idle will
> > still go through.
> >
> Oh yes, good catch. I will fix the comment in the next version.
>
> > > + continue;
> > > +
> > > + mm = get_task_mm(t);
> > > +
> > > + /* mm could be NULL for kernel threads */
> > > + if (mm) {
> > > + rss = mm ? get_mm_rss(mm) : 0;
> > > + mmput_async(mm);
> > > + }
> > > +
> > > + /*
> > > + * Approximate I/O activity: sum of read + write bytes.
> > > + * This uses the task_io_accounting fields in task_struct.
> > > + * Values are best-effort and need not be perfectly accurate
> > > + * for our ranking purpose.
> > > + */
> > > + io_kb = (t->ioac.read_bytes + t->ioac.write_bytes) >> 10;
> > > +
> > > + /*
> > > + * Approximate CPU usage via task_sched_runtime(), converted
> > > + * to milliseconds. This is cumulative since task start, but
> > > + * is still useful for comparing hotspots at a given point.
> > > + */
> > > + cpu_ms = (unsigned long)(task_sched_runtime(t) / NSEC_PER_MSEC);
> > > +
> > > + score = (u64)rss_weight * (u64)rss +
> > > + (u64)io_weight * (u64)io_kb +
> > > + (u64)cpu_weight * (u64)cpu_ms;
> > > +
> > > + if (count < TOP_N) {
> > > + tasks[count].task = t;
> > > + tasks[count].rss = rss;
> > > + tasks[count].io_kb = io_kb;
> > > + tasks[count].cpu_ms = cpu_ms;
> > > + tasks[count].score = score;
> > > + count++;
> > > + } else {
> > > + /* Maintain a simple streaming top-N: replace smallest */
> > > + int min_idx = 0;
> > > + int j;
> > > +
> > > + for (j = 1; j < TOP_N; j++) {
> > > + if (tasks[j].score < tasks[min_idx].score)
> > > + min_idx = j;
> > > + }
> >
> > Can't you just cache the min_idx and re-compute it when it changes
> > instead of taking a O(20) iteration for every task?
> >
> ok will think about it and come back.
> IMO the constant value may not affect the order.
> Currently at RFC stage I wanted to keep things as simple.
>
> > > +
> > > + if (score > tasks[min_idx].score) {
> > > + tasks[min_idx].task = t;
> > > + tasks[min_idx].rss = rss;
> > > + tasks[min_idx].io_kb = io_kb;
> > > + tasks[min_idx].cpu_ms = cpu_ms;
> > > + tasks[min_idx].score = score;
> > > + }
> > > + }
> > > + }
> > > + rcu_read_unlock();
> > > +
> > > + sort(tasks, count, sizeof(struct task_info), compare_score_desc, NULL);
> > > +
> > > + pr_info("psi_monitor: logging top %d tasks under pressure:\n", count);
> > > +
> > > + for (i = 0; i < count; i++) {
> > > + struct task_struct *ts = tasks[i].task;
> > > + unsigned long rss_kb = tasks[i].rss << (PAGE_SHIFT - 10);
> > > + char name[128] = {0,};
> > > +
> > > + if (ts->flags & PF_WQ_WORKER)
> > > + wq_worker_comm(name, sizeof(name), ts);
> > > + else
> > > + scnprintf(name, sizeof(name) - 1, ts->comm);
> > > +
> > > + trace_psi_monitor_top_task(ts->pid, name,
> > > + tasks[i].cpu_ms,
> > > + rss_kb,
> > > + tasks[i].io_kb,
> > > + tasks[i].score);
> > > +
> > > + pr_info("psi_monitor: pid=%d comm=%s psi_flag=%d oncpu=%d cputime(ms)=%lu rss(kB)=%lu io(kB)=%lu score=%llu\n",
> > > + ts->pid, name, ts->psi_flags, task_cpu(ts),
> > > + tasks[i].cpu_ms, rss_kb, tasks[i].io_kb,
> > > + (unsigned long long)tasks[i].score);
> >
> > This will unnecessarily dump to dmesg even if you have tracevent
> > enabled. Why?
> >
> This is also one point of discussion for RFC.
> Currently have kept both the options available and open for suggestion.
> The idea is to dump it like OOM message and only during pressure and
> threshold breach when we really need it.
> Once the pressure releases this will automatically stop.
> We can also make it as pr_debug or ratelimit or even put it under
> another CONFIG.
> The idea is to automatically get the information in logs instead of
> user intervention.
> But I am open for suggestions.
>
> > > + }
> > > +}
> > > +
> > > +static void psi_monitor_fn(struct work_struct *work)
> > > +{
> > > + unsigned long cpu_pct, mem_pct, io_pct;
> > > + bool trigger = false;
> > > +
> > > + cpu_pct = psi_avg10_percent(PSI_CPU_SOME);
> > > + mem_pct = psi_avg10_percent(PSI_MEM_SOME);
> > > + io_pct = psi_avg10_percent(PSI_IO_SOME);
> > > +
> > > + if (cpu_pct >= cpu_thresh || mem_pct >= mem_thresh ||
> > > + io_pct >= io_thresh)
> > > + trigger = true;
> > > +
> > > + if (trigger) {
> > > + pr_info("psi_monitor: pressure high: cpu=%lu%% mem=%lu%% io=%lu%% (thresh cpu=%u mem=%u io=%u)\n",
> > > + cpu_pct, mem_pct, io_pct,
> > > + cpu_thresh, mem_thresh, io_thresh);
> > > + log_top_tasks();
> > > + }
> > > +
> > > + queue_delayed_work(system_wq, &psi_work,
> > > + msecs_to_jiffies(monitor_interval_ms));
> >
> > If I set monitor_interval_ms to 6 hours, and then change it back to 10s,
> > it'll only take effect after this callback has fired 6 hours later.
> >
> Oh yes, good catch, I will fix this in the next version.
> So, it should override with the new request.
>
> > > +}
> > > +
> > > +/* Sysfs helpers */
> > > +#define PSI_ATTR_RW(_name) \
> > > +static ssize_t _name##_show(struct kobject *kobj, \
> > > + struct kobj_attribute *attr, char *buf) \
> > > +{ \
> > > + return sysfs_emit(buf, "%u\n", _name); \
> > > +} \
> > > +static ssize_t _name##_store(struct kobject *kobj, \
> > > + struct kobj_attribute *attr, \
> > > + const char *buf, size_t count) \
> > > +{ \
> > > + unsigned int val; \
> > > + if (kstrtouint(buf, 10, &val)) \
> > > + return -EINVAL; \
> > > + _name = val; \
> > > + return count; \
> > > +} \
> > > +static struct kobj_attribute _name##_attr = __ATTR_RW(_name)
> > > +
> > > +PSI_ATTR_RW(cpu_thresh);
> > > +PSI_ATTR_RW(mem_thresh);
> > > +PSI_ATTR_RW(io_thresh);
> > > +PSI_ATTR_RW(monitor_interval_ms);
> > > +PSI_ATTR_RW(rss_weight);
> > > +PSI_ATTR_RW(io_weight);
> > > +PSI_ATTR_RW(cpu_weight);
> > > +
> > > +static struct attribute *psi_attrs[] = {
> > > + &cpu_thresh_attr.attr,
> > > + &mem_thresh_attr.attr,
> > > + &io_thresh_attr.attr,
> > > + &monitor_interval_ms_attr.attr,
> > > + &rss_weight_attr.attr,
> > > + &io_weight_attr.attr,
> > > + &cpu_weight_attr.attr,
> > > + NULL,
> > > +};
> > > +
> > > +static const struct attribute_group psi_attr_group = {
> > > + .attrs = psi_attrs,
> > > +};
> > > +
> > > +static int __init psi_monitor_init(void)
> > > +{
> > > + int ret;
> > > +
> > > + INIT_DELAYED_WORK(&psi_work, psi_monitor_fn);
> > > + queue_delayed_work(system_wq, &psi_work,
> > > + msecs_to_jiffies(monitor_interval_ms));
> > > +
> > > + psi_kobj = kobject_create_and_add("psi_monitor", kernel_kobj);
> > > + if (!psi_kobj)
> > > + return -ENOMEM;
> > > +
> > > + ret = sysfs_create_group(psi_kobj, &psi_attr_group);
> > > + if (ret) {
> > > + kobject_put(psi_kobj);
> > > + cancel_delayed_work_sync(&psi_work);
> > > + return ret;
> > > + }
> > > +
> > > + pr_info("psi_monitor: in-kernel PSI auto monitor (weighted + tracepoints) loaded\n");
> > > + return 0;
> > > +}
> > > +
> > > +static void __exit psi_monitor_exit(void)
> > > +{
> > > + cancel_delayed_work_sync(&psi_work);
> > > + if (psi_kobj)
> > > + kobject_put(psi_kobj);
> > > + pr_info("psi_monitor: unloaded\n");
> > > +}
> > > +
> > > +module_init(psi_monitor_init);
> > > +module_exit(psi_monitor_exit);
> >
> > There is nothing here that warrants putting this in kernel/sched.
> The feature depends on sched/psi so I decided to keep it close.
> But I am open for any location.
>
> > Also this gets included by default when config is enabled and starts
> > dumping a bunch of stats to dmesg without anyone asking. No?
> >
> This is included as a dependent feature of PSI.
> If someone enables this CONFIG as part of PSI this indicates that they
> are interested in getting auto-monitor stats.
> Also, the dump will happen only if threshold is breached with high
> default values.
> However, for RFC stage I wanted to keep things simple.
> Later, we can add an enable/disable flag in cmdline just like PSI.
>
> > Afaict, almost all of the detail used here is also available from
> > procfs and people can easily put together a userspace tool if they
> > need it. Why do we need an in-kernel module?
> >
> This is the most fundamental aspect of this auto-monitor feature.
> This point is already described in the cover letter.
> Let me put it again:
> - Get kernel stats early during boot_time before userspace comes up.
> -> Set slightly lower threshold and boot stats (helps in analysing boot time)
> - No user intervention or continuous polling or daemons needed
> (Just enable config and start auto monitoring)
> - userspace scheduling delays under high pressure
> - risk of missing short-lived spikes
> - capturing details as soon as pressure hits and at same timestamp
> - useful for analysing real-time latency workload.
> - useful for minimal environment like initramfs or busybox
>
> The motivation is not to replace existing PSI interfaces or the ability
> to build userspace monitoring tools.
> The goal is attribution at the moment pressure thresholds are crossed.
> A userspace implementation observes the system after being scheduled,
> whereas the in-kernel implementation captures contributors at the point
> where pressure is detected.
> During LPC-2024 I have done significant changes to core psi module
> to implement the similar logic.
> But the feedback was not to disturb the core psi interface, instead
> develop a separate interface and make it configurable.
> So, I came up with this auto-monitor idea.
>
> For more details please have a look at my OSS paper with data.
> https://hosted-files.sched.co/ossindia2026/19/OSS-IND-26-PSI-Auto-Monitor.pdf
> And also the reference data here:
> https://github.com/pintuk/KERNEL/tree/master/PSI_WORK
>
> I am also looking out for someone who can test this on a larger
> workload and capture data.
> This will help us to gather insights, how the feature behaves.
>
> > > +
> > > +MODULE_LICENSE("GPL");
> > > +MODULE_AUTHOR("Pintu Kumar Agarwal");
> > > +MODULE_DESCRIPTION("In-kernel PSI automatic monitor with sysfs, weighted scoring and tracepoints");
> > > --
Any other feedback before I post v2 ?
Another thing that I wanted feedback is, whether to keep under
kernel/sched/ or move under tools/sched/ ?
Thanks,
Pintu
^ permalink raw reply
* [PATCH v6 0/2] Add tracepoints support for Qualcomm GENI Serial drivers
From: Praveen Talari @ 2026-07-10 17:12 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, konrad.dybcio
Cc: Praveen Talari, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
Add tracepoints to the Qualcomm GENI (Generic Interface) serial driver.
These trace events enable runtime debugging and performance analysis of
UART operations.
The trace events cover UART termios configuration, clock setup, manual
control state, interrupt status, and actual transmitted/received data in
hexadecimal format.
Usage examples:
Enable all serial traces:
echo 1 > /sys/kernel/debug/tracing/events/qcom_geni_serial/enable
cat /sys/kernel/debug/tracing/trace_pipe
Example trace output:
2517.938432: geni_serial_clk_cfg: a94000.serial: desired_rate=1843200
clk_rate=7372800 clk_div=4 clk_idx=0
2517.938753: geni_serial_irq: a94000.serial: m_irq=0x88800000
s_irq=0x08000111 dma_tx=0x00000000 dma_rx=0x00000000
2517.938803: geni_serial_set_termios: a94000.serial: baud=115200 bpc=8
tx_trans=0x00000002 tx_par=0x00000000 rx_trans=0x00000000
rx_par=0x00000000 stop=0
2517.938807: geni_serial_set_mctrl: a94000.serial: mctrl=0x8006
uart_manual_rfr=0x00000000
2517.938818: geni_serial_get_mctrl: a94000.serial: mctrl=0x0160
geni_ios=0x00000001
2517.939165: geni_serial_irq: a94000.serial: m_irq=0x00400000
s_irq=0x00000000 dma_tx=0x00000000 dma_rx=0x00000000
2517.939592: geni_serial_tx_data: a94000.serial: tx_len=8 data=61 62 63
64 65 66 67 68
2517.940610: geni_serial_irq: a94000.serial: m_irq=0x00000001
s_irq=0x00000000 dma_tx=0x00000003 dma_rx=0x00000000
2517.942174: geni_serial_irq: a94000.serial: m_irq=0x08000000
s_irq=0x08000100 dma_tx=0x00000000 dma_rx=0x00000003
2517.942323: geni_serial_rx_data: a94000.serial: rx_len=8 data=61 62 63
64 65 66 67 68
2517.942680: geni_serial_set_mctrl: a94000.serial: mctrl=0x8000
uart_manual_rfr=0x80000002
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Changes in v6:
- Rebased patch(02/02) on latest linux-next.
- Link to v5: https://lore.kernel.org/all/20260615-add-tracepoints-for-qcom-geni-serial-v5-0-2efa4c97e0e2@oss.qualcomm.com/
Changes in v5:
- Remove serial trace header patch since it was accepted and merged.
- Added a new patch to fix for remove unwanted variable len.
- Link to v4: https://lore.kernel.org/all/20260526-add-tracepoints-for-qcom-geni-serial-v4-0-e94fbaec0232@oss.qualcomm.com
Changes in v4:
- Rebased patch(02/02) on latest linux-next.
- Link to v3: https://lore.kernel.org/all/20260518-add-tracepoints-for-qcom-geni-serial-v3-0-b4addb151376@oss.qualcomm.com
Changes in v3:
- Removed \n from geni_serial_tx_data and geni_serial_rx_data events.
- Resolved aligment issues in geni_serial_data, geni_serial_tx_data and
geni_serial_rx_data events.
- Link to v2: https://lore.kernel.org/r/20260512-add-tracepoints-for-qcom-geni-serial-v2-0-a5726421b3af@oss.qualcomm.com
Changes in v2:
- removed multiple trace events for TX/RX events, instead used
DECLARE_EVENT_CLASS and DEFINE_EVENT.
- Link to v1: https://lore.kernel.org/r/20260506-add-tracepoints-for-qcom-geni-serial-v1-0-544b22612e08@oss.qualcomm.com
To: Steven Rostedt <rostedt@goodmis.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jiri Slaby <jirislaby@kernel.org>
To: konrad.dybcio@oss.qualcomm.com
Cc: linux-kernel@vger.kernel.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: mukesh.savaliya@oss.qualcomm.com
Cc: aniket.randive@oss.qualcomm.com
Cc: chandana.chiluveru@oss.qualcomm.com
---
Praveen Talari (2):
serial: qcom-geni: trace: Drop redundant len field from geni_serial_data
serial: qcom-geni: Add tracepoints for Qualcomm GENI serial driver
drivers/tty/serial/qcom_geni_serial.c | 27 +++++++++++++++++++++++----
include/trace/events/qcom_geni_serial.h | 7 +++----
2 files changed, 26 insertions(+), 8 deletions(-)
---
base-commit: bee763d5f341b99cf472afeb508d4988f62a6ca1
change-id: 20260427-add-tracepoints-for-qcom-geni-serial-948777218b7b
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v6 1/2] serial: qcom-geni: trace: Drop redundant len field from geni_serial_data
From: Praveen Talari @ 2026-07-10 17:12 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Greg Kroah-Hartman, Jiri Slaby, konrad.dybcio
Cc: Praveen Talari, linux-kernel, linux-trace-kernel, linux-arm-msm,
linux-serial, mukesh.savaliya, aniket.randive, chandana.chiluveru
In-Reply-To: <20260710-add-tracepoints-for-qcom-geni-serial-v6-0-2bb6b6836dfd@oss.qualcomm.com>
The dynamic array stored in the ring buffer already carries its own
length in the array metadata. There is no need to also store it as a
separate scalar field in the entry struct.
Drop __field(unsigned int, len) and the corresponding __entry->len
assignment, and use __get_dynamic_array_len(data) in the TP_printk for
both the len=%u format argument and the __print_hex() size argument.
This saves 4 bytes per event on the ring buffer.
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
include/trace/events/qcom_geni_serial.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/trace/events/qcom_geni_serial.h b/include/trace/events/qcom_geni_serial.h
index 417ec01f9fc8..e1aa551d525e 100644
--- a/include/trace/events/qcom_geni_serial.h
+++ b/include/trace/events/qcom_geni_serial.h
@@ -97,18 +97,17 @@ DECLARE_EVENT_CLASS(geni_serial_data,
TP_ARGS(dev, buf, len),
TP_STRUCT__entry(__string(name, dev_name(dev))
- __field(unsigned int, len)
__dynamic_array(u8, data, len)
),
TP_fast_assign(__assign_str(name);
- __entry->len = len;
memcpy(__get_dynamic_array(data), buf, len);
),
TP_printk("%s: len=%u data=%s",
- __get_str(name), __entry->len,
- __print_hex(__get_dynamic_array(data), __entry->len))
+ __get_str(name), __get_dynamic_array_len(data),
+ __print_hex(__get_dynamic_array(data),
+ __get_dynamic_array_len(data)))
);
DEFINE_EVENT(geni_serial_data, geni_serial_tx_data,
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox