* Re: [PATCH 1/4] mm: introduce zone lock wrappers
From: Shakeel Butt @ 2026-02-24 15:30 UTC (permalink / raw)
To: Dmitry Ilvokhin
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Johannes Weiner, Zi Yan, Oscar Salvador, Qi Zheng, Axel Rasmussen,
Yuanchu Xie, Wei Xu, linux-kernel, linux-mm, linux-trace-kernel,
linux-cxl, kernel-team
In-Reply-To: <aZ3BLKzhIIZvkbwL@shell.ilvokhin.com>
On Tue, Feb 24, 2026 at 03:18:04PM +0000, Dmitry Ilvokhin wrote:
> On Mon, Feb 23, 2026 at 02:36:01PM -0800, Shakeel Butt wrote:
> > On Wed, Feb 11, 2026 at 03:22:13PM +0000, Dmitry Ilvokhin wrote:
> > > Add thin wrappers around zone lock acquire/release operations. This
> > > prepares the code for future tracepoint instrumentation without
> > > modifying individual call sites.
> > >
> > > Centralizing zone lock operations behind wrappers allows future
> > > instrumentation or debugging hooks to be added without touching
> > > all users.
> > >
> > > No functional change intended. The wrappers are introduced in
> > > preparation for subsequent patches and are not yet used.
> > >
> > > Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> > > ---
> > > MAINTAINERS | 1 +
> > > include/linux/zone_lock.h | 38 ++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 39 insertions(+)
> > > create mode 100644 include/linux/zone_lock.h
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index b4088f7290be..680c9ae02d7e 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -16498,6 +16498,7 @@ F: include/linux/pgtable.h
> > > F: include/linux/ptdump.h
> > > F: include/linux/vmpressure.h
> > > F: include/linux/vmstat.h
> > > +F: include/linux/zone_lock.h
> > > F: kernel/fork.c
> > > F: mm/Kconfig
> > > F: mm/debug.c
> > > diff --git a/include/linux/zone_lock.h b/include/linux/zone_lock.h
> > > new file mode 100644
> > > index 000000000000..c531e26280e6
> > > --- /dev/null
> > > +++ b/include/linux/zone_lock.h
> > > @@ -0,0 +1,38 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +#ifndef _LINUX_ZONE_LOCK_H
> > > +#define _LINUX_ZONE_LOCK_H
> > > +
> > > +#include <linux/mmzone.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +static inline void zone_lock_init(struct zone *zone)
> > > +{
> > > + spin_lock_init(&zone->lock);
> > > +}
> > > +
> > > +#define zone_lock_irqsave(zone, flags) \
> > > +do { \
> > > + spin_lock_irqsave(&(zone)->lock, flags); \
> > > +} while (0)
> > > +
> > > +#define zone_trylock_irqsave(zone, flags) \
> > > +({ \
> > > + spin_trylock_irqsave(&(zone)->lock, flags); \
> > > +})
> >
> > Any reason you used macros for above two and inlined functions for remaining?
> >
>
> The reason for using macros in those two cases is that they need to
> modify the flags variable passed by the caller, just like
> spin_lock_irqsave() and spin_trylock_irqsave() do. I followed the same
> convention here.
>
> If we used normal inline functions instead, we would need to pass a
> pointer to flags, which would change the call sites and diverge from the
> existing *_irqsave() locking pattern.
>
> There is also a difference between zone_lock_irqsave() and
> zone_trylock_irqsave() implementations: the former is implemented as a
> do { } while (0) macro since it does not return a value, while the
> latter uses a GCC extension in order to return the trylock result. This
> matches spin_lock_* convention as well.
>
Cool, thanks for the explanation.
^ permalink raw reply
* Re: [PATCH v4 1/3] PCI: trace: Add PCI controller LTSSM transition tracepoint
From: Ilpo Järvinen @ 2026-02-24 15:22 UTC (permalink / raw)
To: Shawn Lin
Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Steven Rostedt, Masami Hiramatsu
In-Reply-To: <1769047340-113287-2-git-send-email-shawn.lin@rock-chips.com>
On Thu, 22 Jan 2026, Shawn Lin wrote:
> Some platforms may provide LTSSM trace functionality, recording historical
> LTSSM state transition information. This is very useful for debugging, such
> as when certain devices cannot be recognized or link broken during test.
> Implement the pci controller tracepoint for recording LTSSM and rate.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
> Changes in v4:
> - use TRACE_EVENT_FN to notify when to start and stop the tracepoint,
> and export pci_ltssm_tp_enabled() for host drivers to use
>
> Changes in v3:
> - add TRACE_DEFINE_ENUM for all enums(Steven Rostedt)
>
> Changes in v2: None
>
> drivers/pci/trace.c | 20 ++++++++++++
> include/linux/pci.h | 4 +++
> include/trace/events/pci_controller.h | 57 +++++++++++++++++++++++++++++++++++
> 3 files changed, 81 insertions(+)
> create mode 100644 include/trace/events/pci_controller.h
>
> diff --git a/drivers/pci/trace.c b/drivers/pci/trace.c
> index cf11abc..d351a51 100644
> --- a/drivers/pci/trace.c
> +++ b/drivers/pci/trace.c
> @@ -9,3 +9,23 @@
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/pci.h>
> +#include <trace/events/pci_controller.h>
> +
> +static atomic_t pcie_ltssm_tp_enabled = ATOMIC_INIT(0);
> +
> +bool pci_ltssm_tp_enabled(void)
> +{
> + return atomic_read(&pcie_ltssm_tp_enabled) > 0;
> +}
> +EXPORT_SYMBOL(pci_ltssm_tp_enabled);
> +
> +int pci_ltssm_tp_reg(void)
> +{
> + atomic_inc(&pcie_ltssm_tp_enabled);
> + return 0;
> +}
> +
> +void pci_ltssm_tp_unreg(void)
> +{
> + atomic_dec(&pcie_ltssm_tp_enabled);
> +}
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e7cb527..ac25a3e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2770,6 +2770,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
> }
> #endif
>
> +#ifdef CONFIG_TRACING
> +bool pci_ltssm_tp_enabled(void);
> +#endif
> +
> void pci_add_dma_alias(struct pci_dev *dev, u8 devfn_from, unsigned nr_devfns);
> bool pci_devs_are_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
> int pci_for_each_dma_alias(struct pci_dev *pdev,
> diff --git a/include/trace/events/pci_controller.h b/include/trace/events/pci_controller.h
> new file mode 100644
> index 0000000..db4a960
> --- /dev/null
> +++ b/include/trace/events/pci_controller.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM pci_controller
I find putting this into "pci_controller" little odd as LTSSM is related
to PCIe links/ports. To me looks something that belongs to the existing
include/trace/events/pci.h.
> +#if !defined(_TRACE_HW_EVENT_PCI_CONTROLLER_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_HW_EVENT_PCI_CONTROLLER_H
> +
> +#include <uapi/linux/pci_regs.h>
> +#include <linux/tracepoint.h>
> +
> +TRACE_DEFINE_ENUM(PCIE_SPEED_2_5GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_5_0GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_8_0GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_16_0GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_32_0GT);
> +TRACE_DEFINE_ENUM(PCIE_SPEED_64_0GT);
> +TRACE_DEFINE_ENUM(PCI_SPEED_UNKNOWN);
> +
> +extern int pci_ltssm_tp_reg(void);
> +extern void pci_ltssm_tp_unreg(void);
> +
> +TRACE_EVENT_FN(pcie_ltssm_state_transition,
> + TP_PROTO(const char *dev_name, const char *state, u32 rate),
> + TP_ARGS(dev_name, state, rate),
> +
> + TP_STRUCT__entry(
> + __string(dev_name, dev_name)
> + __string(state, state)
> + __field(u32, rate)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(dev_name);
> + __assign_str(state);
> + __entry->rate = rate;
> + ),
> +
> + TP_printk("dev: %s state: %s rate: %s",
> + __get_str(dev_name), __get_str(state),
> + __print_symbolic(__entry->rate,
> + { PCIE_SPEED_2_5GT, "2.5 GT/s" },
> + { PCIE_SPEED_5_0GT, "5.0 GT/s" },
> + { PCIE_SPEED_8_0GT, "8.0 GT/s" },
> + { PCIE_SPEED_16_0GT, "16.0 GT/s" },
> + { PCIE_SPEED_32_0GT, "32.0 GT/s" },
> + { PCIE_SPEED_64_0GT, "64.0 GT/s" },
> + { PCI_SPEED_UNKNOWN, "Unknown" }
Why are these done inline instead of using EM/EMe()? Or simply with
pci_speed_string()?
Unrelated to this, sadly I failed to notice Shuai's version of
pcie_link_event() did not translate link speeds (my own version used
pci_speed_string()).
> + )
> + ),
> +
> + pci_ltssm_tp_reg, pci_ltssm_tp_unreg
> +);
> +
> +#endif /* _TRACE_HW_EVENT_PCI_CONTROLLER_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
>
--
i.
^ permalink raw reply
* Re: [PATCH 1/4] mm: introduce zone lock wrappers
From: Dmitry Ilvokhin @ 2026-02-24 15:18 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Brendan Jackman,
Johannes Weiner, Zi Yan, Oscar Salvador, Qi Zheng, Axel Rasmussen,
Yuanchu Xie, Wei Xu, linux-kernel, linux-mm, linux-trace-kernel,
linux-cxl, kernel-team
In-Reply-To: <aZzWNM06gNYKDoQW@linux.dev>
On Mon, Feb 23, 2026 at 02:36:01PM -0800, Shakeel Butt wrote:
> On Wed, Feb 11, 2026 at 03:22:13PM +0000, Dmitry Ilvokhin wrote:
> > Add thin wrappers around zone lock acquire/release operations. This
> > prepares the code for future tracepoint instrumentation without
> > modifying individual call sites.
> >
> > Centralizing zone lock operations behind wrappers allows future
> > instrumentation or debugging hooks to be added without touching
> > all users.
> >
> > No functional change intended. The wrappers are introduced in
> > preparation for subsequent patches and are not yet used.
> >
> > Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
> > ---
> > MAINTAINERS | 1 +
> > include/linux/zone_lock.h | 38 ++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 39 insertions(+)
> > create mode 100644 include/linux/zone_lock.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b4088f7290be..680c9ae02d7e 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -16498,6 +16498,7 @@ F: include/linux/pgtable.h
> > F: include/linux/ptdump.h
> > F: include/linux/vmpressure.h
> > F: include/linux/vmstat.h
> > +F: include/linux/zone_lock.h
> > F: kernel/fork.c
> > F: mm/Kconfig
> > F: mm/debug.c
> > diff --git a/include/linux/zone_lock.h b/include/linux/zone_lock.h
> > new file mode 100644
> > index 000000000000..c531e26280e6
> > --- /dev/null
> > +++ b/include/linux/zone_lock.h
> > @@ -0,0 +1,38 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _LINUX_ZONE_LOCK_H
> > +#define _LINUX_ZONE_LOCK_H
> > +
> > +#include <linux/mmzone.h>
> > +#include <linux/spinlock.h>
> > +
> > +static inline void zone_lock_init(struct zone *zone)
> > +{
> > + spin_lock_init(&zone->lock);
> > +}
> > +
> > +#define zone_lock_irqsave(zone, flags) \
> > +do { \
> > + spin_lock_irqsave(&(zone)->lock, flags); \
> > +} while (0)
> > +
> > +#define zone_trylock_irqsave(zone, flags) \
> > +({ \
> > + spin_trylock_irqsave(&(zone)->lock, flags); \
> > +})
>
> Any reason you used macros for above two and inlined functions for remaining?
>
The reason for using macros in those two cases is that they need to
modify the flags variable passed by the caller, just like
spin_lock_irqsave() and spin_trylock_irqsave() do. I followed the same
convention here.
If we used normal inline functions instead, we would need to pass a
pointer to flags, which would change the call sites and diverge from the
existing *_irqsave() locking pattern.
There is also a difference between zone_lock_irqsave() and
zone_trylock_irqsave() implementations: the former is implemented as a
do { } while (0) macro since it does not return a value, while the
latter uses a GCC extension in order to return the trylock result. This
matches spin_lock_* convention as well.
> > +
> > +static inline void zone_unlock_irqrestore(struct zone *zone, unsigned long flags)
> > +{
> > + spin_unlock_irqrestore(&zone->lock, flags);
> > +}
> > +
> > +static inline void zone_lock_irq(struct zone *zone)
> > +{
> > + spin_lock_irq(&zone->lock);
> > +}
> > +
> > +static inline void zone_unlock_irq(struct zone *zone)
> > +{
> > + spin_unlock_irq(&zone->lock);
> > +}
> > +
> > +#endif /* _LINUX_ZONE_LOCK_H */
> > --
> > 2.47.3
> >
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Gregory Price @ 2026-02-24 15:17 UTC (permalink / raw)
To: Alistair Popple
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <fzy6f6dpv3oq3ksr2mkst7pz3daeb3buhuvdvcw4633pcl7h6u@mxjgiwpg5acv>
On Tue, Feb 24, 2026 at 05:19:11PM +1100, Alistair Popple wrote:
> On 2026-02-22 at 19:48 +1100, Gregory Price <gourry@gourry.net> wrote...
>
> Based on our discussion at LPC I believe one of the primary motivators here was
> to re-use the existing mm buddy allocator rather than writing your own. I remain
> to be convinced that alone is justification enough for doing all this - DRM for
> example already has quite a nice standalone buddy allocator (drm_buddy.c) that
> could presumably be used, or adapted for use, by any device driver.
>
> The interesting part of this series (which I have skimmed but not read in
> detail) is how device memory gets exposed to userspace - this is something that
> existing ZONE_DEVICE implementations don't address, instead leaving it up to
> drivers and associated userspace stacks to deal with allocation, migration, etc.
>
I agree that buddy-access alone is insufficient justification, it
started off that way - but if you want mempolicy/NUMA UAPI access,
it turns into "Re-use all of MM" - and that means using the buddy.
I also expected ZONE_DEVICE vs NODE_DATA to be the primary discussion,
I raise replacing it as a thought experiment, but not the proposal.
The idea that drm/ is going to switch to private nodes is outside the
realm of reality, but part of that is because of years of infrastructure
built on the assumption that re-using mm/ is infeasible.
But, lets talk about DEVICE_COHERENT
---
DEVICE_COHERENT is the odd-man out among ZONE_DEVICE modes. The others
use softleaf entries and don't allow direct mappings.
(DEVICE_PRIVATE sort of does if you squint, but you can also view that
a bit like PROT_NONE or read-only controls to force migrations).
If you take DEVICE_COHERENT and:
- Move pgmap out of the struct page (page_ext, NODE_DATA, etc) to free
the LRU list_head
- Put pages in the buddy (free lists, watermarks, managed_pages) or add
pgmap->device_alloc() at every allocation callsite / buddy hook
- Add LRU support (aging, reclaim, compaction)
- Add isolated gating (new GFP flag and adjusted zonelist filtering)
- Add new dev_pagemap_ops callbacks for the various mm/ features
- Audit evey folio_is_zone_device() to distinguish zone device modes
... you've built N_MEMORY_PRIVATE inside ZONE_DEVICE. Except now
page_zone(page) returns ZONE_DEVICE - so you inherit the wrong
defaults at every existing ZONE_DEVICE check.
Skip-sites become things to opt-out of instead of opting into.
You just end up with
if (folio_is_zone_device(folio))
if (folio_is_my_special_zone_device())
else ....
and this just generalizes to
if (folio_is_private_managed(folio))
folio_managed_my_hooked_operation()
So you get the same code, but have added more complexity to ZONE_DEVICE.
I don't think that's needed if we just recognize ZONE is the wrong
abstraction to be operating on.
Honestly, even ZONE_MOVABLE becomes pointless with N_MEMORY_PRIVATE
if you disallow longterm pinning - because the managing service handles
allocations (it has to inject GFP_PRIVATE to get access) or selectively
enables the mm/ services it knows are safe (mempolicy).
Even if you allow longterm pinning, if your service controls what does
the pinning it can still be reclaimable - just manually (killing
processes) instead of letting hotplug do it via migration.
If your service only allocates movable pages - your ZONE_NORMAL is
effectively ZONE_MOVABLE.
In some cases we use ZONE_MOVABLE to prevent the kernel from allocating
memory onto devices (like CXL). This means struct page is forced to
take up DRAM or use memmap_on_memory - meaning you lose high-value
capacity or sacrifice contiguity (less huge page support).
This entire problem can evaporate if you can just use ZONE_NORMAL.
There are a lot of benefits to just re-using the buddy like this.
Zones are the wrong abstraction and cause more problems.
> > free_folio - mirrors ZONE_DEVICE's
> > folio_split - mirrors ZONE_DEVICE's
> > migrate_to - ... same as ZONE_DEVICE
> > handle_fault - mirrors the ZONE_DEVICE ...
> > memory_failure - parallels memory_failure_dev_pagemap(),
>
> One does not have to squint too hard to see that the above is not so different
> from what ZONE_DEVICE provides today via dev_pagemap_ops(). So I think I think
> it would be worth outlining why the existing ZONE_DEVICE mechanism can't be
> extended to provide these kind of services.
>
> This seems to add a bunch of code just to use NODE_DATA instead of page->pgmap,
> without really explaining why just extending dev_pagemap_ops wouldn't work. The
> obvious reason is that if you want to support things like reclaim, compaction,
> etc. these pages need to be on the LRU, which is a little bit hard when that
> field is also used by the pgmap pointer for ZONE_DEVICE pages.
>
You don't have to squint because it was deliberate :]
The callback similarity is the feature - they're the same logical
operations. The difference is the direction of the defaults.
Extending ZONE_DEVICE into these areas requires the same set of hooks,
plus distinguishing "old ZONE_DEVICE" from "new ZONE_DEVICE".
Where there are new injection sites, it's because ZONE_DEVICE opts
out of ever touching that code in some other silently implied way.
For example, reclaim/compaction doesn't run because ZONE_DEVICE doesn't
add to managed_pages (among other reasons).
You'd have to go figure out how to hack those things into ZONE_DEVICE
*and then* opt every *other* ZONE_DEVICE mode *back out*.
So you still end up with something like this anyway:
static inline bool folio_managed_handle_fault(struct folio *folio,
struct vm_fault *vmf,
enum pgtable_level level,
vm_fault_t *ret)
{
/* Zone device pages use swap entries; handled in do_swap_page */
if (folio_is_zone_device(folio))
return false;
if (folio_is_private_node(folio))
...
return false;
}
> example page_ext could be used. Or I hear struct page may go away in place of
> folios any day now, so maybe that gives us space for both :-)
>
If NUMA is the interface we want, then NODE_DATA is the right direction
regardless of struct page's future or what zone it lives in.
There's no reason to keep per-page pgmap w/ device-to-node mappings.
You can have one driver manage multiple devices with the same numa node
if it uses the same owner context (PFN already differentiates devices).
The existing code allows for this.
> The above also looks pretty similar to the existing ZONE_DEVICE methods for
> doing this which is another reason to argue for just building up the feature set
> of the existing boondoggle rather than adding another thingymebob.
>
> It seems the key thing we are looking for is:
>
> 1) A userspace API to allocate/manage device memory (ie. move_pages(), mbind(),
> etc.)
>
> 2) Allowing reclaim/LRU list processing of device memory.
>
> From my perspective both of these are interesting and I look forward to the
> discussion (hopefully I can make it to LSFMM). Mostly I'm interested in the
> implementation as this does on the surface seem to sprinkle around and duplicate
> a lot of hooks similar to what ZONE_DEVICE already provides.
>
On (1): ZONE_DEVICE NUMA UAPI is harder than it looks from the surface
Much of the kernel mm/ infrastructure is written on top of the buddy and
expects N_MEMORY to be the sole arbiter of "Where to Acquire Pages".
Mempolicy depends on:
- Buddy support or a new alloc hook around the buddy
- Migration support (mbind() after allocation migrates)
- Migration also deeply assumes buddy and LRU support
- Changing validations on node states
- mempolicy checks N_MEMORY membership, so you have to hack
N_MEMORY onto ZONE_DEVICE
(or teach it about a new node state... N_MEMORY_PRIVATE)
Getting mempolicy to work with N_MEMORY_PRIVATE amounts to adding 2
lines of code in vma_alloc_folio_noprof:
struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order,
struct vm_area_struct *vma,
unsigned long addr)
{
if (pol->flags & MPOL_F_PRIVATE)
gfp |= __GFP_PRIVATE;
folio = folio_alloc_mpol_noprof(gfp, order, pol, ilx, numa_node_id());
/* Woo! I faulted a DEVICE PAGE! */
}
But this requires the pages to be managed by the buddy.
The rest of the mempolicy support is around keeping sane nodemasks when
things like cpuset.mems rebinds occur and validating you don't end up
with private nodes that don't support mempolicy in your nodemask.
You have to do all of this anyway, but with the added bonus of fighting
with the overloaded nature of ZONE_DEVICE at every step.
==========
On (2): Assume you solve LRU.
Zone Device has no free lists, managed_pages, or watermarks.
kswapd can't run, compaction has no targets, vmscan's pressure model
doesn't function. These all come for free when the pages are
buddy-managed on a real zone. Why re-invent the wheel?
==========
So you really have two options here:
a) Put pages in the buddy, or
b) Add pgmap->device_alloc() callbacks at every allocation site that
could target a node:
- vma_alloc_folio
- alloc_migration_target
- alloc_demote_folio
- alloc_pages_node
- alloc_contig_pages
- list goes on
Or more likely - hooking get_page_from_freelist. Which at that
point... just use the buddy? You're already deep in the hot path.
>
> For basic allocation I agree this is the case. But there's no reason some device
> allocator library couldn't be written. Or in fact as pointed out above reuse the
> already existing one in drm_buddy.c. So would be interested to hear arguments
> for why allocation has to be done by the mm allocator and/or why an allocation
> library wouldn't work here given DRM already has them.
>
Using the buddy underpins the rest of mm/ services we want to re-use.
That's basically it. Otherwise you have to inject hooks into every
surface that touches the buddy...
... or in the buddy (get_page_from_freelist), at which point why not
just use the buddy?
~Gregory
^ permalink raw reply
* Re: [PATCH v4 3/3] PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support
From: Steven Rostedt @ 2026-02-24 14:16 UTC (permalink / raw)
To: Shawn Lin
Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Masami Hiramatsu
In-Reply-To: <20260224091115.6e32c38e@fedora>
On Tue, 24 Feb 2026 09:11:15 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> > +#ifdef CONFIG_TRACING
> > +static void rockchip_pcie_ltssm_trace_work(struct work_struct *work)
> > +{
> > + struct rockchip_pcie *rockchip = container_of(work,
> > + struct rockchip_pcie,
> > + trace_work.work);
> > + struct dw_pcie *pci = &rockchip->pci;
> > + enum dw_pcie_ltssm state;
> > + u32 i, l1ss, prev_val = DW_PCIE_LTSSM_UNKNOWN, rate, val;
> > +
> > + if (!pci_ltssm_tp_enabled())
> > + goto skip_trace;
>
> You can use:
>
> if (!trace_pcie_ltssm_state_transition_enabled())
> goto skip_trace;
>
> The above is a static branch. That means when tracing is disabled, it is
> basically:
>
> goto skip_trace;
>
> and when tracing is enabled it is a nop.
>
> -- Steve
>
>
> > +
> > + for (i = 0; i < PCIE_DBG_LTSSM_HISTORY_CNT; i++) {
> > + val = rockchip_pcie_readl_apb(rockchip,
> > + PCIE_CLIENT_DBG_FIFO_STATUS);
> > + rate = FIELD_GET(PCIE_DBG_FIFO_RATE_MASK, val);
> > + l1ss = FIELD_GET(PCIE_DBG_FIFO_L1SUB_MASK, val);
> > + val = FIELD_GET(PCIE_LTSSM_STATUS_MASK, val);
> > +
> > + /*
> > + * Hardware Mechanism: The ring FIFO employs two tracking
> > + * counters:
> > + * - 'last-read-point': maintains the user's last read position
> > + * - 'last-valid-point': tracks the HW's last state update
> > + *
> > + * Software Handling: When two consecutive LTSSM states are
> > + * identical, it indicates invalid subsequent data in the FIFO.
> > + * In this case, we skip the remaining entries. The dual counter
> > + * design ensures that on the next state transition, reading can
> > + * resume from the last user position.
> > + */
> > + if ((i > 0 && val == prev_val) || val > DW_PCIE_LTSSM_RCVRY_EQ3)
> > + break;
> > +
> > + state = prev_val = val;
> > + if (val == DW_PCIE_LTSSM_L1_IDLE) {
> > + if (l1ss == 2)
> > + state = DW_PCIE_LTSSM_L1_2;
> > + else if (l1ss == 1)
> > + state = DW_PCIE_LTSSM_L1_1;
> > + }
> > +
> > + trace_pcie_ltssm_state_transition(dev_name(pci->dev),
> > + dw_pcie_ltssm_status_string(state),
> > + ((rate + 1) > pci->max_link_speed) ?
> > + PCI_SPEED_UNKNOWN : PCIE_SPEED_2_5GT + rate);
> > + }
> > +
> > +skip_trace:
> > + schedule_delayed_work(&rockchip->trace_work, msecs_to_jiffies(5000));
> > +}
> > +
Hmm, so basically you only want to call the work when tracing is
enabled? That's what I was thinking should be enabled by the reg and
unreg functions. That is, the reg should enabled the delayed work, and
the unreg should disable it from being called.
This looks like it calls the work regardless of if tracing is enabled
or not. Why waste the cycles when tracing is disabled?
-- Steve
^ permalink raw reply
* Re: [PATCH v4 3/3] PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support
From: Steven Rostedt @ 2026-02-24 14:11 UTC (permalink / raw)
To: Shawn Lin
Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Masami Hiramatsu
In-Reply-To: <1769047340-113287-4-git-send-email-shawn.lin@rock-chips.com>
On Thu, 22 Jan 2026 10:02:20 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> +#ifdef CONFIG_TRACING
> +static void rockchip_pcie_ltssm_trace_work(struct work_struct *work)
> +{
> + struct rockchip_pcie *rockchip = container_of(work,
> + struct rockchip_pcie,
> + trace_work.work);
> + struct dw_pcie *pci = &rockchip->pci;
> + enum dw_pcie_ltssm state;
> + u32 i, l1ss, prev_val = DW_PCIE_LTSSM_UNKNOWN, rate, val;
> +
> + if (!pci_ltssm_tp_enabled())
> + goto skip_trace;
You can use:
if (!trace_pcie_ltssm_state_transition_enabled())
goto skip_trace;
The above is a static branch. That means when tracing is disabled, it is
basically:
goto skip_trace;
and when tracing is enabled it is a nop.
-- Steve
> +
> + for (i = 0; i < PCIE_DBG_LTSSM_HISTORY_CNT; i++) {
> + val = rockchip_pcie_readl_apb(rockchip,
> + PCIE_CLIENT_DBG_FIFO_STATUS);
> + rate = FIELD_GET(PCIE_DBG_FIFO_RATE_MASK, val);
> + l1ss = FIELD_GET(PCIE_DBG_FIFO_L1SUB_MASK, val);
> + val = FIELD_GET(PCIE_LTSSM_STATUS_MASK, val);
> +
> + /*
> + * Hardware Mechanism: The ring FIFO employs two tracking
> + * counters:
> + * - 'last-read-point': maintains the user's last read position
> + * - 'last-valid-point': tracks the HW's last state update
> + *
> + * Software Handling: When two consecutive LTSSM states are
> + * identical, it indicates invalid subsequent data in the FIFO.
> + * In this case, we skip the remaining entries. The dual counter
> + * design ensures that on the next state transition, reading can
> + * resume from the last user position.
> + */
> + if ((i > 0 && val == prev_val) || val > DW_PCIE_LTSSM_RCVRY_EQ3)
> + break;
> +
> + state = prev_val = val;
> + if (val == DW_PCIE_LTSSM_L1_IDLE) {
> + if (l1ss == 2)
> + state = DW_PCIE_LTSSM_L1_2;
> + else if (l1ss == 1)
> + state = DW_PCIE_LTSSM_L1_1;
> + }
> +
> + trace_pcie_ltssm_state_transition(dev_name(pci->dev),
> + dw_pcie_ltssm_status_string(state),
> + ((rate + 1) > pci->max_link_speed) ?
> + PCI_SPEED_UNKNOWN : PCIE_SPEED_2_5GT + rate);
> + }
> +
> +skip_trace:
> + schedule_delayed_work(&rockchip->trace_work, msecs_to_jiffies(5000));
> +}
> +
> +static void rockchip_pcie_ltssm_trace(struct rockchip_pcie *rockchip,
> + bool enable)
> +{
> + if (enable) {
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_TRANSITION_DATA,
> + PCIE_CLIENT_DBG_FIFO_PTN_HIT_D0);
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_TRANSITION_DATA,
> + PCIE_CLIENT_DBG_FIFO_PTN_HIT_D1);
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_TRANSITION_DATA,
> + PCIE_CLIENT_DBG_FIFO_TRN_HIT_D0);
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_TRANSITION_DATA,
> + PCIE_CLIENT_DBG_FIFO_TRN_HIT_D1);
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_EN,
> + PCIE_CLIENT_DBG_FIFO_MODE_CON);
> +
> + INIT_DELAYED_WORK(&rockchip->trace_work,
> + rockchip_pcie_ltssm_trace_work);
> + schedule_delayed_work(&rockchip->trace_work, 0);
> + } else {
> + rockchip_pcie_writel_apb(rockchip,
> + PCIE_CLIENT_DBG_DIS,
> + PCIE_CLIENT_DBG_FIFO_MODE_CON);
> + cancel_delayed_work_sync(&rockchip->trace_work);
> + }
> +}
> +#else
> +static void rockchip_pcie_ltssm_trace(struct rockchip_pcie *rockchip,
> + bool enable)
> +{
> +}
> +#endif
> +
> static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
> {
> rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
> @@ -291,6 +398,9 @@ static int rockchip_pcie_start_link(struct dw_pcie *pci)
> * 100us as we don't know how long should the device need to reset.
> */
> msleep(PCIE_T_PVPERL_MS);
> +
> + rockchip_pcie_ltssm_trace(rockchip, true);
> +
> gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
>
> return 0;
> @@ -301,6 +411,7 @@ static void rockchip_pcie_stop_link(struct dw_pcie *pci)
> struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
>
> rockchip_pcie_disable_ltssm(rockchip);
> + rockchip_pcie_ltssm_trace(rockchip, false);
> }
>
> static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
^ permalink raw reply
* Re: [PATCH v4 1/3] PCI: trace: Add PCI controller LTSSM transition tracepoint
From: Steven Rostedt @ 2026-02-24 14:08 UTC (permalink / raw)
To: Shawn Lin
Cc: Manivannan Sadhasivam, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Masami Hiramatsu
In-Reply-To: <1769047340-113287-2-git-send-email-shawn.lin@rock-chips.com>
On Thu, 22 Jan 2026 10:02:18 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:
> +bool pci_ltssm_tp_enabled(void)
> +{
> + return atomic_read(&pcie_ltssm_tp_enabled) > 0;
> +}
> +EXPORT_SYMBOL(pci_ltssm_tp_enabled);
> +
> +int pci_ltssm_tp_reg(void)
> +{
> + atomic_inc(&pcie_ltssm_tp_enabled);
> + return 0;
> +}
> +
> +void pci_ltssm_tp_unreg(void)
> +{
> + atomic_dec(&pcie_ltssm_tp_enabled);
> +}
This seems totally unnecessary. Why the atomic operations? Why not just
use:
if (trace_pcie_ltssm_state_transition_enabled()) ...
?
-- Steve
^ permalink raw reply
* Re: [PATCH v4 0/3] PCI Controller event and LTSSM tracepoint support
From: Steven Rostedt @ 2026-02-24 14:06 UTC (permalink / raw)
To: Shawn Lin
Cc: Bjorn Helgaas, linux-rockchip, linux-pci, linux-trace-kernel,
linux-doc, Masami Hiramatsu, Manivannan Sadhasivam
In-Reply-To: <5549b86e-36f7-e5b2-e16c-f70e328ca6c2@rock-chips.com>
On Tue, 24 Feb 2026 16:49:47 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:
> I'd appreciate it if you could share any concerns you might have about
> v4. :)
Sure. I'll reply to the patches.
-- Steve
^ permalink raw reply
* Re: [RFC PATCH bpf-next 3/3] selftests/bpf: add tests for kprobe.session optimization
From: Jiri Olsa @ 2026-02-24 13:12 UTC (permalink / raw)
To: Andrey Grodzovsky
Cc: bpf, ast, daniel, andrii, rostedt, linux-trace-kernel,
linux-open-source
In-Reply-To: <20260223215113.924599-4-andrey.grodzovsky@crowdstrike.com>
On Mon, Feb 23, 2026 at 04:51:13PM -0500, Andrey Grodzovsky wrote:
> Add two new subtests to kprobe_multi_test to validate the
> kprobe.session exact function name optimization:
SNIP
> +static void test_session_errors(void)
> +{
> + struct kprobe_multi_session_errors *skel = NULL;
> + struct bpf_link *link_wildcard = NULL;
> + struct bpf_link *link_exact = NULL;
> + int err_wildcard, err_exact;
> +
> + skel = kprobe_multi_session_errors__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "kprobe_multi_session_errors__open_and_load"))
> + return;
> +
> + /*
> + * Test error code consistency: both wildcard (slow path) and exact name
> + * (fast path) should return the same error code (ENOENT) for non-existent
> + * functions. This protects against future kernel changes that might alter
> + * error return values.
> + */
> +
> + /* Try to attach with non-existent wildcard pattern (slow path) */
> + link_wildcard = bpf_program__attach(skel->progs.test_nonexistent_wildcard);
> + err_wildcard = -errno;
> + ASSERT_ERR_PTR(link_wildcard, "attach_nonexistent_wildcard");
> + ASSERT_EQ(err_wildcard, -ENOENT, "wildcard_error_enoent");
> +
> + /* Try to attach with non-existent exact name (fast path) */
> + link_exact = bpf_program__attach(skel->progs.test_nonexistent_exact);
> + err_exact = -errno;
> + ASSERT_ERR_PTR(link_exact, "attach_nonexistent_exact");
> + ASSERT_EQ(err_exact, -ENOENT, "exact_error_enoent");
> +
> + /*
> + * Verify both paths return identical error codes - this is critical for
> + * API consistency and prevents user code from breaking when switching
> + * between wildcard patterns and exact function names.
> + */
> + ASSERT_EQ(err_wildcard, err_exact, "error_consistency");
> +
> + kprobe_multi_session_errors__destroy(skel);
there's already subtest for attach failures (test_attach_api_fails),
so maybe let's put this over there?
SNIP
> diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_session_syms.c b/tools/testing/selftests/bpf/progs/kprobe_multi_session_syms.c
> new file mode 100644
> index 000000000000..6a4bd57af1fc
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/kprobe_multi_session_syms.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Test kprobe.session with exact function names to verify syms[] optimization */
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <stdbool.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +int pid = 0;
> +
> +/* Results for each function: incremented on entry and return */
> +__u64 test1_count = 0;
> +
> +/* Track entry vs return */
> +bool test1_return = false;
> +
> +/*
> + * No tests in here, just to trigger 'bpf_fentry_test*'
> + * through tracing test_run
> + */
> +SEC("fentry/bpf_modify_return_test")
> +int BPF_PROG(trigger)
> +{
> + return 0;
> +}
> +
> +/*
> + * Test 1: Exact function name (no wildcards) - uses fast syms[] path
> + * This should attach via opts.syms array, bypassing kallsyms parsing
> + */
> +SEC("kprobe.session/bpf_fentry_test1")
> +int test_kprobe_syms_1(struct pt_regs *ctx)
perhaps we could execute this as part of test_session_skel_api test?
seems like we could put this directly to progs/kprobe_multi_session.c and
call session_check(ctx) and change test_results validation accordingly
thanks,
jirka
> +{
> + if (bpf_get_current_pid_tgid() >> 32 != pid)
> + return 0;
> +
> + test1_count++;
> +
> + /* Check if this is return probe */
> + if (bpf_session_is_return(ctx))
> + test1_return = true;
> +
> + return 0; /* Always execute return probe */
> +}
> --
> 2.34.1
>
^ permalink raw reply
* Re: [RFC PATCH bpf-next 2/3] ftrace: Use kallsyms binary search for single-symbol lookup
From: Jiri Olsa @ 2026-02-24 13:12 UTC (permalink / raw)
To: Andrey Grodzovsky
Cc: bpf, ast, daniel, andrii, rostedt, linux-trace-kernel,
linux-open-source
In-Reply-To: <20260223215113.924599-3-andrey.grodzovsky@crowdstrike.com>
On Mon, Feb 23, 2026 at 04:51:12PM -0500, Andrey Grodzovsky wrote:
> When ftrace_lookup_symbols() is called with a single symbol (cnt == 1),
> use kallsyms_lookup_name() for O(log N) binary search instead of the
> full linear scan via kallsyms_on_each_symbol().
>
> ftrace_lookup_symbols() was designed for batch resolution of many
> symbols in a single pass. For large cnt this is efficient: a single
> O(N) walk over all symbols with O(log cnt) binary search into the
> sorted input array. But for cnt == 1 it still decompresses all ~200K
> kernel symbols only to match one.
>
> kallsyms_lookup_name() uses the sorted kallsyms index and needs only
> ~17 decompressions for a single lookup.
>
> This is the common path for kprobe.session with exact function names,
> where libbpf sends one symbol per BPF_LINK_CREATE syscall.
>
> If binary lookup fails (duplicate symbol names where the first match
> is not ftrace-instrumented, or module symbols), the function falls
> through to the existing linear scan path.
>
> Before (cnt=1, 50 kprobe.session programs):
> Attach: 858 ms (kallsyms_expand_symbol 25% of CPU)
>
> After:
> Attach: 52 ms (16x faster)
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
> ---
> kernel/trace/ftrace.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 827fb9a0bf0d..bfd7670669c2 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -9263,6 +9263,19 @@ static int kallsyms_callback(void *data, const char *name, unsigned long addr)
> * @addrs array, which needs to be big enough to store at least @cnt
> * addresses.
> *
> + * For a single symbol (cnt == 1), uses kallsyms_lookup_name() which
> + * performs an O(log N) binary search via the sorted kallsyms index.
> + * This avoids the full O(N) linear scan over all kernel symbols that
> + * the multi-symbol path requires.
> + *
> + * For multiple symbols, uses a single-pass linear scan via
> + * kallsyms_on_each_symbol() with binary search into the sorted input
> + * array. While individual lookups are O(log N), doing K lookups
> + * totals O(K * log N) which loses to a single sequential O(N) pass
> + * at scale due to cache-friendly memory access patterns of the linear
> + * walk. Empirical testing shows the linear scan is faster for batch
> + * lookups even well below 10K symbols.
> + *
> * Returns: 0 if all provided symbols are found, -ESRCH otherwise.
> */
> int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *addrs)
> @@ -9270,6 +9283,21 @@ int ftrace_lookup_symbols(const char **sorted_syms, size_t cnt, unsigned long *a
> struct kallsyms_data args;
> int found_all;
>
> + /* Fast path: single symbol uses O(log N) binary search */
> + if (cnt == 1) {
> + addrs[0] = kallsyms_lookup_name(sorted_syms[0]);
> + if (addrs[0])
> + addrs[0] = ftrace_location(addrs[0]);
the kallsyms_callback callback code does not take the address
from ftrace_location, just checks it exists .. I think it is
done later in the fprobe layer .. let's keep it the same
jirka
> + if (addrs[0])
> + return 0;
> + /*
> + * Binary lookup can fail for duplicate symbol names
> + * where the first match is not ftrace-instrumented,
> + * or for module symbols. Retry with linear scan.
> + */
> + }
> +
> + /* Batch path: single-pass O(N) linear scan */
> memset(addrs, 0, sizeof(*addrs) * cnt);
> args.addrs = addrs;
> args.syms = sorted_syms;
> --
> 2.34.1
>
^ permalink raw reply
* Re: [RFC PATCH bpf-next 1/3] libbpf: Optimize kprobe.session attachment for exact function names
From: Jiri Olsa @ 2026-02-24 13:10 UTC (permalink / raw)
To: Andrey Grodzovsky
Cc: bpf, ast, daniel, andrii, rostedt, linux-trace-kernel,
linux-open-source
In-Reply-To: <20260223215113.924599-2-andrey.grodzovsky@crowdstrike.com>
On Mon, Feb 23, 2026 at 04:51:11PM -0500, Andrey Grodzovsky wrote:
> Implement dual-path optimization in attach_kprobe_session():
> - Fast path: Use syms[] array for exact function names
> (no kallsyms parsing)
> - Slow path: Use pattern matching with kallsyms only for
> wildcards
>
> This avoids expensive kallsyms file parsing (~150ms) when function names
> are specified exactly, improving attachment time 50x (~3-5ms).
>
> Error code normalization: The fast path returns ESRCH from kernel's
> ftrace_lookup_symbols(), while slow path returns ENOENT from userspace
> kallsyms parsing. Convert ESRCH to ENOENT in fast path to maintain API
> consistency - both paths now return identical error codes for "symbol
> not found".
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@crowdstrike.com>
> ---
> tools/lib/bpf/libbpf.c | 32 +++++++++++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 0be7017800fe..87a71eab4308 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -12192,7 +12192,7 @@ static int attach_kprobe_session(const struct bpf_program *prog, long cookie,
> {
> LIBBPF_OPTS(bpf_kprobe_multi_opts, opts, .session = true);
> const char *spec;
> - char *pattern;
> + char *func_name;
> int n;
>
> *link = NULL;
> @@ -12202,14 +12202,36 @@ static int attach_kprobe_session(const struct bpf_program *prog, long cookie,
> return 0;
>
> spec = prog->sec_name + sizeof("kprobe.session/") - 1;
> - n = sscanf(spec, "%m[a-zA-Z0-9_.*?]", &pattern);
> + n = sscanf(spec, "%m[a-zA-Z0-9_.*?]", &func_name);
> if (n < 1) {
> - pr_warn("kprobe session pattern is invalid: %s\n", spec);
> + pr_warn("kprobe session function name is invalid: %s\n", spec);
> return -EINVAL;
> }
>
> - *link = bpf_program__attach_kprobe_multi_opts(prog, pattern, &opts);
> - free(pattern);
> + /* Check if pattern contains wildcards */
> + if (strpbrk(func_name, "*?")) {
> + /* Wildcard pattern - use pattern matching path with kallsyms parsing */
> + *link = bpf_program__attach_kprobe_multi_opts(prog, func_name, &opts);
> + } else {
> + /* Exact function name - use syms array path (fast, no kallsyms parsing) */
> + const char *syms[1];
> +
> + syms[0] = func_name;
> + opts.syms = syms;
> + opts.cnt = 1;
> + *link = bpf_program__attach_kprobe_multi_opts(prog, NULL, &opts);
hi,
good idea, could we do this directly in bpf_program__attach_kprobe_multi_opts ?
seems like it's not drectly related to session
jirka
> + if (!*link && errno == ESRCH) {
> + /*
> + * Normalize error code for API consistency: fast path returns ESRCH
> + * from kernel's ftrace_lookup_symbols(), while slow path returns ENOENT
> + * from userspace kallsyms parsing. Convert ESRCH to ENOENT so both paths
> + * return the same error for "symbol not found".
> + */
> + errno = ENOENT;
> + }
> + }
> +
> + free(func_name);
> return *link ? 0 : -errno;
> }
>
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH bpf-next 02/17] bpf: Use mutex lock pool for bpf trampolines
From: Jiri Olsa @ 2026-02-24 12:27 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
bpf, linux-trace-kernel, Martin KaFai Lau, Eduard Zingerman,
Song Liu, Yonghong Song, Menglong Dong, Steven Rostedt
In-Reply-To: <CAADnVQL_Jpe_7a55HTn5CyYoxWcHahiK3+CDeOeURqZAuf+teA@mail.gmail.com>
On Mon, Feb 23, 2026 at 11:35:29AM -0800, Alexei Starovoitov wrote:
> On Sun, Feb 22, 2026 at 6:34 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Fri, Feb 20, 2026 at 11:58:13AM -0800, Alexei Starovoitov wrote:
> > > On Fri, Feb 20, 2026 at 2:07 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > Adding mutex lock pool that replaces bpf trampolines mutex.
> > > >
> > > > For tracing_multi link coming in following changes we need to lock all
> > > > the involved trampolines during the attachment. This could mean thousands
> > > > of mutex locks, which is not convenient.
> > > >
> > > > As suggested by Andrii we can replace bpf trampolines mutex with mutex
> > > > pool, where each trampoline is hash-ed to one of the locks from the pool.
> > > >
> > > > It's better to lock all the pool mutexes (64 at the moment) than
> > > > thousands of them.
> > > >
> > > > Removing the mutex_is_locked in bpf_trampoline_put, because we removed
> > > > the mutex from bpf_trampoline.
> > > >
> > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > > ---
> > > > include/linux/bpf.h | 2 --
> > > > kernel/bpf/trampoline.c | 74 +++++++++++++++++++++++++++++++----------
> > > > 2 files changed, 56 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > > index cd9b96434904..46bf3d86bdb2 100644
> > > > --- a/include/linux/bpf.h
> > > > +++ b/include/linux/bpf.h
> > > > @@ -1335,8 +1335,6 @@ struct bpf_trampoline {
> > > > /* hlist for trampoline_ip_table */
> > > > struct hlist_node hlist_ip;
> > > > struct ftrace_ops *fops;
> > > > - /* serializes access to fields of this trampoline */
> > > > - struct mutex mutex;
> > > > refcount_t refcnt;
> > > > u32 flags;
> > > > u64 key;
> > > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > > > index 952cd7932461..05dc0358654d 100644
> > > > --- a/kernel/bpf/trampoline.c
> > > > +++ b/kernel/bpf/trampoline.c
> > > > @@ -30,6 +30,45 @@ static struct hlist_head trampoline_ip_table[TRAMPOLINE_TABLE_SIZE];
> > > > /* serializes access to trampoline tables */
> > > > static DEFINE_MUTEX(trampoline_mutex);
> > > >
> > > > +#define TRAMPOLINE_LOCKS_BITS 6
> > > > +#define TRAMPOLINE_LOCKS_TABLE_SIZE (1 << TRAMPOLINE_LOCKS_BITS)
> > > > +
> > > > +static struct {
> > > > + struct mutex mutex;
> > > > + struct lock_class_key key;
> > > > +} *trampoline_locks;
> > > > +
> > > > +static struct mutex *trampoline_locks_lookup(struct bpf_trampoline *tr)
> > >
> > > select_trampoline_lock() ?
> >
> > ok
> >
> > >
> > > > +{
> > > > + return &trampoline_locks[hash_64((u64) tr, TRAMPOLINE_LOCKS_BITS)].mutex;
> > > > +}
> > > > +
> > > > +static void trampoline_lock(struct bpf_trampoline *tr)
> > > > +{
> > > > + mutex_lock(trampoline_locks_lookup(tr));
> > > > +}
> > > > +
> > > > +static void trampoline_unlock(struct bpf_trampoline *tr)
> > > > +{
> > > > + mutex_unlock(trampoline_locks_lookup(tr));
> > > > +}
> > > > +
> > > > +static int __init trampoline_locks_init(void)
> > > > +{
> > > > + int i;
> > > > +
> > > > + trampoline_locks = kmalloc_array(TRAMPOLINE_LOCKS_TABLE_SIZE,
> > > > + sizeof(trampoline_locks[0]), GFP_KERNEL);
> > >
> > > why bother with memory allocation? This is just 64 mutexes.
> >
> > ok, I could probably use __mutex_init directly for static key
> >
> > about 64.. not sure how I missed that but there's lockdep limit for
> > maximum locks depth and it's 48.. so we'll need to use 32 locks,
> > which is probably still ok
> >
> > >
> > > > + if (!trampoline_locks)
> > > > + return -ENOMEM;
> > > > +
> > > > + for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++) {
> > > > + lockdep_register_key(&trampoline_locks[i].key);
> > >
> > > why special key?
> >
> > if we keep single key we will get lockdep 'recursive locking' warning
> > during bpf_trampoline_multi_attach, because lockdep will think we lock
> > the same mutex
> >
> > there's support to annotate nested locking with mutex_lock_nested but
> > it allows maximum of 8 nested instances
>
> yeah. subclass limit of 8 is there for a different use case.
>
>
> I guess you never validated your earlier approach of "let's take
> all trampoline mutexes" with lockdep ? ;)
nope, the rfc had workaround for lockdep ;-)
+#ifdef CONFIG_LOCKDEP
+ mutex_init_with_key(&tr->mutex, &__lockdep_no_track__);
+#else
but I overlooked lockdep config for this version
> MAX_LOCK_DEPTH is indeed 48.
>
> See fs/configfs/inode.c and default_group_class.
> It does:
> lockdep_set_class(&inode->i_rwsem,
> &default_group_class[depth - 1]);
>
> the idea here is that the number of lockdep keys doesn't have
> to be equal to the actual number of mutexes.
I see, thanks for the pointer
>
> I guess we can keep a total of 32 mutexes to avoid making it too fancy.
> Please add a comment explaining 32 and why it needs lockdep_key.
ok
>
> I thought declaring all mutexes as static will avoid the need for the key,
> but DEFINE_MUTEX doesn't support an array.
> So since we need a loop anyway to init mutex and the key,
> let's keep kmalloc_array() above. Which is now renamed to kmalloc_objs()
> after 7.0-rc1.
I don't mind either way, meanwhile I used this version:
static struct {
struct mutex mutex;
struct lock_class_key key;
} trampoline_locks[TRAMPOLINE_LOCKS_TABLE_SIZE];
for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++)
__mutex_init(&trampoline_locks[i].mutex, "trampoline_lock", &trampoline_locks[i].key);
thanks,
jirka
^ permalink raw reply
* Re: [RFC PATCH v2 09/37] KVM: guest_memfd: Add support for KVM_SET_MEMORY_ATTRIBUTES2
From: Ackerley Tng @ 2026-02-24 10:14 UTC (permalink / raw)
To: kvm, linux-doc, linux-kernel, linux-kselftest, linux-trace-kernel,
x86
Cc: aik, andrew.jones, binbin.wu, bp, brauner, chao.p.peng,
chao.p.peng, chenhuacai, corbet, dave.hansen, david, hpa,
ira.weiny, jgg, jmattson, jroedel, jthoughton, maobibo,
mathieu.desnoyers, maz, mhiramat, michael.roth, mingo, mlevitsk,
oupton, pankaj.gupta, pbonzini, prsampat, qperret, ricarkol,
rick.p.edgecombe, rientjes, rostedt, seanjc, shivankg, shuah,
steven.price, tabba, tglx, vannapurve, vbabka, willy, wyihan,
yan.y.zhao
In-Reply-To: <CAEvNRgFMNywpDRr+WeNsVj=MnsbhZp9H3j0QRDo_eOP+kGCNJw@mail.gmail.com>
Ackerley Tng <ackerleytng@google.com> writes:
> Ackerley Tng <ackerleytng@google.com> writes:
>
>>
>> [...snip...]
>>
> Before this lands, Sean wants, at the very minimum, an in-principle
> agreement on guest_memfd behavior with respect to whether or not memory
> should be preserved on conversion.
>>
>> [...snip...]
>>
Here's what I've come up with, following up from last guest_memfd
biweekly.
Every KVM_SET_MEMORY_ATTRIBUTES2 request will be accompanied by an
enum set_memory_attributes_content_policy:
enum set_memory_attributes_content_policy {
SET_MEMORY_ATTRIBUTES_CONTENT_ZERO,
SET_MEMORY_ATTRIBUTES_CONTENT_READABLE,
SET_MEMORY_ATTRIBUTES_CONTENT_ENCRYPTED,
}
Within guest_memfd's KVM_SET_MEMORY_ATTRIBUTES2 handler, guest_memfd
will make an arch call
kvm_gmem_arch_content_policy_supported(kvm, policy, gfn, nr_pages)
where every arch will get to return some error if the requested policy
is not supported for the given range.
ZERO is the simplest of the above, it means that after the conversion
the memory will be zeroed for the next reader.
+ TDX and SNP today will support ZERO since the firmware handles
zeroing.
+ pKVM and SW_PROTECTED_VM will apply software zeroing.
+ Purpose: having this policy in the API allows userspace to be sure
that the memory is zeroed after the conversion - there is no need to
zero again in userspace (addresses concern that Sean pointed out)
READABLE means that after the conversion, the memory is readable by
userspace (if converting to shared) or readable by the guest (if
converting to private).
+ TDX and SNP (today) can't support this, so return -EOPNOTSUPP
+ SW_PROTECTED_VM will support this and do nothing extra on
conversion, since there is no encryption anyway and all content
remains readable.
+ pKVM will make use of the arch function above.
Here's where I need input: (David's questions during the call about
the full flow beginning with the guest prompted this).
Since pKVM doesn't encrypt the memory contents, there must be some way
that pKVM can say no when userspace requests to convert and retain
READABLE contents? I think pKVM's arch function can be used to check
if the guest previously made a conversion request. Fuad, to check that
the guest made a conversion request, what's other parameters are
needed other than gfn and nr_pages?
ENCRYPTED means that after the conversion, the memory contents are
retained as-is, with no decryption.
+ TDX and SNP (today) can't support this, so return -EOPNOTSUPP
+ pKVM and SW_PROTECTED_VM can do nothing, but doing nothing retains
READABLE content, not ENCRYPTED content, hence SW_PROTECTED_VM
should return -EOPNOTSUPP.
+ Michael, you mentioned during the call that SNP is planning to
introduce a policy that retains the ENCRYPTED version for a special
GHCB call. ENCRYPTED is meant for that use case. Does it work? I'm
assuming that SNP should only support this policy given some
conditions, so would the arch call as described above work?
+ If this policy is specified on conversion from shared to private,
always return -EOPNOTSUPP.
+ When this first lands, ENCRYPTED will not be a valid option, but I'm
listing it here so we have line of sight to having this support.
READABLE and ENCRYPTED defines the state after conversion clearly
(instead of DONT_CARE or similar).
DESTROY could be another policy, which means that after the
conversion, the memory is unreadable. This is the option to address
what David brought up during the call, for cases where userspace knows
it is going to free the memory already and doesn't care about the
state as long as nobody gets to read it. This will not implemented
when feature first lands, but is presented here just to show how this
can be extended in future.
Right now, I'm thinking that one of the above policies MUST be
specified (not specifying a policy will result in -EINVAL).
How does this sound?
^ permalink raw reply
* Re: [PATCH v3 07/19] rv/rvgen: fix typos in automata and generator docstring and comments
From: Gabriele Monaco @ 2026-02-24 9:37 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <20260223162407.147003-8-wander@redhat.com>
On Mon, 2026-02-23 at 13:17 -0300, Wander Lairson Costa wrote:
> - # when a transition has more than one lables, they are like
> this
> + # when a transition has more than one labels, they are like
> this
> # "local_irq_enable\nhw_local_irq_enable_n"
> # so split them.
>
Missing one little thing:
s/labels/label/
I'm going to apply it directly on my tree so you don't need to send a V4 just
for this.
Thanks,
Gabriele
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index fa44795adef46..9255cc2153a31 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -3,7 +3,7 @@
> #
> # Copyright (C) 2019-2022 Red Hat, Inc. Daniel Bristot de Oliveira
> <bristot@kernel.org>
> #
> -# dot2c: parse an automata in dot file digraph format into a C
> +# dot2c: parse an automaton in dot file digraph format into a C
> #
> # This program was written in the development of this paper:
> # de Oliveira, D. B. and Cucinotta, T. and de Oliveira, R. S.
> diff --git a/tools/verification/rvgen/rvgen/dot2k.py
> b/tools/verification/rvgen/rvgen/dot2k.py
> index 47af9f104a829..aedc2a7799b32 100644
> --- a/tools/verification/rvgen/rvgen/dot2k.py
> +++ b/tools/verification/rvgen/rvgen/dot2k.py
> @@ -167,14 +167,14 @@ class da2k(dot2k):
> def __init__(self, *args, **kwargs):
> super().__init__(*args, **kwargs)
> if self.is_hybrid_automata():
> - raise AutomataError("Detected hybrid automata, use the 'ha'
> class")
> + raise AutomataError("Detected hybrid automaton, use the 'ha'
> class")
>
> class ha2k(dot2k):
> """Hybrid automata only"""
> def __init__(self, *args, **kwargs):
> super().__init__(*args, **kwargs)
> if not self.is_hybrid_automata():
> - raise AutomataError("Detected deterministic automata, use the
> 'da' class")
> + raise AutomataError("Detected deterministic automaton, use the
> 'da' class")
> self.trace_h = self._read_template_file("trace_hybrid.h")
> self.__parse_constraints()
>
> diff --git a/tools/verification/rvgen/rvgen/generator.py
> b/tools/verification/rvgen/rvgen/generator.py
> index d932e96dd66d3..988ccdc27fa37 100644
> --- a/tools/verification/rvgen/rvgen/generator.py
> +++ b/tools/verification/rvgen/rvgen/generator.py
> @@ -3,7 +3,7 @@
> #
> # Copyright (C) 2019-2022 Red Hat, Inc. Daniel Bristot de Oliveira
> <bristot@kernel.org>
> #
> -# Abtract class for generating kernel runtime verification monitors from
> specification file
> +# Abstract class for generating kernel runtime verification monitors from
> specification file
>
> import platform
> import os
^ permalink raw reply
* Re: [PATCH v3 01/19] rv/rvgen: introduce AutomataError exception class
From: Gabriele Monaco @ 2026-02-24 9:32 UTC (permalink / raw)
To: Wander Lairson Costa
Cc: Steven Rostedt, Nam Cao, open list:RUNTIME VERIFICATION (RV),
open list
In-Reply-To: <20260223162407.147003-2-wander@redhat.com>
On Mon, 2026-02-23 at 13:17 -0300, Wander Lairson Costa wrote:
> Replace the generic except Exception block with a custom AutomataError
> class that inherits from Exception. This provides more precise exception
> handling for automata parsing and validation errors while avoiding
> overly broad exception catches that could mask programming errors like
> SyntaxError or TypeError.
>
> The AutomataError class is raised when DOT file processing fails due to
> invalid format, I/O errors, or malformed automaton definitions. The
> main entry point catches this specific exception and provides a
> user-friendly error message to stderr before exiting.
>
> Also, replace generic exceptions raising in HA and LTL with
> AutomataError.
>
> Co-authored-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
All good, thanks!
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
> tools/verification/rvgen/__main__.py | 6 ++---
> tools/verification/rvgen/rvgen/automata.py | 17 ++++++++++----
> tools/verification/rvgen/rvgen/dot2c.py | 4 ++--
> tools/verification/rvgen/rvgen/dot2k.py | 26 ++++++++++-----------
> tools/verification/rvgen/rvgen/generator.py | 7 ++----
> tools/verification/rvgen/rvgen/ltl2ba.py | 9 +++----
> tools/verification/rvgen/rvgen/ltl2k.py | 8 +++++--
> 7 files changed, 43 insertions(+), 34 deletions(-)
>
> diff --git a/tools/verification/rvgen/__main__.py
> b/tools/verification/rvgen/__main__.py
> index 9a5a9f08eae21..5a3f090ac3316 100644
> --- a/tools/verification/rvgen/__main__.py
> +++ b/tools/verification/rvgen/__main__.py
> @@ -13,6 +13,7 @@ if __name__ == '__main__':
> from rvgen.generator import Monitor
> from rvgen.container import Container
> from rvgen.ltl2k import ltl2k
> + from rvgen.automata import AutomataError
> import argparse
> import sys
>
> @@ -55,9 +56,8 @@ if __name__ == '__main__':
> sys.exit(1)
> else:
> monitor = Container(vars(params))
> - except Exception as e:
> - print('Error: '+ str(e))
> - print("Sorry : :-(")
> + except AutomataError as e:
> + print(f"There was an error processing {params.spec}: {e}",
> file=sys.stderr)
> sys.exit(1)
>
> print("Writing the monitor into the directory %s" % monitor.name)
> diff --git a/tools/verification/rvgen/rvgen/automata.py
> b/tools/verification/rvgen/rvgen/automata.py
> index 5c1c5597d839f..9cc452305a2aa 100644
> --- a/tools/verification/rvgen/rvgen/automata.py
> +++ b/tools/verification/rvgen/rvgen/automata.py
> @@ -25,6 +25,13 @@ class _EventConstraintKey(_ConstraintKey, tuple):
> def __new__(cls, state_id: int, event_id: int):
> return super().__new__(cls, (state_id, event_id))
>
> +class AutomataError(Exception):
> + """Exception raised for errors in automata parsing and validation.
> +
> + Raised when DOT file processing fails due to invalid format, I/O errors,
> + or malformed automaton definitions.
> + """
> +
> class Automata:
> """Automata class: Reads a dot file and part it as an automata.
>
> @@ -72,11 +79,11 @@ class Automata:
> basename = ntpath.basename(self.__dot_path)
> if not basename.endswith(".dot") and not basename.endswith(".gv"):
> print("not a dot file")
> - raise Exception("not a dot file: %s" % self.__dot_path)
> + raise AutomataError("not a dot file: %s" % self.__dot_path)
>
> model_name = ntpath.splitext(basename)[0]
> if model_name.__len__() == 0:
> - raise Exception("not a dot file: %s" % self.__dot_path)
> + raise AutomataError("not a dot file: %s" % self.__dot_path)
>
> return model_name
>
> @@ -85,8 +92,8 @@ class Automata:
> dot_lines = []
> try:
> dot_file = open(self.__dot_path)
> - except:
> - raise Exception("Cannot open the file: %s" % self.__dot_path)
> + except OSError as exc:
> + raise AutomataError(exc.strerror) from exc
>
> dot_lines = dot_file.read().splitlines()
> dot_file.close()
> @@ -95,7 +102,7 @@ class Automata:
> line = dot_lines[cursor].split()
>
> if (line[0] != "digraph") and (line[1] != "state_automaton"):
> - raise Exception("Not a valid .dot format: %s" % self.__dot_path)
> + raise AutomataError("Not a valid .dot format: %s" %
> self.__dot_path)
> else:
> cursor += 1
> return dot_lines
> diff --git a/tools/verification/rvgen/rvgen/dot2c.py
> b/tools/verification/rvgen/rvgen/dot2c.py
> index f779d9528af3f..6878cc79e6f70 100644
> --- a/tools/verification/rvgen/rvgen/dot2c.py
> +++ b/tools/verification/rvgen/rvgen/dot2c.py
> @@ -13,7 +13,7 @@
> # For further information, see:
> # Documentation/trace/rv/deterministic_automata.rst
>
> -from .automata import Automata
> +from .automata import Automata, AutomataError
>
> class Dot2c(Automata):
> enum_suffix = ""
> @@ -103,7 +103,7 @@ class Dot2c(Automata):
> min_type = "unsigned int"
>
> if self.states.__len__() > 1000000:
> - raise Exception("Too many states: %d" % self.states.__len__())
> + raise AutomataError("Too many states: %d" %
> self.states.__len__())
>
> return min_type
>
> diff --git a/tools/verification/rvgen/rvgen/dot2k.py
> b/tools/verification/rvgen/rvgen/dot2k.py
> index e7ba68a54c1f8..55222e38323f5 100644
> --- a/tools/verification/rvgen/rvgen/dot2k.py
> +++ b/tools/verification/rvgen/rvgen/dot2k.py
> @@ -11,7 +11,7 @@
> from collections import deque
> from .dot2c import Dot2c
> from .generator import Monitor
> -from .automata import _EventConstraintKey, _StateConstraintKey
> +from .automata import _EventConstraintKey, _StateConstraintKey, AutomataError
>
>
> class dot2k(Monitor, Dot2c):
> @@ -166,14 +166,14 @@ class da2k(dot2k):
> def __init__(self, *args, **kwargs):
> super().__init__(*args, **kwargs)
> if self.is_hybrid_automata():
> - raise ValueError("Detected hybrid automata, use the 'ha' class")
> + raise AutomataError("Detected hybrid automata, use the 'ha'
> class")
>
> class ha2k(dot2k):
> """Hybrid automata only"""
> def __init__(self, *args, **kwargs):
> super().__init__(*args, **kwargs)
> if not self.is_hybrid_automata():
> - raise ValueError("Detected deterministic automata, use the 'da'
> class")
> + raise AutomataError("Detected deterministic automata, use the
> 'da' class")
> self.trace_h = self._read_template_file("trace_hybrid.h")
> self.__parse_constraints()
>
> @@ -266,22 +266,22 @@ class ha2k(dot2k):
> # state constraints are only used for expirations (e.g. clk<N)
> if self.is_event_constraint(key):
> if not rule and not reset:
> - raise ValueError("Unrecognised event constraint "
> -
> f"({self.states[key[0]]}/{self.events[key[1]]}: {constr})")
> + raise AutomataError("Unrecognised event constraint "
> +
> f"({self.states[key[0]]}/{self.events[key[1]]}: {constr})")
> if rule and (rule["env"] in self.env_types and
> rule["env"] not in self.env_stored):
> - raise ValueError("Clocks in hybrid automata always require a
> storage"
> - f" ({rule["env"]})")
> + raise AutomataError("Clocks in hybrid automata always require
> a storage"
> + f" ({rule["env"]})")
> else:
> if not rule:
> - raise ValueError("Unrecognised state constraint "
> - f"({self.states[key]}: {constr})")
> + raise AutomataError("Unrecognised state constraint "
> + f"({self.states[key]}: {constr})")
> if rule["env"] not in self.env_stored:
> - raise ValueError("State constraints always require a storage
> "
> - f"({rule["env"]})")
> + raise AutomataError("State constraints always require a
> storage "
> + f"({rule["env"]})")
> if rule["op"] not in ["<", "<="]:
> - raise ValueError("State constraints must be clock expirations
> like"
> - f" clk<N ({rule.string})")
> + raise AutomataError("State constraints must be clock
> expirations like"
> + f" clk<N ({rule.string})")
>
> def __parse_constraints(self) -> None:
> self.guards: dict[_EventConstraintKey, str] = {}
> diff --git a/tools/verification/rvgen/rvgen/generator.py
> b/tools/verification/rvgen/rvgen/generator.py
> index 5eac12e110dce..571093a92bdc8 100644
> --- a/tools/verification/rvgen/rvgen/generator.py
> +++ b/tools/verification/rvgen/rvgen/generator.py
> @@ -51,10 +51,7 @@ class RVGenerator:
> raise FileNotFoundError("Could not find the rv directory, do you have
> the kernel source installed?")
>
> def _read_file(self, path):
> - try:
> - fd = open(path, 'r')
> - except OSError:
> - raise Exception("Cannot open the file: %s" % path)
> + fd = open(path, 'r')
>
> content = fd.read()
>
> @@ -65,7 +62,7 @@ class RVGenerator:
> try:
> path = os.path.join(self.abs_template_dir, file)
> return self._read_file(path)
> - except Exception:
> + except OSError:
> # Specific template file not found. Try the generic template file
> in the template/
> # directory, which is one level up
> path = os.path.join(self.abs_template_dir, "..", file)
> diff --git a/tools/verification/rvgen/rvgen/ltl2ba.py
> b/tools/verification/rvgen/rvgen/ltl2ba.py
> index f14e6760ac3db..f9855dfa3bc1c 100644
> --- a/tools/verification/rvgen/rvgen/ltl2ba.py
> +++ b/tools/verification/rvgen/rvgen/ltl2ba.py
> @@ -9,6 +9,7 @@
>
> from ply.lex import lex
> from ply.yacc import yacc
> +from .automata import AutomataError
>
> # Grammar:
> # ltl ::= opd | ( ltl ) | ltl binop ltl | unop ltl
> @@ -62,7 +63,7 @@ t_ignore_COMMENT = r'\#.*'
> t_ignore = ' \t\n'
>
> def t_error(t):
> - raise ValueError(f"Illegal character '{t.value[0]}'")
> + raise AutomataError(f"Illegal character '{t.value[0]}'")
>
> lexer = lex()
>
> @@ -487,7 +488,7 @@ def p_unop(p):
> elif p[1] == "not":
> op = NotOp(p[2])
> else:
> - raise ValueError(f"Invalid unary operator {p[1]}")
> + raise AutomataError(f"Invalid unary operator {p[1]}")
>
> p[0] = ASTNode(op)
>
> @@ -507,7 +508,7 @@ def p_binop(p):
> elif p[2] == "imply":
> op = ImplyOp(p[1], p[3])
> else:
> - raise ValueError(f"Invalid binary operator {p[2]}")
> + raise AutomataError(f"Invalid binary operator {p[2]}")
>
> p[0] = ASTNode(op)
>
> @@ -526,7 +527,7 @@ def parse_ltl(s: str) -> ASTNode:
> subexpr[assign[0]] = assign[1]
>
> if rule is None:
> - raise ValueError("Please define your specification in the \"RULE =
> <LTL spec>\" format")
> + raise AutomataError("Please define your specification in the \"RULE =
> <LTL spec>\" format")
>
> for node in rule:
> if not isinstance(node.op, Variable):
> diff --git a/tools/verification/rvgen/rvgen/ltl2k.py
> b/tools/verification/rvgen/rvgen/ltl2k.py
> index b075f98d50c47..08ad245462e7d 100644
> --- a/tools/verification/rvgen/rvgen/ltl2k.py
> +++ b/tools/verification/rvgen/rvgen/ltl2k.py
> @@ -4,6 +4,7 @@
> from pathlib import Path
> from . import generator
> from . import ltl2ba
> +from .automata import AutomataError
>
> COLUMN_LIMIT = 100
>
> @@ -60,8 +61,11 @@ class ltl2k(generator.Monitor):
> if MonitorType != "per_task":
> raise NotImplementedError("Only per_task monitor is supported for
> LTL")
> super().__init__(extra_params)
> - with open(file_path) as f:
> - self.atoms, self.ba, self.ltl = ltl2ba.create_graph(f.read())
> + try:
> + with open(file_path) as f:
> + self.atoms, self.ba, self.ltl = ltl2ba.create_graph(f.read())
> + except OSError as exc:
> + raise AutomataError(exc.strerror) from exc
> self.atoms_abbr = abbreviate_atoms(self.atoms)
> self.name = extra_params.get("model_name")
> if not self.name:
^ permalink raw reply
* Re: [PATCH v4 0/3] PCI Controller event and LTSSM tracepoint support
From: Shawn Lin @ 2026-02-24 8:49 UTC (permalink / raw)
To: Steven Rostedt
Cc: shawn.lin, Bjorn Helgaas, linux-rockchip, linux-pci,
linux-trace-kernel, linux-doc, Masami Hiramatsu,
Manivannan Sadhasivam
In-Reply-To: <u3es7h4mgzdqfhvpl52s3x4vcisrpazcm4qb6x7lu4srmagxpn@hca2begqypeu>
Hi Steven,
在 2026/02/11 星期三 23:40, Manivannan Sadhasivam 写道:
> On Wed, Feb 11, 2026 at 09:13:50PM +0800, Shawn Lin wrote:
>> 在 2026/01/22 星期四 10:02, Shawn Lin 写道:
>>> This patch-set adds new pci controller event and LTSSM tracepoint used by host drivers
>>> which provide LTSSM trace functionality. The first user is pcie-dw-rockchip with a 256
>>> Bytes FIFO for recording LTSSM transition.
>>>
>>
>> Gentle ping...
>>
>
> Merge window is open now, so we can't accept any patches until -rc1. And I'm
> also waiting for an Ack from Steven for the tracing part.
>
I'd appreciate it if you could share any concerns you might have about
v4. :)
Thanks.
> - Mani
>
>>> Testing
>>> =========
>>>
>>> This series was tested on RK3588/RK3588s EVB1 with NVMe SSD connected to PCIe3 and PCIe2
>>> root ports.
>>>
>>> echo 1 > /sys/kernel/debug/tracing/events/pci_controller/pcie_ltssm_state_transition/enable
>>> cat /sys/kernel/debug/tracing/trace_pipe
>>>
>>> # tracer: nop
>>> #
>>> # entries-in-buffer/entries-written: 64/64 #P:8
>>> #
>>> # _-----=> irqs-off/BH-disabled
>>> # / _----=> need-resched
>>> # | / _---=> hardirq/softirq
>>> # || / _--=> preempt-depth
>>> # ||| / _-=> migrate-disable
>>> # |||| / delay
>>> # TASK-PID CPU# ||||| TIMESTAMP FUNCTION
>>> # | | | ||||| | |
>>> kworker/0:0-9 [000] ..... 5.600194: pcie_ltssm_state_transition: dev: a40000000.pcie state: DETECT_ACT rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600198: pcie_ltssm_state_transition: dev: a40000000.pcie state: DETECT_WAIT rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600199: pcie_ltssm_state_transition: dev: a40000000.pcie state: DETECT_ACT rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600201: pcie_ltssm_state_transition: dev: a40000000.pcie state: POLL_ACTIVE rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600202: pcie_ltssm_state_transition: dev: a40000000.pcie state: POLL_CONFIG rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600204: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_LINKWD_START rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600206: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_LINKWD_ACEPT rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600207: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_LANENUM_WAI rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600208: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_LANENUM_ACEPT rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600210: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_COMPLETE rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600212: pcie_ltssm_state_transition: dev: a40000000.pcie state: CFG_IDLE rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600213: pcie_ltssm_state_transition: dev: a40000000.pcie state: L0 rate: 2.5 GT/s
>>> kworker/0:0-9 [000] ..... 5.600214: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600216: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_RCVRCFG rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600217: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_SPEED rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600218: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600220: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_EQ1 rate: Unknown
>>> kworker/0:0-9 [000] ..... 5.600221: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_EQ2 rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600222: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_EQ3 rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600224: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600225: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_RCVRCFG rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600226: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_IDLE rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600227: pcie_ltssm_state_transition: dev: a40000000.pcie state: L0 rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600228: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600229: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_RCVRCFG rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600231: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_IDLE rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600232: pcie_ltssm_state_transition: dev: a40000000.pcie state: L0 rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600233: pcie_ltssm_state_transition: dev: a40000000.pcie state: L123_SEND_EIDLE rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600234: pcie_ltssm_state_transition: dev: a40000000.pcie state: L1_IDLE rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600236: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_LOCK rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600237: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_RCVRCFG rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600238: pcie_ltssm_state_transition: dev: a40000000.pcie state: RCVRY_IDLE rate: 8.0 GT/s
>>> kworker/0:0-9 [000] ..... 5.600239: pcie_ltssm_state_transition: dev: a40000000.pcie state: L0 rate: 8.0 GT/s
>>>
>>>
>>> Changes in v4:
>>> - use TRACE_EVENT_FN to notify when to start and stop the tracepoint,
>>> and export pci_ltssm_tp_enabled() for host drivers to use
>>> - skip trace if pci_ltssm_tp_enabled() is false.(Steven)
>>> - wrap into 80 columns(Bjorn)
>>>
>>> Changes in v3:
>>> - add TRACE_DEFINE_ENUM for all enums(Steven Rostedt)
>>> - Add toctree entry in Documentation/trace/index.rst(Bagas Sanjaya)
>>> - fix mismatch section underline length(Bagas Sanjaya)
>>> - Make example snippets in code block(Bagas Sanjaya)
>>> - warp context into 80 columns and fix the file name(Bjorn)
>>> - reorder variables(Mani)
>>> - rename loop to i; rename en to enable(Mani)
>>> - use FIELD_GET(Mani)
>>> - add comment about how the FIFO works(Mani)
>>>
>>> Changes in v2:
>>> - use tracepoint
>>>
>>> Shawn Lin (3):
>>> PCI: trace: Add PCI controller LTSSM transition tracepoint
>>> Documentation: tracing: Add PCI controller event documentation
>>> PCI: dw-rockchip: Add pcie_ltssm_state_transition trace support
>>>
>>> Documentation/trace/events-pci-controller.rst | 42 ++++++++++
>>> Documentation/trace/index.rst | 1 +
>>> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 111 ++++++++++++++++++++++++++
>>> drivers/pci/trace.c | 20 +++++
>>> include/linux/pci.h | 4 +
>>> include/trace/events/pci_controller.h | 57 +++++++++++++
>>> 6 files changed, 235 insertions(+)
>>> create mode 100644 Documentation/trace/events-pci-controller.rst
>>> create mode 100644 include/trace/events/pci_controller.h
>>>
>
^ permalink raw reply
* Re: [PATCH 2/2] fgraph: fix thresh_return nosleeptime double-adjust
From: Masami Hiramatsu @ 2026-02-24 8:36 UTC (permalink / raw)
To: hu.shengming
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, zhang.run, yang.tao172, yang.yang29
In-Reply-To: <20260221113314048jE4VRwIyZEALiYByGK0My@zte.com.cn>
On Sat, 21 Feb 2026 11:33:14 +0800 (CST)
<hu.shengming@zte.com.cn> wrote:
> From: Shengming Hu <hu.shengming@zte.com.cn>
>
> trace_graph_thresh_return() called handle_nosleeptime() and then
> delegated to trace_graph_return(), which calls
> handle_nosleeptime() again. When sleep-time accounting is
> disabled this double-adjusts calltime and can produce bogus
> durations (including underflow).
>
> Fix this by computing rettime once, applying
> handle_nosleeptime() only once, using the adjusted calltime
> for threshold comparison, and writing the return event
> directly via __trace_graph_return() when the threshold is met.
>
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> Fixes: 3c9880f3ab52b ("ftrace: Use a running sleeptime instead of
> saving on shadow stack")
> Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> ---
> kernel/trace/trace_functions_graph.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index cbe43680c..b9c81fbd9 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -403,8 +403,12 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
> unsigned long *task_var = fgraph_get_task_var(gops);
> struct fgraph_times *ftimes;
> struct trace_array *tr;
> + unsigned int trace_ctx;
> + u64 calltime, rettime;
> int size;
>
> + rettime = trace_clock_local();
> +
> ftrace_graph_addr_finish(gops, trace);
>
> if (*task_var & TRACE_GRAPH_NOTRACE) {
> @@ -419,11 +423,13 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
> tr = gops->private;
> handle_nosleeptime(tr, trace, ftimes, size);
>
> - if (tracing_thresh &&
> - (trace_clock_local() - ftimes->calltime < tracing_thresh))
> + calltime = ftimes->calltime;
> +
> + if (tracing_thresh && (rettime - calltime < tracing_thresh))
> return;
> - else
> - trace_graph_return(trace, gops, fregs);
> +
> + trace_ctx = tracing_gen_ctx();
> + __trace_graph_return(tr, trace, trace_ctx, calltime, rettime);
> }
>
> static struct fgraph_ops funcgraph_ops = {
> --
> 2.25.1
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/2] fgraph: fix thresh_return clear per-task notrace
From: Masami Hiramatsu @ 2026-02-24 8:33 UTC (permalink / raw)
To: hu.shengming
Cc: rostedt, mhiramat, mathieu.desnoyers, linux-kernel,
linux-trace-kernel, zhang.run, yang.tao172, yang.yang29
In-Reply-To: <20260221113007819YgrZsMGABff4Rc-O_fZxL@zte.com.cn>
On Sat, 21 Feb 2026 11:30:07 +0800 (CST)
<hu.shengming@zte.com.cn> wrote:
> From: Shengming Hu <hu.shengming@zte.com.cn>
>
> When tracing_thresh is enabled, function graph tracing uses
> trace_graph_thresh_return() as the return handler. Unlike
> trace_graph_return(), it did not clear the per-task
> TRACE_GRAPH_NOTRACE flag set by the entry handler for
> set_graph_notrace addresses. This could leave the task
> permanently in "notrace" state and effectively disable
> function graph tracing for that task.
>
> Mirror trace_graph_return()'s per-task notrace handling by
> clearing TRACE_GRAPH_NOTRACE and returning early when set.
>
Good catch!
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks!
> Fixes: b84214890a9bc ("function_graph: Move graph notrace bit to
> shadow stack global var")
> Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> ---
> kernel/trace/trace_functions_graph.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index 1de6f1573..cbe43680c 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -400,14 +400,15 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
> struct fgraph_ops *gops,
> struct ftrace_regs *fregs)
> {
> + unsigned long *task_var = fgraph_get_task_var(gops);
> struct fgraph_times *ftimes;
> struct trace_array *tr;
> int size;
>
> ftrace_graph_addr_finish(gops, trace);
>
> - if (trace_recursion_test(TRACE_GRAPH_NOTRACE_BIT)) {
> - trace_recursion_clear(TRACE_GRAPH_NOTRACE_BIT);
> + if (*task_var & TRACE_GRAPH_NOTRACE) {
> + *task_var &= ~TRACE_GRAPH_NOTRACE;
> return;
> }
>
> --
> 2.25.1
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH v4 3/3] ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer
From: Masami Hiramatsu (Google) @ 2026-02-24 6:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <177191595057.435720.10116014529518464418.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Skip invalid sub-buffers when validating the persistent ring buffer
instead of discarding the entire ring buffer. Also, mark there are
missed events on the discarded buffer.
If the cache data in memory fails to be synchronized during a reboot,
the persistent ring buffer may become partially corrupted, but other
sub-buffers may still contain readable event data. Only discard the
subbuffersa that ar found to be corrupted.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v3:
- Record missed data event on commit.
---
kernel/trace/ring_buffer.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 67bc652ba02a..72f82e2e6740 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2058,17 +2058,18 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
if (ret < 0) {
pr_info("Ring buffer meta [%d] invalid buffer page\n",
cpu_buffer->cpu);
- goto invalid;
- }
-
- /* If the buffer has content, update pages_touched */
- if (ret)
- local_inc(&cpu_buffer->pages_touched);
-
- entries += ret;
- entry_bytes += rb_page_size(head_page);
- local_set(&cpu_buffer->head_page->entries, ret);
+ /* Instead of discard whole ring buffer, discard only this sub-buffer. */
+ local_set(&head_page->entries, 0);
+ local_set(&head_page->page->commit, RB_MISSED_EVENTS);
+ } else {
+ /* If the buffer has content, update pages_touched */
+ if (ret)
+ local_inc(&cpu_buffer->pages_touched);
+ entries += ret;
+ entry_bytes += rb_page_size(head_page);
+ local_set(&cpu_buffer->head_page->entries, ret);
+ }
if (head_page == cpu_buffer->commit_page)
break;
}
^ permalink raw reply related
* [PATCH v4 2/3] ring-buffer: Handle RB_MISSED_* flags on commit field correctly
From: Masami Hiramatsu (Google) @ 2026-02-24 6:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <177191595057.435720.10116014529518464418.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Since the MSBs of rb_data_page::commit are used for storing
RB_MISSED_EVENTS and RB_MISSED_STORED, we need to mask out those bits
when it is used for finding the size of data pages.
Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
Fixes: 5b7be9c709e1 ("ring-buffer: Add test to validate the time stamp deltas")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v4:
- Fix to move rb_commit_index() after ring_buffer_per_cpu definition.
---
kernel/trace/ring_buffer.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 3eb124c93d72..67bc652ba02a 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -395,6 +395,12 @@ static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
return local_read(&bpage->page->commit);
}
+/* Size is determined by what has been committed */
+static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
+{
+ return rb_page_commit(bpage) & ~RB_MISSED_MASK;
+}
+
static void free_buffer_page(struct buffer_page *bpage)
{
/* Range pages are not to be freed */
@@ -614,6 +620,12 @@ struct ring_buffer_iter {
int missed_events;
};
+static __always_inline unsigned
+rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
+{
+ return rb_page_commit(cpu_buffer->commit_page);
+}
+
int ring_buffer_print_page_header(struct trace_buffer *buffer, struct trace_seq *s)
{
struct buffer_data_page field;
@@ -1907,7 +1919,7 @@ static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu)
u64 delta;
int tail;
- tail = local_read(&dpage->commit);
+ tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
}
@@ -1934,7 +1946,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
goto invalid;
}
entries += ret;
- entry_bytes += local_read(&cpu_buffer->reader_page->page->commit);
+ entry_bytes += rb_page_size(cpu_buffer->reader_page);
local_set(&cpu_buffer->reader_page->entries, ret);
ts = head_page->page->time_stamp;
@@ -2054,7 +2066,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
local_inc(&cpu_buffer->pages_touched);
entries += ret;
- entry_bytes += local_read(&head_page->page->commit);
+ entry_bytes += rb_page_size(head_page);
local_set(&cpu_buffer->head_page->entries, ret);
if (head_page == cpu_buffer->commit_page)
@@ -3257,18 +3269,6 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
return NULL;
}
-/* Size is determined by what has been committed */
-static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
-{
- return rb_page_commit(bpage) & ~RB_MISSED_MASK;
-}
-
-static __always_inline unsigned
-rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
-{
- return rb_page_commit(cpu_buffer->commit_page);
-}
-
static __always_inline unsigned
rb_event_index(struct ring_buffer_per_cpu *cpu_buffer, struct ring_buffer_event *event)
{
@@ -4433,7 +4433,7 @@ static void check_buffer(struct ring_buffer_per_cpu *cpu_buffer,
if (tail == CHECK_FULL_PAGE) {
full = true;
- tail = local_read(&bpage->commit);
+ tail = local_read(&bpage->commit) & ~RB_MISSED_MASK;
} else if (info->add_timestamp &
(RB_ADD_STAMP_FORCE | RB_ADD_STAMP_ABSOLUTE)) {
/* Ignore events with absolute time stamps */
^ permalink raw reply related
* [PATCH v4 1/3] ring-buffer: Flush and stop persistent ring buffer on panic
From: Masami Hiramatsu (Google) @ 2026-02-24 6:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <177191595057.435720.10116014529518464418.stgit@mhiramat.tok.corp.google.com>
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
On real hardware, panic and machine reboot may not flush hardware cache
to memory. This means the persistent ring buffer, which relies on a
coherent state of memory, may not have its events written to the buffer
and they may be lost. Moreover, there may be inconsistency with the
counters which are used for validation of the integrity of the
persistent ring buffer which may cause all data to be discarded.
To avoid this issue, stop recording of the ring buffer on panic and
flush the cache of the ring buffer's memory.
Fixes: e645535a954a ("tracing: Add option to use memmapped memory for trace boot instance")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
Changes in v3:
- update patch description.
---
kernel/trace/ring_buffer.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index f16f053ef77d..3eb124c93d72 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -6,6 +6,7 @@
*/
#include <linux/sched/isolation.h>
#include <linux/trace_recursion.h>
+#include <linux/panic_notifier.h>
#include <linux/trace_events.h>
#include <linux/ring_buffer.h>
#include <linux/trace_clock.h>
@@ -589,6 +590,7 @@ struct trace_buffer {
unsigned long range_addr_start;
unsigned long range_addr_end;
+ struct notifier_block flush_nb;
struct ring_buffer_meta *meta;
@@ -2471,6 +2473,16 @@ static void rb_free_cpu_buffer(struct ring_buffer_per_cpu *cpu_buffer)
kfree(cpu_buffer);
}
+static int rb_flush_buffer_cb(struct notifier_block *nb, unsigned long event, void *data)
+{
+ struct trace_buffer *buffer = container_of(nb, struct trace_buffer, flush_nb);
+
+ ring_buffer_record_disable(buffer);
+ flush_kernel_vmap_range((void *)buffer->range_addr_start,
+ buffer->range_addr_end - buffer->range_addr_start);
+ return NOTIFY_DONE;
+}
+
static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
int order, unsigned long start,
unsigned long end,
@@ -2590,6 +2602,12 @@ static struct trace_buffer *alloc_buffer(unsigned long size, unsigned flags,
mutex_init(&buffer->mutex);
+ /* Persistent ring buffer needs to flush cache before reboot. */
+ if (start & end) {
+ buffer->flush_nb.notifier_call = rb_flush_buffer_cb;
+ atomic_notifier_chain_register(&panic_notifier_list, &buffer->flush_nb);
+ }
+
return_ptr(buffer);
fail_free_buffers:
@@ -2677,6 +2695,9 @@ ring_buffer_free(struct trace_buffer *buffer)
{
int cpu;
+ if (buffer->range_addr_start && buffer->range_addr_end)
+ atomic_notifier_chain_unregister(&panic_notifier_list, &buffer->flush_nb);
+
cpuhp_state_remove_instance(CPUHP_TRACE_RB_PREPARE, &buffer->node);
irq_work_sync(&buffer->irq_work.work);
^ permalink raw reply related
* [PATCH v4 0/3] ring-buffer: Making persistent ring buffers robust
From: Masami Hiramatsu (Google) @ 2026-02-24 6:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
Hi,
Here is the 4th version of improvement patches for making persistent
ring buffers robust to failures.
The previous version is here:
https://lore.kernel.org/linux-trace-kernel/177186335195.133407.907308822749006594.stgit@devnote2/
In this version, I fixed to move rb_commit_index() after the
definition of ring_buffer_per_cpu[2/3].
Thank you,
---
Masami Hiramatsu (Google) (3):
ring-buffer: Flush and stop persistent ring buffer on panic
ring-buffer: Handle RB_MISSED_* flags on commit field correctly
ring-buffer: Skip invalid sub-buffers when validating persistent ring buffer
kernel/trace/ring_buffer.c | 72 +++++++++++++++++++++++++++++---------------
1 file changed, 47 insertions(+), 25 deletions(-)
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 2/3] ring-buffer: Handle RB_MISSED_* flags on commit field correctly
From: Masami Hiramatsu @ 2026-02-24 6:25 UTC (permalink / raw)
To: Masami Hiramatsu (Google)
Cc: Steven Rostedt, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel
In-Reply-To: <177186337098.133407.17816322157924593847.stgit@devnote2>
On Tue, 24 Feb 2026 01:16:11 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Since the MSBs of rb_data_page::commit are used for storing
> RB_MISSED_EVENTS and RB_MISSED_STORED, we need to mask out those bits
> when it is used for finding the size of data pages.
>
> Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
> Fixes: 5b7be9c709e1 ("ring-buffer: Add test to validate the time stamp deltas")
> Cc: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> Changes in v3:
> - Newly added.
> ---
> kernel/trace/ring_buffer.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 84a6459ed494..1ef718d21796 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -395,6 +395,18 @@ static __always_inline unsigned int rb_page_commit(struct buffer_page *bpage)
> return local_read(&bpage->page->commit);
> }
>
> +/* Size is determined by what has been committed */
> +static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
> +{
> + return rb_page_commit(bpage) & ~RB_MISSED_MASK;
> +}
> +
> +static __always_inline unsigned
> +rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
> +{
> + return rb_page_commit(cpu_buffer->commit_page);
> +}
> +
> static void free_buffer_page(struct buffer_page *bpage)
> {
> /* Range pages are not to be freed */
> @@ -1907,7 +1919,7 @@ static int rb_validate_buffer(struct buffer_data_page *dpage, int cpu)
> u64 delta;
> int tail;
>
> - tail = local_read(&dpage->commit);
> + tail = local_read(&dpage->commit) & ~RB_MISSED_MASK;
> return rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
> }
>
> @@ -1934,7 +1946,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> goto invalid;
> }
> entries += ret;
> - entry_bytes += local_read(&cpu_buffer->reader_page->page->commit);
> + entry_bytes += rb_page_size(cpu_buffer->reader_page);
> local_set(&cpu_buffer->reader_page->entries, ret);
>
> ts = head_page->page->time_stamp;
> @@ -2054,7 +2066,7 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
> local_inc(&cpu_buffer->pages_touched);
>
> entries += ret;
> - entry_bytes += local_read(&head_page->page->commit);
> + entry_bytes += rb_page_size(head_page);
> local_set(&cpu_buffer->head_page->entries, ret);
>
> if (head_page == cpu_buffer->commit_page)
> @@ -3257,18 +3269,6 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
> return NULL;
> }
>
> -/* Size is determined by what has been committed */
> -static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
> -{
> - return rb_page_commit(bpage) & ~RB_MISSED_MASK;
> -}
> -
> -static __always_inline unsigned
> -rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
Oops, this data structure is defined after this line.
Let me fix it.
> -{
> - return rb_page_commit(cpu_buffer->commit_page);
> -}
> -
> static __always_inline unsigned
> rb_event_index(struct ring_buffer_per_cpu *cpu_buffer, struct ring_buffer_event *event)
> {
> @@ -4433,7 +4433,7 @@ static void check_buffer(struct ring_buffer_per_cpu *cpu_buffer,
>
> if (tail == CHECK_FULL_PAGE) {
> full = true;
> - tail = local_read(&bpage->commit);
> + tail = local_read(&bpage->commit) & ~RB_MISSED_MASK;
> } else if (info->add_timestamp &
> (RB_ADD_STAMP_FORCE | RB_ADD_STAMP_ABSOLUTE)) {
> /* Ignore events with absolute time stamps */
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [LSF/MM/BPF TOPIC][RFC PATCH v4 00/27] Private Memory Nodes (w/ Compressed RAM)
From: Alistair Popple @ 2026-02-24 6:19 UTC (permalink / raw)
To: Gregory Price
Cc: lsf-pc, linux-kernel, linux-cxl, cgroups, linux-mm,
linux-trace-kernel, damon, kernel-team, gregkh, rafael, dakr,
dave, jonathan.cameron, dave.jiang, alison.schofield,
vishal.l.verma, ira.weiny, dan.j.williams, longman, akpm, david,
lorenzo.stoakes, Liam.Howlett, vbabka, rppt, surenb, mhocko,
osalvador, ziy, matthew.brost, joshua.hahnjy, rakie.kim,
byungchul, ying.huang, axelrasmussen, yuanchu, weixugc,
yury.norov, linux, mhiramat, mathieu.desnoyers, tj, hannes,
mkoutny, jackmanb, sj, baolin.wang, npache, ryan.roberts,
dev.jain, baohua, lance.yang, muchun.song, xu.xin16,
chengming.zhou, jannh, linmiaohe, nao.horiguchi, pfalcato,
rientjes, shakeel.butt, riel, harry.yoo, cl, roman.gushchin,
chrisl, kasong, shikemeng, nphamcs, bhe, zhengqi.arch,
terry.bowman
In-Reply-To: <20260222084842.1824063-1-gourry@gourry.net>
On 2026-02-22 at 19:48 +1100, Gregory Price <gourry@gourry.net> wrote...
> Topic type: MM
>
> Presenter: Gregory Price <gourry@gourry.net>
>
> This series introduces N_MEMORY_PRIVATE, a NUMA node state for memory
> managed by the buddy allocator but excluded from normal allocations.
>
> I present it with an end-to-end Compressed RAM service (mm/cram.c)
> that would otherwise not be possible (or would be considerably more
> difficult, be device-specific, and add to the ZONE_DEVICE boondoggle).
>
>
> TL;DR
> ===
>
> N_MEMORY_PRIVATE is all about isolating NUMA nodes and then punching
> explicit holes in that isolation to do useful things we couldn't do
> before without re-implementing entire portions of mm/ in a driver.
Having had to re-implement entire portions of mm/ in a driver I agree this isn't
something anyone sane should do :-) However aspects of ZONE_DEVICE were added
precisely to help with that so I'm not sure N_MEMORY_PRIVATE is the only or best
way to do that.
Based on our discussion at LPC I believe one of the primary motivators here was
to re-use the existing mm buddy allocator rather than writing your own. I remain
to be convinced that alone is justification enough for doing all this - DRM for
example already has quite a nice standalone buddy allocator (drm_buddy.c) that
could presumably be used, or adapted for use, by any device driver.
The interesting part of this series (which I have skimmed but not read in
detail) is how device memory gets exposed to userspace - this is something that
existing ZONE_DEVICE implementations don't address, instead leaving it up to
drivers and associated userspace stacks to deal with allocation, migration, etc.
>
>
> /* This is my memory. There are many like it, but this one is mine. */
> rc = add_private_memory_driver_managed(nid, start, size, name, flags,
> online_type, private_context);
>
> page = alloc_pages_node(nid, __GFP_PRIVATE, 0);
>
> /* Ok but I want to do something useful with it */
> static const struct node_private_ops ops = {
> .migrate_to = my_migrate_to,
> .folio_migrate = my_folio_migrate,
> .flags = NP_OPS_MIGRATION | NP_OPS_MEMPOLICY,
> };
> node_private_set_ops(nid, &ops);
>
> /* And now I can use mempolicy with my memory */
> buf = mmap(...);
> mbind(buf, len, mode, private_node, ...);
> buf[0] = 0xdeadbeef; /* Faults onto private node */
>
> /* And to be clear, no one else gets my memory */
> buf2 = malloc(4096); /* Standard allocation */
> buf2[0] = 0xdeadbeef; /* Can never land on private node */
>
> /* But i can choose to migrate it to the private node */
> move_pages(0, 1, &buf, &private_node, NULL, ...);
>
> /* And more fun things like this */
This is I think is one of the key things that should be enabled - providing a
standard interface to userspace for managing device memory. The existing NUMA
APIs do seem like a reasonable way to do this.
> Patchwork
> ===
> A fully working branch based on cxl/next can be found here:
> https://github.com/gourryinverse/linux/tree/private_compression
>
> A QEMU device which can inject high/low interrupts can be found here:
> https://github.com/gourryinverse/qemu/tree/compressed_cxl_clean
>
> The additional patches on these branches are CXL and DAX driver
> housecleaning only tangentially relevant to this RFC, so i've
> omitted them for the sake of trying to keep it somewhat clean
> here. Those patches should (hopefully) be going upstream anyway.
>
> Patches 1-22: Core Private Node Infrastructure
>
> Patch 1: Introduce N_MEMORY_PRIVATE scaffolding
> Patch 2: Introduce __GFP_PRIVATE
> Patch 3: Apply allocation isolation mechanisms
> Patch 4: Add N_MEMORY nodes to private fallback lists
> Patches 5-9: Filter operations not yet supported
> Patch 10: free_folio callback
> Patch 11: split_folio callback
> Patches 12-20: mm/ service opt-ins:
> Migration, Mempolicy, Demotion, Write Protect,
> Reclaim, OOM, NUMA Balancing, Compaction,
> LongTerm Pinning
> Patch 21: memory_failure callback
> Patch 22: Memory hotplug plumbing for private nodes
>
> Patch 23: mm/cram -- Compressed RAM Management
>
> Patches 24-27: CXL Driver examples
> Sysram Regions with Private node support
> Basic Driver Example: (MIGRATION | MEMPOLICY)
> Compression Driver Example (Generic)
>
>
> Background
> ===
>
> Today, drivers that want mm-like services on non-general-purpose
> memory either use ZONE_DEVICE (self-managed memory) or hotplug into
> N_MEMORY and accept the risk of uncontrolled allocation.
>
> Neither option provides what we really want - the ability to:
> 1) selectively participate in mm/ subsystems, while
> 2) isolating that memory from general purpose use.
>
> Some device-attached memory cannot be managed as fully general-purpose
> system RAM. CXL devices with inline compression, for example, may
> corrupt data or crash the machine if the compression ratio drops
> below a threshold -- we simply run out of physical memory.
>
> This is a hard problem to solve: how does an operating system deal
> with a device that basically lies about how much capacity it has?
>
> (We'll discuss that in the CRAM section)
>
>
> Core Proposal: N_MEMORY_PRIVATE
> ===
>
> Introduce N_MEMORY_PRIVATE, a NUMA node state for memory managed by
> the buddy allocator, but excluded from normal allocation paths.
>
> Private nodes:
>
> - Are filtered from zonelist fallback: all existing callers to
> get_page_from_freelist cannot reach these nodes through any
> normal fallback mechanism.
>
> - Filter allocation requests on __GFP_PRIVATE
> numa_zone_allowed() excludes them otherwise.
>
> Applies to systems with and without cpusets.
>
> GFP_PRIVATE is (__GFP_PRIVATE | __GFP_THISNODE).
>
> Services use it when they need to allocate specifically from
> a private node (e.g., CRAM allocating a destination folio).
>
> No existing allocator path sets __GFP_PRIVATE, so private nodes
> are unreachable by default.
>
> - Use standard struct page / folio. No ZONE_DEVICE, no pgmap,
> no struct page metadata limitations.
>
> - Use a node-scoped metadata structure to accomplish filtering
> and callback support.
>
> - May participate in the buddy allocator, reclaim, compaction,
> and LRU like normal memory, gated by an opt-in set of flags.
>
> The key abstraction is node_private_ops: a per-node callback table
> registered by a driver or service.
>
> Each callback is individually gated by an NP_OPS_* capability flag.
>
> A driver opts in only to the mm/ operations it needs.
>
> It is similar to ZONE_DEVICE's pgmap at a node granularity.
>
> In fact...
>
>
> Re-use of ZONE_DEVICE Hooks
> ===
>
> The callback insertion points deliberately mirror existing ZONE_DEVICE
> hooks to minimize the surface area of the mechanism.
>
> I believe this could subsume most DEVICE_COHERENT users, and greatly
> simplify the device-managed memory development process (no more
> per-driver allocator and migration code).
>
> (Also it's just "So Fresh, So Clean").
>
> The base set of callbacks introduced include:
>
> free_folio - mirrors ZONE_DEVICE's
> free_zone_device_page() hook in
> __folio_put() / folios_put_refs()
>
> folio_split - mirrors ZONE_DEVICE's
> called when a huge page is split up
>
> migrate_to - demote_folio_list() custom demotion (same
> site as ZONE_DEVICE demotion rejection)
>
> folio_migrate - called when private node folio is moved to
> another location (e.g. compaction)
>
> handle_fault - mirrors the ZONE_DEVICE fault dispatch in
> handle_pte_fault() (do_wp_page path)
>
> reclaim_policy - called by reclaim to let a driver own the
> boost lifecycle (driver can driver node reclaim)
>
> memory_failure - parallels memory_failure_dev_pagemap(),
> but for online pages that enter the normal
> hwpoison path
One does not have to squint too hard to see that the above is not so different
from what ZONE_DEVICE provides today via dev_pagemap_ops(). So I think I think
it would be worth outlining why the existing ZONE_DEVICE mechanism can't be
extended to provide these kind of services.
This seems to add a bunch of code just to use NODE_DATA instead of page->pgmap,
without really explaining why just extending dev_pagemap_ops wouldn't work. The
obvious reason is that if you want to support things like reclaim, compaction,
etc. these pages need to be on the LRU, which is a little bit hard when that
field is also used by the pgmap pointer for ZONE_DEVICE pages.
But it might be good to explore other options for storing the pgmap - for
example page_ext could be used. Or I hear struct page may go away in place of
folios any day now, so maybe that gives us space for both :-)
> At skip sites (mlock, madvise, KSM, user migration), a unified
> folio_is_private_managed() predicate covers both ZONE_DEVICE and
> N_MEMORY_PRIVATE folios, consolidating existing zone_device checks
> with private node checks rather than adding new ones.
>
> static inline bool folio_is_private_managed(struct folio *folio)
> {
> return folio_is_zone_device(folio) ||
> folio_is_private_node(folio);
> }
>
> Most integration points become a one-line swap:
>
> - if (folio_is_zone_device(folio))
> + if (unlikely(folio_is_private_managed(folio)))
>
>
> Where a one-line integration is insufficient, the integration is
> kept as clean as possible with zone_device, rather than simply
> adding more call-sites on top of it:
>
> static inline bool folio_managed_handle_fault(struct folio *folio,
> struct vm_fault *vmf, vm_fault_t *ret)
> {
> /* Zone device pages use swap entries; handled in do_swap_page */
> if (folio_is_zone_device(folio))
> return false;
>
> if (folio_is_private_node(folio)) {
> const struct node_private_ops *ops = folio_node_private_ops(folio);
>
> if (ops && ops->handle_fault) {
> *ret = ops->handle_fault(vmf);
> return true;
> }
> }
> return false;
> }
>
>
>
> Flag-gated behavior (NP_OPS_*) controls:
> ===
>
> We use OPS flags to denote what mm/ services we want to allow on our
> private node. I've plumbed these through so far:
>
> NP_OPS_MIGRATION - Node supports migration
> NP_OPS_MEMPOLICY - Node supports mempolicy actions
> NP_OPS_DEMOTION - Node appears in demotion target lists
> NP_OPS_PROTECT_WRITE - Node memory is read-only (wrprotect)
> NP_OPS_RECLAIM - Node supports reclaim
> NP_OPS_NUMA_BALANCING - Node supports numa balancing
> NP_OPS_COMPACTION - Node supports compaction
> NP_OPS_LONGTERM_PIN - Node supports longterm pinning
> NP_OPS_OOM_ELIGIBLE - (MIGRATION | DEMOTION), node is reachable
> as normal system ram storage, so it should
> be considered in OOM pressure calculations.
>
> I wasn't quite sure how to classify ksm, khugepaged, madvise, and
> mlock - so i have omitted those for now.
>
> Most hooks are straightforward.
>
> Including a node as a demotion-eligible target was as simple as:
>
> static void establish_demotion_targets(void)
> {
> ..... snip .....
> /*
> * Include private nodes that have opted in to demotion
> * via NP_OPS_DEMOTION. A node might have custom migrate
> */
> all_memory = node_states[N_MEMORY];
> for_each_node_state(node, N_MEMORY_PRIVATE) {
> if (node_private_has_flag(node, NP_OPS_DEMOTION))
> node_set(node, all_memory);
> }
> ..... snip .....
> }
>
> The Migration and Mempolicy support are the two most complex pieces,
> and most useful things are built on top of Migration (meaning the
> remaining implementations are usually simple).
>
>
> Private Node Hotplug Lifecycle
> ===
>
> Registration follows a strict order enforced by
> add_private_memory_driver_managed():
>
> 1. Driver calls add_private_memory_driver_managed(nid, start,
> size, resource_name, mhp_flags, online_type, &np).
>
> 2. node_private_register(nid, &np) stores the driver's
> node_private in pgdat and sets pgdat->private. N_MEMORY and
> N_MEMORY_PRIVATE are mutually exclusive -- registration fails
> with -EBUSY if the node already has N_MEMORY set.
>
> Only one driver may register per private node.
>
> 3. Memory is hotplugged via __add_memory_driver_managed().
>
> When online_pages() runs, it checks pgdat->private and sets
> N_MEMORY_PRIVATE instead of N_MEMORY.
>
> Zonelist construction gives private nodes a self-only NOFALLBACK
> list and an N_MEMORY fallback list (so kernel/slab allocations on
> behalf of private node work can fall back to DRAM).
>
> 4. kswapd and kcompactd are NOT started for private nodes. The
> owning service is responsible for driving reclaim if needed
> (e.g., CRAM uses watermark_boost to wake kswapd on demand).
>
> Teardown is the reverse:
>
> 1. Driver calls offline_and_remove_private_memory(nid, start,
> size).
>
> 2. offline_pages() offlines the memory. When the last block is
> offlined, N_MEMORY_PRIVATE is cleared automatically.
>
> 3. node_private_unregister() clears pgdat->node_private and
> drops the refcount. It refuses to unregister (-EBUSY) if
> N_MEMORY_PRIVATE is still set (other memory ranges remain).
>
> The driver is responsible for ensuring memory is hot-unpluggable
> before teardown. The service must ensure all memory is cleaned
> up before hot-unplug - or the service must support migration (so
> memory_hotplug.c can evacuate the memory itself).
>
> In the CRAM example, the service supports migration, so memory
> hot-unplug can remove memory without any special infrastructure.
The above also looks pretty similar to the existing ZONE_DEVICE methods for
doing this which is another reason to argue for just building up the feature set
of the existing boondoggle rather than adding another thingymebob.
It seems the key thing we are looking for is:
1) A userspace API to allocate/manage device memory (ie. move_pages(), mbind(),
etc.)
2) Allowing reclaim/LRU list processing of device memory.
From my perspective both of these are interesting and I look forward to the
discussion (hopefully I can make it to LSFMM). Mostly I'm interested in the
implementation as this does on the surface seem to sprinkle around and duplicate
a lot of hooks similar to what ZONE_DEVICE already provides.
> Application: Compressed RAM (mm/cram)
> ===
>
> Compressed RAM has a serious design issue: Its capacity a lie.
>
> A compression device reports more capacity than it physically has.
> If workloads write faster than the OS can reclaim from the device,
> we run out of real backing store and corrupt data or crash.
>
> I call this problem: "Trying to Out Run A Bear"
>
> I.e. This is only stable as long as we stay ahead of the pressure.
>
> We don't want to design a system where stability depends on outrunning
> a bear - I am slow and do not know where to acquire bear spray.
>
> Fun fact: Grizzly bears have a top-speed of 56-64 km/h.
> Unfun Fact: Humans typically top out at ~24 km/h.
>
> This MVP takes a conservative position:
>
> all compressed memory is mapped read-only.
>
> - Folios reach the private node only via reclaim (demotion)
> - migrate_to implements custom demotion with backpressure.
> - fixup_migration_pte write-protects PTEs on arrival.
> - wrprotect hooks prevent silent upgrades
> - handle_fault promotes folios back to DRAM on write.
> - free_folio scrubs stale data before buddy free.
>
> Because pages are read-only, writes can never cause runaway
> compression ratio loss behind the allocator's back. Every write
> goes through handle_fault, which promotes the folio to DRAM first.
>
> The device only ever sees net compression (demotion in) and explicit
> decompression (promotion out via fault or reclaim), and has a much
> wider timeframe to respond to poor compression scenarios.
>
> That means there's no bear to out run. The bears are safely asleep in
> their bear den, and even if they show up we have a bear-proof cage.
>
> The backpressure system is our bear-proof cage: the driver reports real
> device utilization (generalized via watermark_boost on the private
> node's zone), and CRAM throttles demotion when capacity is tight.
>
> If compression ratios are bad, we stop demoting pages and start
> evicting pages aggressively.
>
> The service as designed is ~350 functional lines of code because it
> re-uses mm/ services:
>
> - Existing reclaim/vmscan code handles demotion.
> - Existing migration code handles migration to/from.
> - Existing page fault handling dispatches faults.
>
> The driver contains all the CXL nastiness core developers don't want
> anything to do with - No vendor logic touches mm/ internals.
>
>
>
> Future CRAM : Loosening the read-only constraint
> ===
>
> The read-only model is safe but conservative. For workloads where
> compressed pages are occasionally written, the promotion fault adds
> latency. A future optimization could allow a tunable fraction of
> compressed pages to be mapped writable, accepting some risk of
> write-driven decompression in exchange for lower overhead.
>
> The private node ops make this straightforward:
>
> - Adjust fixup_migration_pte to selectively skip
> write-protection.
> - Use the backpressure system to either revoke writable mappings,
> deny additional demotions, or evict when device pressure rises.
>
> This comes at a mild memory overhead: 32MB of DRAM per 1TB of CRAM.
> (1 bit per 4KB page).
>
> This is not proposed here, but it should be somewhat trivial.
>
>
> Discussion Topics
> ===
> 0. Obviously I've included the set as an RFC, please rip it apart.
>
> 1. Is N_MEMORY_PRIVATE the right isolation abstraction, or should
> this extend ZONE_DEVICE? Prior feedback pushed away from new
> ZONE logic, but this will likely be debated further.
>
> My comments on this:
>
> ZONE_DEVICE requires re-implementing every service you want to
> provide to your device memory, including basic allocation.
For basic allocation I agree this is the case. But there's no reason some device
allocator library couldn't be written. Or in fact as pointed out above reuse the
already existing one in drm_buddy.c. So would be interested to hear arguments
for why allocation has to be done by the mm allocator and/or why an allocation
library wouldn't work here given DRM already has them.
> Private nodes use real struct pages with no metadata
> limitations, participate in the buddy allocator, and get NUMA
> topology for free.
ZONE_DEVICE pages are in fact real struct pages, but I will concede that
perspective probably depends on which bits of the mm you play in. The real
limitations you seem to be addressing is more around how we get these pages in
an LRU, or are there other limitations?
> 2. Can this subsume ZONE_DEVICE COHERENT users? The architecture
> was designed with this in mind, but it is only a thought experiment.
What I'd like to explore is why ZONE_DEVICE_COHERENT couldn't just be extended
to support your usecase? It seems a couple of extra dev_pagemap_ops and being
able to go on the LRU would get you there.
- Alistair
> 3. Is a dedicated mm/ service (cram) the right place for compressed
> memory management, or should this be purely driver-side until
> more devices exist?
>
> I wrote it this way because I forsee more "innovation" in the
> compressed RAM space given current... uh... "Market Conditions".
>
> I don't see CRAM being CXL-specific, though the only solutions I've
> seen have been CXL. Nothing is stopping someone from soldering such
> memory directly to a PCB.
>
> 5. Where is your hardware-backed data that shows this works?
>
> I should have some by conference time.
>
> Thanks for reading
> Gregory (Gourry)
>
>
> Gregory Price (27):
> numa: introduce N_MEMORY_PRIVATE node state
> mm,cpuset: gate allocations from N_MEMORY_PRIVATE behind __GFP_PRIVATE
> mm/page_alloc: add numa_zone_allowed() and wire it up
> mm/page_alloc: Add private node handling to build_zonelists
> mm: introduce folio_is_private_managed() unified predicate
> mm/mlock: skip mlock for managed-memory folios
> mm/madvise: skip madvise for managed-memory folios
> mm/ksm: skip KSM for managed-memory folios
> mm/khugepaged: skip private node folios when trying to collapse.
> mm/swap: add free_folio callback for folio release cleanup
> mm/huge_memory.c: add private node folio split notification callback
> mm/migrate: NP_OPS_MIGRATION - support private node user migration
> mm/mempolicy: NP_OPS_MEMPOLICY - support private node mempolicy
> mm/memory-tiers: NP_OPS_DEMOTION - support private node demotion
> mm/mprotect: NP_OPS_PROTECT_WRITE - gate PTE/PMD write-upgrades
> mm: NP_OPS_RECLAIM - private node reclaim participation
> mm/oom: NP_OPS_OOM_ELIGIBLE - private node OOM participation
> mm/memory: NP_OPS_NUMA_BALANCING - private node NUMA balancing
> mm/compaction: NP_OPS_COMPACTION - private node compaction support
> mm/gup: NP_OPS_LONGTERM_PIN - private node longterm pin support
> mm/memory-failure: add memory_failure callback to node_private_ops
> mm/memory_hotplug: add add_private_memory_driver_managed()
> mm/cram: add compressed ram memory management subsystem
> cxl/core: Add cxl_sysram region type
> cxl/core: Add private node support to cxl_sysram
> cxl: add cxl_mempolicy sample PCI driver
> cxl: add cxl_compression PCI driver
>
> drivers/base/node.c | 250 +++-
> drivers/cxl/Kconfig | 2 +
> drivers/cxl/Makefile | 2 +
> drivers/cxl/core/Makefile | 1 +
> drivers/cxl/core/core.h | 4 +
> drivers/cxl/core/port.c | 2 +
> drivers/cxl/core/region_sysram.c | 381 ++++++
> drivers/cxl/cxl.h | 53 +
> drivers/cxl/type3_drivers/Kconfig | 3 +
> drivers/cxl/type3_drivers/Makefile | 3 +
> .../cxl/type3_drivers/cxl_compression/Kconfig | 20 +
> .../type3_drivers/cxl_compression/Makefile | 4 +
> .../cxl_compression/compression.c | 1025 +++++++++++++++++
> .../cxl/type3_drivers/cxl_mempolicy/Kconfig | 16 +
> .../cxl/type3_drivers/cxl_mempolicy/Makefile | 4 +
> .../type3_drivers/cxl_mempolicy/mempolicy.c | 297 +++++
> include/linux/cpuset.h | 9 -
> include/linux/cram.h | 66 ++
> include/linux/gfp_types.h | 15 +-
> include/linux/memory-tiers.h | 9 +
> include/linux/memory_hotplug.h | 11 +
> include/linux/migrate.h | 17 +-
> include/linux/mm.h | 22 +
> include/linux/mmzone.h | 16 +
> include/linux/node_private.h | 532 +++++++++
> include/linux/nodemask.h | 1 +
> include/trace/events/mmflags.h | 4 +-
> include/uapi/linux/mempolicy.h | 1 +
> kernel/cgroup/cpuset.c | 49 +-
> mm/Kconfig | 10 +
> mm/Makefile | 1 +
> mm/compaction.c | 32 +-
> mm/cram.c | 508 ++++++++
> mm/damon/paddr.c | 3 +
> mm/huge_memory.c | 23 +-
> mm/hugetlb.c | 2 +-
> mm/internal.h | 226 +++-
> mm/khugepaged.c | 7 +-
> mm/ksm.c | 9 +-
> mm/madvise.c | 5 +-
> mm/memory-failure.c | 15 +
> mm/memory-tiers.c | 46 +-
> mm/memory.c | 26 +
> mm/memory_hotplug.c | 122 +-
> mm/mempolicy.c | 69 +-
> mm/migrate.c | 63 +-
> mm/mlock.c | 5 +-
> mm/mprotect.c | 4 +-
> mm/oom_kill.c | 52 +-
> mm/page_alloc.c | 79 +-
> mm/rmap.c | 4 +-
> mm/slub.c | 3 +-
> mm/swap.c | 21 +-
> mm/vmscan.c | 55 +-
> 54 files changed, 4057 insertions(+), 152 deletions(-)
> create mode 100644 drivers/cxl/core/region_sysram.c
> create mode 100644 drivers/cxl/type3_drivers/Kconfig
> create mode 100644 drivers/cxl/type3_drivers/Makefile
> create mode 100644 drivers/cxl/type3_drivers/cxl_compression/Kconfig
> create mode 100644 drivers/cxl/type3_drivers/cxl_compression/Makefile
> create mode 100644 drivers/cxl/type3_drivers/cxl_compression/compression.c
> create mode 100644 drivers/cxl/type3_drivers/cxl_mempolicy/Kconfig
> create mode 100644 drivers/cxl/type3_drivers/cxl_mempolicy/Makefile
> create mode 100644 drivers/cxl/type3_drivers/cxl_mempolicy/mempolicy.c
> create mode 100644 include/linux/cram.h
> create mode 100644 include/linux/node_private.h
> create mode 100644 mm/cram.c
>
> --
> 2.53.0
>
^ permalink raw reply
* [PATCH] tracing: Fix syscall events activation by ensuring refcount hits zero
From: Huiwen He @ 2026-02-24 2:35 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, linux-trace-kernel, linux-kernel, Huiwen He
When multiple syscall events are specified in the kernel command line
(e.g., trace_event=syscalls:sys_enter_openat,syscalls:sys_enter_close),
they are often not captured after boot, even though they appear enabled
in the tracing/set_event file.
The issue stems from how syscall events are initialized. Syscall
tracepoints require the global reference count (sys_tracepoint_refcount)
to transition from 0 to 1 to trigger the registration of the syscall
work (TIF_SYSCALL_TRACEPOINT) for tasks, including the init process (pid 1).
The current implementation of early_enable_events() with disable_first=true
used an interleaved sequence of "Disable A -> Enable A -> Disable B -> Enable B".
If multiple syscalls are enabled, the refcount never drops to zero,
preventing the 0->1 transition that triggers actual registration.
Fix this by splitting early_enable_events() into two distinct phases:
1. Disable all events specified in the buffer.
2. Enable all events specified in the buffer.
This ensures the refcount hits zero before re-enabling, allowing syscall
events to be properly activated during early boot.
The code is also refactored to use a helper function to avoid logic
duplication between the disable and enable phases.
Fixes: ce1039bd3a89 ("tracing: Fix enabling of syscall events on the command line")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
---
kernel/trace/trace_events.c | 52 ++++++++++++++++++++++++++-----------
1 file changed, 37 insertions(+), 15 deletions(-)
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 137b4d9bb116..879ed8b0cc78 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -4514,26 +4514,22 @@ static __init int event_trace_memsetup(void)
return 0;
}
-__init void
-early_enable_events(struct trace_array *tr, char *buf, bool disable_first)
+/*
+ * Helper function to enable or disable a comma-separated list of events
+ * from the bootup buffer.
+ */
+static __init void __early_set_events(struct trace_array *tr, char *buf, bool enable)
{
char *token;
- int ret;
-
- while (true) {
- token = strsep(&buf, ",");
-
- if (!token)
- break;
+ while ((token = strsep(&buf, ","))) {
if (*token) {
- /* Restarting syscalls requires that we stop them first */
- if (disable_first)
+ if (enable) {
+ if (ftrace_set_clr_event(tr, token, 1))
+ pr_warn("Failed to enable trace event: %s\n", token);
+ } else {
ftrace_set_clr_event(tr, token, 0);
-
- ret = ftrace_set_clr_event(tr, token, 1);
- if (ret)
- pr_warn("Failed to enable trace event: %s\n", token);
+ }
}
/* Put back the comma to allow this to be called again */
@@ -4542,6 +4538,32 @@ early_enable_events(struct trace_array *tr, char *buf, bool disable_first)
}
}
+/**
+ * early_enable_events - enable events from the bootup buffer
+ * @tr: The trace array to enable the events in
+ * @buf: The buffer containing the comma separated list of events
+ * @disable_first: If true, disable all events in @buf before enabling them
+ *
+ * This function enables events from the bootup buffer. If @disable_first
+ * is true, it will first disable all events in the buffer before enabling
+ * them.
+ *
+ * For syscall events, which rely on a global refcount to register the
+ * SYSCALL_WORK_SYSCALL_TRACEPOINT flag (especially for pid 1), we must
+ * ensure the refcount hits zero before re-enabling them. A simple
+ * "disable then enable" per-event is not enough if multiple syscalls are
+ * used, as the refcount will stay above zero. Thus, we need a two-phase
+ * approach: disable all, then enable all.
+ */
+__init void
+early_enable_events(struct trace_array *tr, char *buf, bool disable_first)
+{
+ if (disable_first)
+ __early_set_events(tr, buf, false);
+
+ __early_set_events(tr, buf, true);
+}
+
static __init int event_trace_enable(void)
{
struct trace_array *tr = top_trace_array();
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox