* Re: [PATCH 0/2] cgroup/dmem: introduce a peak file
From: Michal Koutný @ 2026-05-06 13:53 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: Tejun Heo, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, Jonathan Corbet,
Shuah Khan, Maarten Lankhorst, Maxime Ripard, Natalie Vock,
Tvrtko Ursulin, cgroups, linux-kernel, linux-mm, linux-doc,
dri-devel, kernel-dev
In-Reply-To: <20260506-dmem_peak-v1-0-8d803eb3449c@igalia.com>
[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]
Hello Thadeu.
On Wed, May 06, 2026 at 08:58:23AM -0300, Thadeu Lima de Souza Cascardo <cascardo@igalia.com> wrote:
> Just like we have memory.peak, introduce a dmem.peak, which uses the
> page_counter support for that.
>
> It can be written to in order to reset the peak, but different from
> memory.peak, which expects any write, dmem.peak expects the region name to
> be written to it. That region peak is the one that is reset.
>
> That requires ofp_peak to carry a pointer to the pool that was reset.
(It'd be nicer to have generic data in that generic structure, at least
some void *priv. But see below.)
> Writing a different region name will reset the different region and make
> the original region peak get back to its non-reset value.
I'm slightly confused by this fds x pool matricity when there's only
a single slot in cgroup_file_ctx::cgroup_of_peak.
The intended use case is that users should maintain one fd per pool and
not mix it up?
This stanza would better fit to cgroup-v2.rst proper than the commit
message. Or make it simpler and start with non-resettable peak file
(like memory.peak had started too) and see how it fares. WDYT?
Thanks,
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]
^ permalink raw reply
* [PATCH v2] rust: maple_tree: implement Send and Sync for MapleTree
From: Joel Fernandes @ 2026-05-06 13:51 UTC (permalink / raw)
To: linux-kernel
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Bjorn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Dave Airlie, Daniel Almeida, dri-devel, rust-for-linux, nova-gpu,
Nikola Djukic, David Airlie, Boqun Feng, John Hubbard,
Alistair Popple, Timur Tabi, Edwin Peer, Alexandre Courbot,
Andrea Righi, Andy Ritger, Zhi Wang, Balbir Singh,
Philipp Stanner, alexeyi, Eliot Courtney, joel, linux-doc,
Joel Fernandes
The C maple_tree struct contains a *mut c_void, which prevents Rust from
auto-deriving Send/Sync. Following is an example error message when using
MapleTree in nova-core's Vmm.
This propagates up through MapleTreeAlloc to Vmm, BarUser, Gpu, and NovaCore,
causing NovaCore to fail the Send bound required by pci::Driver:
error[E0277]: `*mut c_void` cannot be sent between threads safely
--> drivers/gpu/nova-core/driver.rs:77:22
|
77 | impl pci::Driver for NovaCore {
| ^^^^^^^^ `*mut c_void` cannot be sent between threads safely
|
= help: within `MapleTreeAlloc<()>`, the trait `Send` is not implemented for `*mut c_void`
note: required because it appears within the type `kernel::bindings::maple_tree`
note: required because it appears within the type `Opaque<kernel::bindings::maple_tree>`
note: required because it appears within the type `MapleTree<()>`
note: required because it appears within the type `MapleTreeAlloc<()>`
= note: required for `Box<MapleTreeAlloc<()>, Kmalloc>` to implement `Send`
note: required because it appears within the type `core::pin::Pin<Box<MapleTreeAlloc<()>, Kmalloc>>`
note: required because it appears within the type `Vmm`
note: required because it appears within the type `BarUser`
note: required because it appears within the type `Gpu`
note: required because it appears within the type `NovaCore`
note: required by a bound in `kernel::pci::Driver`
--> rust/kernel/pci.rs:294:19
Implement Send and Sync for MapleTree. The tree contains no thread-local
state, and all shared access goes through the internal ma_lock spinlock.
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
RFC->v2: Just adjusted a few comments as suggested by Gary.
Sending this separately as discussed in the nova mm patch series that needs it:
https://lore.kernel.org/all/252a4eef-f4f4-4edf-8154-06cae4ad8518@nvidia.com/
rust/kernel/maple_tree.rs | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/rust/kernel/maple_tree.rs b/rust/kernel/maple_tree.rs
index 265d6396a78a..2400c905270d 100644
--- a/rust/kernel/maple_tree.rs
+++ b/rust/kernel/maple_tree.rs
@@ -16,7 +16,11 @@
alloc::Flags,
error::to_result,
prelude::*,
- types::{ForeignOwnable, Opaque},
+ types::{
+ ForeignOwnable,
+ NotThreadSafe,
+ Opaque, //
+ },
};
/// A maple tree optimized for storing non-overlapping ranges.
@@ -240,7 +244,10 @@ pub fn lock(&self) -> MapleGuard<'_, T> {
unsafe { bindings::spin_lock(self.ma_lock()) };
// INVARIANT: We just took the spinlock.
- MapleGuard(self)
+ MapleGuard {
+ tree: self,
+ _not_send: NotThreadSafe,
+ }
}
#[inline]
@@ -302,19 +309,29 @@ fn drop(mut self: Pin<&mut Self>) {
}
}
+// SAFETY: `MapleTree<T>` is `Send` if `T` is `Send` because `MapleTree` owns its elements.
+unsafe impl<T: ForeignOwnable + Send> Send for MapleTree<T> {}
+// SAFETY: `&MapleTree<T>` never hands out `&T`; all entry access is serialized
+// by `ma_lock` or `&mut Guard`, so `T: Send` suffices (`T: Sync` not required).
+unsafe impl<T: ForeignOwnable + Send> Sync for MapleTree<T> {}
+
/// A reference to a [`MapleTree`] that owns the inner lock.
///
/// # Invariants
///
/// This guard owns the inner spinlock.
#[must_use = "if unused, the lock will be immediately unlocked"]
-pub struct MapleGuard<'tree, T: ForeignOwnable>(&'tree MapleTree<T>);
+pub struct MapleGuard<'tree, T: ForeignOwnable> {
+ tree: &'tree MapleTree<T>,
+ // A held spinlock must be released on the same CPU that acquired it.
+ _not_send: NotThreadSafe,
+}
impl<'tree, T: ForeignOwnable> Drop for MapleGuard<'tree, T> {
#[inline]
fn drop(&mut self) {
// SAFETY: By the type invariants, we hold this spinlock.
- unsafe { bindings::spin_unlock(self.0.ma_lock()) };
+ unsafe { bindings::spin_unlock(self.tree.ma_lock()) };
}
}
@@ -323,7 +340,7 @@ impl<'tree, T: ForeignOwnable> MapleGuard<'tree, T> {
pub fn ma_state(&mut self, first: usize, end: usize) -> MaState<'_, T> {
// SAFETY: The `MaState` borrows this `MapleGuard`, so it can also borrow the `MapleGuard`s
// read/write permissions to the maple tree.
- unsafe { MaState::new_raw(self.0, first, end) }
+ unsafe { MaState::new_raw(self.tree, first, end) }
}
/// Load the value at the given index.
@@ -375,7 +392,7 @@ pub fn ma_state(&mut self, first: usize, end: usize) -> MaState<'_, T> {
#[inline]
pub fn load(&mut self, index: usize) -> Option<T::BorrowedMut<'_>> {
// SAFETY: `self.tree` contains a valid maple tree.
- let ret = unsafe { bindings::mtree_load(self.0.tree.get(), index) };
+ let ret = unsafe { bindings::mtree_load(self.tree.tree.get(), index) };
if ret.is_null() {
return None;
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Maxime Ripard @ 2026-05-06 13:31 UTC (permalink / raw)
To: Nicolas Frattaroli
Cc: Ketil Johnsen, David Airlie, Simona Vetter, Maarten Lankhorst,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Boris Brezillon, Steven Price, Liviu Dudau,
Daniel Almeida, Alice Ryhl, Matthias Brugger,
AngeloGioacchino Del Regno, dri-devel, linux-doc, linux-kernel,
linux-media, linaro-mm-sig, linux-arm-kernel, linux-mediatek,
Florent Tomasin
In-Reply-To: <SurytM7FTOazQNVXXqCU7g@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 3341 bytes --]
On Wed, May 06, 2026 at 02:43:42PM +0200, Nicolas Frattaroli wrote:
> On Wednesday, 6 May 2026 12:08:24 Central European Summer Time Maxime Ripard wrote:
> > Hi,
> >
> > On Tue, May 05, 2026 at 04:05:10PM +0200, Ketil Johnsen wrote:
> > > From: Florent Tomasin <florent.tomasin@arm.com>
> > >
> > > This patch allows Panthor to allocate buffer objects from a
> > > protected heap. The Panthor driver should be seen as a consumer
> > > of the heap and not an exporter.
> > >
> > > Protected memory buffers needed by the Panthor driver:
> > > - On CSF FW load, the Panthor driver must allocate a protected
> > > buffer object to hold data to use by the FW when in protected
> > > mode. This protected buffer object is owned by the device
> > > and does not belong to a process.
> > > - On CSG creation, the Panthor driver must allocate a protected
> > > suspend buffer object for the FW to store data when suspending
> > > the CSG while in protected mode. The kernel owns this allocation
> > > and does not allow user space mapping. The format of the data
> > > in this buffer is only known by the FW and does not need to be
> > > shared with other entities.
> > >
> > > The driver will retrieve the protected heap using the name of the
> > > heap provided to the driver as module parameter.
> >
> > I know it's what dma_heap_find asks for, but I wonder if it wouldn't be
> > better in the device tree and lookup through the device node? heaps are
> > going to have a node anyway, right?
> >
> > This would allow you to have a default that works and not mess to much
> > with the kernel parameters that aren't always easy to change for
> > end-users.
>
> Hopefully the kernel parameters aren't easy to change for end-users on
> systems that deploy this. :) The use-case is copy protection for embedded
> devices running on locked-down systems. Though admittedly the mechanism
> works even on "tampered"-with systems, as long as the underlying hardware
> implements the access restrictions properly.
I guess it wasn't just about the user tampering it, but also about
distros shipping, say, a signed UKI that would support multiple
platforms with 42 versions of optee but all using panthor. I'm not sure
we can expect a single heap name to work for all of them.
> I'm a bit hesitant about making this DT myself. It would solve the problem
> that panthor could probe before the heap provider and needs to handle
> deferral by itself, but it does mean that we'd be putting software
> configuration into the device tree.
Is it? If the system has a protected allocator, and if panthor
absolutely needs to allocate from that allocator, it's not software
configuration: it's a description of what the platform looks like from
Linux PoV.
Or put it differently, it's not more software than optee is, and yet it
has its own node.
> Having the secure heap be a node with no address would allow the tee
> (or whatever else) to still dynamically allocate it wherever, and let
> us handle the dependency relationship between dma heap and GPU, but
> then we require that tee heap driver implementations play nice with
> this scheme, and bring OF into the dma_heap APIs.
I mean, it's a dma_heap API that we create so we don't bring anything
more to it :)
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Maxime Ripard @ 2026-05-06 13:12 UTC (permalink / raw)
To: Boris Brezillon
Cc: Ketil Johnsen, David Airlie, Simona Vetter, Maarten Lankhorst,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin
In-Reply-To: <20260506125015.0108ef44@fedora>
[-- Attachment #1: Type: text/plain, Size: 3374 bytes --]
On Wed, May 06, 2026 at 12:50:15PM +0200, Boris Brezillon wrote:
> On Wed, 6 May 2026 12:08:24 +0200
> Maxime Ripard <mripard@kernel.org> wrote:
>
> > Hi,
> >
> > On Tue, May 05, 2026 at 04:05:10PM +0200, Ketil Johnsen wrote:
> > > From: Florent Tomasin <florent.tomasin@arm.com>
> > >
> > > This patch allows Panthor to allocate buffer objects from a
> > > protected heap. The Panthor driver should be seen as a consumer
> > > of the heap and not an exporter.
> > >
> > > Protected memory buffers needed by the Panthor driver:
> > > - On CSF FW load, the Panthor driver must allocate a protected
> > > buffer object to hold data to use by the FW when in protected
> > > mode. This protected buffer object is owned by the device
> > > and does not belong to a process.
> > > - On CSG creation, the Panthor driver must allocate a protected
> > > suspend buffer object for the FW to store data when suspending
> > > the CSG while in protected mode. The kernel owns this allocation
> > > and does not allow user space mapping. The format of the data
> > > in this buffer is only known by the FW and does not need to be
> > > shared with other entities.
> > >
> > > The driver will retrieve the protected heap using the name of the
> > > heap provided to the driver as module parameter.
> >
> > I know it's what dma_heap_find asks for, but I wonder if it wouldn't be
> > better in the device tree and lookup through the device node? heaps are
> > going to have a node anyway, right?
>
> I'm not too sure. Take the PROTMEM (name="protected,xxxx") dma_heaps
> instantiated by optee for instance, I don't think the originating
> tee_device comes from a device node, nor is the underlying heap
> described as a device node. The reserved memory pool this protected heap
> comes from is most likely defined somewhere as reserved memory in the
> DT, but there's nothing to correlate this range of reserved mem to some
> sub-range that the TEE implementation is carving out to provide
> protected memory.
Maybe we should be working on a dt bindings for heaps then? Something
simple like we have for clocks with a phandle and an ID would probably
be enough. In optee's case, it looks like it would map nicely with
TEE_DMA_HEAP_* flags too.
The only two that wouldn't be covered would be the system and default
CMA heap if not setup in the DT, which shouldn't be too bad for this
particular use-case.
> > This would allow you to have a default that works and not mess to much
> > with the kernel parameters that aren't always easy to change for
> > end-users.
>
> I guess we can have a default list of heaps that we know provide
> protected memory for GPU rendering if that helps. Right now this list
> would contain only "protected,trusted-ui" :D. The other option would be
> to make this list a panthor Kconfig option and not expose it as a module
> param.
My main concern is that firmware builds are board specific, and thus its
capabilities isn't something we can reasonably expect to be consistent
across boards, SoCs and platforms. Kernel images (and the kernel
parameters) however can be made generic and unreasonably hard for users
to modify once you start playing with things like secure boot or
measured boot.
The only thing bridging the gap between the two is the DT.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Nicolas Frattaroli @ 2026-05-06 12:43 UTC (permalink / raw)
To: Ketil Johnsen, Maxime Ripard
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
Jonathan Corbet, Shuah Khan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Boris Brezillon, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin
In-Reply-To: <20260506-energetic-azure-pig-2b6ec4@houat>
On Wednesday, 6 May 2026 12:08:24 Central European Summer Time Maxime Ripard wrote:
> Hi,
>
> On Tue, May 05, 2026 at 04:05:10PM +0200, Ketil Johnsen wrote:
> > From: Florent Tomasin <florent.tomasin@arm.com>
> >
> > This patch allows Panthor to allocate buffer objects from a
> > protected heap. The Panthor driver should be seen as a consumer
> > of the heap and not an exporter.
> >
> > Protected memory buffers needed by the Panthor driver:
> > - On CSF FW load, the Panthor driver must allocate a protected
> > buffer object to hold data to use by the FW when in protected
> > mode. This protected buffer object is owned by the device
> > and does not belong to a process.
> > - On CSG creation, the Panthor driver must allocate a protected
> > suspend buffer object for the FW to store data when suspending
> > the CSG while in protected mode. The kernel owns this allocation
> > and does not allow user space mapping. The format of the data
> > in this buffer is only known by the FW and does not need to be
> > shared with other entities.
> >
> > The driver will retrieve the protected heap using the name of the
> > heap provided to the driver as module parameter.
>
> I know it's what dma_heap_find asks for, but I wonder if it wouldn't be
> better in the device tree and lookup through the device node? heaps are
> going to have a node anyway, right?
>
> This would allow you to have a default that works and not mess to much
> with the kernel parameters that aren't always easy to change for
> end-users.
Hopefully the kernel parameters aren't easy to change for end-users on
systems that deploy this. :) The use-case is copy protection for embedded
devices running on locked-down systems. Though admittedly the mechanism
works even on "tampered"-with systems, as long as the underlying hardware
implements the access restrictions properly.
I'm a bit hesitant about making this DT myself. It would solve the problem
that panthor could probe before the heap provider and needs to handle
deferral by itself, but it does mean that we'd be putting software
configuration into the device tree. Having the secure heap be a node with
no address would allow the tee (or whatever else) to still dynamically
allocate it wherever, and let us handle the dependency relationship
between dma heap and GPU, but then we require that tee heap driver
implementations play nice with this scheme, and bring OF into the
dma_heap APIs.
I'm not against making the dma heap a phandle property for the GPU
node and then extending the dma-heap API to get a heap by name or
by index from a user device's standardised phandle property/names
property, but that's potentially a very large can of worms to open.
>
> Maxime
>
Kind regards,
Nicolas Frattaroli
^ permalink raw reply
* [RFC net-next 4/4] net/mlx5: Apply devlink boot defaults during init
From: Mark Bloch @ 2026-05-06 12:37 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jonathan Corbet, Shuah Khan, Jiri Pirko, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch,
Andrew Morton, Borislav Petkov (AMD), Randy Dunlap, Dave Hansen,
Christian Brauner, Petr Mladek, Peter Zijlstra (Intel),
Thomas Gleixner, Pawan Gupta, Dapeng Mi, Kees Cook, Marco Elver,
Eric Biggers, Li RongQing, Paul E. McKenney, linux-doc,
linux-kernel, netdev, linux-rdma
In-Reply-To: <20260506123739.1959770-1-mbloch@nvidia.com>
Apply devlink boot defaults for mlx5 devices after successful device
initialization, while holding the devlink instance lock.
At this point the devlink instance is registered and the mlx5 devlink
operations and parameters have been registered, so generic devlink
defaults such as eswitch mode and runtime parameters can be applied to
the matching PCI devlink handle.
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index b1b9ebfd3866..a119d199f9a5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1464,6 +1464,8 @@ int mlx5_init_one(struct mlx5_core_dev *dev)
err = mlx5_init_one_devl_locked(dev);
if (err)
devl_unregister(devlink);
+ else
+ devl_apply_defaults(devlink);
unlock:
devl_unlock(devlink);
return err;
--
2.34.1
^ permalink raw reply related
* [RFC net-next 3/4] devlink: Add runtime parameter boot defaults
From: Mark Bloch @ 2026-05-06 12:37 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jonathan Corbet, Shuah Khan, Jiri Pirko, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch,
Andrew Morton, Borislav Petkov (AMD), Randy Dunlap, Dave Hansen,
Christian Brauner, Petr Mladek, Peter Zijlstra (Intel),
Thomas Gleixner, Pawan Gupta, Dapeng Mi, Kees Cook, Marco Elver,
Eric Biggers, Li RongQing, Paul E. McKenney, linux-doc,
linux-kernel, netdev, linux-rdma
In-Reply-To: <20260506123739.1959770-1-mbloch@nvidia.com>
Add support for setting devlink device parameters from the devlink=
kernel command line parameter.
The supported syntax is:
devlink=[<handle>]:param:<name>:<value>
Parameter values are parsed according to the registered devlink
parameter type and are applied in runtime configuration mode. Driverinit
and permanent configuration modes are intentionally not part of the
boot-default syntax.
Add a helper that finds a parameter by name, verifies runtime mode
support, converts the string value, runs the parameter validator and
invokes the existing set callback.
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
.../admin-guide/kernel-parameters.txt | 2 +
.../networking/devlink/devlink-defaults.rst | 18 ++-
net/devlink/core.c | 110 ++++++++++++------
net/devlink/devl_internal.h | 3 +
net/devlink/param.c | 70 +++++++++++
5 files changed, 165 insertions(+), 38 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 150202882870..761ae45b8607 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1293,9 +1293,11 @@ Kernel parameters
Currently supported settings:
esw:mode:{ legacy | switchdev | switchdev_inactive }
+ param:<name>:<value>
Examples:
devlink=[pci/0000:08:00.0]:esw:mode:switchdev
+ devlink=[pci/0000:08:00.0]:param:flow_steering_mode:smfs
devlink=[pci/0000:08:00.0,pci/0000:08:00.1]:esw:mode:legacy
devlink=[pci/0000:08:00.0]:esw:mode:switchdev,[pci/0000:08:00.1]:esw:mode:switchdev_inactive
diff --git a/Documentation/networking/devlink/devlink-defaults.rst b/Documentation/networking/devlink/devlink-defaults.rst
index 7d6ccaddca86..0d4036e59e88 100644
--- a/Documentation/networking/devlink/devlink-defaults.rst
+++ b/Documentation/networking/devlink/devlink-defaults.rst
@@ -55,13 +55,16 @@ The following syntax rules apply:
* Defaults for the same handle are applied in command-line order.
* The same ``esw`` attribute may be specified only once for a given devlink
handle.
+* The same ``param`` name may be specified only once for a given devlink
+ handle.
* Duplicate entries for the same handle are rejected and all devlink defaults
are ignored.
+* Parameter names and values must not contain ``:`` or ``,``.
Supported defaults
==================
-The supported command is ``esw``:
+The supported commands are ``esw`` and ``param``:
.. list-table::
:widths: 10 25 35
@@ -73,11 +76,16 @@ The supported command is ``esw``:
* - ``esw``
- ``mode:<mode>``
- ``legacy``, ``switchdev``, ``switchdev_inactive``
+ * - ``param``
+ - ``<name>:<value>``
+ - ``<value>`` is parsed according to the registered devlink parameter
+ type. Only runtime devlink parameters are supported.
The ``esw:mode`` default corresponds to the userspace command::
devlink dev eswitch set <handle> mode <value>
+The ``param`` default applies the named devlink parameter in runtime mode.
Examples
========
@@ -90,6 +98,10 @@ Set two PCI devlink instances to legacy mode::
devlink=[pci/0000:08:00.0,pci/0000:08:00.1]:esw:mode:legacy
+Set a runtime devlink device parameter::
+
+ devlink=[pci/0000:08:00.0]:param:flow_steering_mode:smfs
+
Set different modes for different PCI devlink instances::
devlink=[pci/0000:08:00.0]:esw:mode:switchdev,[pci/0000:08:00.1]:esw:mode:switchdev_inactive
@@ -97,3 +109,7 @@ Set different modes for different PCI devlink instances::
The following is invalid because the same handle receives ``esw:mode`` twice::
devlink=[pci/0000:08:00.0]:esw:mode:legacy,[pci/0000:08:00.0]:esw:mode:switchdev
+
+The following is invalid because the same handle receives ``param:x`` twice::
+
+ devlink=[pci/0]:param:x:1,[pci/0]:param:x:2
diff --git a/net/devlink/core.c b/net/devlink/core.c
index 4b404135181c..22990793ab8c 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -50,6 +50,13 @@ struct devlink_default_node {
struct list_head cmd_list;
};
+struct devlink_default_attr_spec {
+ const char *name;
+ enum devlink_attr attr;
+ int (*value_parse)(const char *value,
+ struct devlink_default_attr_item *attr_item);
+};
+
struct devlink_default_cmd_spec {
const char *name;
enum devlink_command cmd;
@@ -73,13 +80,6 @@ devlink_default_attr_free(struct devlink_default_attr_item *attr)
kfree(attr->value.param.value);
}
-struct devlink_default_attr_spec {
- const char *name;
- enum devlink_attr attr;
- int (*value_parse)(const char *value,
- struct devlink_default_attr_item *attr_item);
-};
-
static int __init
devlink_default_attr_parse(char *str,
const struct devlink_default_attr_spec *attrs,
@@ -153,6 +153,33 @@ devlink_default_esw_attr_parse(char *str,
attr_item);
}
+static int __init
+devlink_default_param_attr_parse(char *str,
+ struct devlink_default_attr_item *attr_item)
+{
+ char *name;
+ char *value;
+
+ attr_item->attr = DEVLINK_ATTR_PARAM;
+
+ name = strsep(&str, ":");
+ value = strsep(&str, ":");
+ if (!name || !*name || !value || !*value || str)
+ return -EINVAL;
+
+ attr_item->value.param.name = kstrdup(name, GFP_KERNEL);
+ if (!attr_item->value.param.name)
+ return -ENOMEM;
+ attr_item->value.param.value = kstrdup(value, GFP_KERNEL);
+ if (!attr_item->value.param.value) {
+ kfree(attr_item->value.param.name);
+ attr_item->value.param.name = NULL;
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int
devlink_default_eswitch_apply(struct devlink *devlink,
const struct devlink_default_attr_item *attr)
@@ -171,6 +198,17 @@ devlink_default_eswitch_apply(struct devlink *devlink,
}
}
+static int
+devlink_default_param_apply(struct devlink *devlink,
+ const struct devlink_default_attr_item *attr)
+{
+ if (attr->attr != DEVLINK_ATTR_PARAM)
+ return -EOPNOTSUPP;
+
+ return devlink_param_set_from_string(devlink, attr->value.param.name,
+ attr->value.param.value);
+}
+
static const struct devlink_default_cmd_spec devlink_default_cmds[] __initconst = {
{
.name = "esw",
@@ -178,52 +216,46 @@ static const struct devlink_default_cmd_spec devlink_default_cmds[] __initconst
.run = devlink_default_eswitch_apply,
.attr_parse = devlink_default_esw_attr_parse,
},
+ {
+ .name = "param",
+ .cmd = DEVLINK_CMD_PARAM_SET,
+ .run = devlink_default_param_apply,
+ .attr_parse = devlink_default_param_attr_parse,
+ },
};
-static const struct devlink_default_cmd_spec *__init
-devlink_default_cmd_spec_find(const char *name)
-{
- size_t i;
-
- for (i = 0; i < ARRAY_SIZE(devlink_default_cmds); i++) {
- if (!strcmp(name, devlink_default_cmds[i].name))
- return &devlink_default_cmds[i];
- }
-
- return NULL;
-}
-
static int __init
devlink_default_cmd_parse(char *str,
struct devlink_default_cmd_item *cmd_item)
{
- const struct devlink_default_cmd_spec *spec;
struct devlink_default_attr_item attr_item = {};
char *cmd_name;
int err;
+ size_t i;
cmd_name = strsep(&str, ":");
if (!cmd_name || !*cmd_name || !str || !*str)
return -EINVAL;
- spec = devlink_default_cmd_spec_find(cmd_name);
- if (!spec)
- return -EINVAL;
-
- err = spec->attr_parse(str, &attr_item);
- if (err) {
- devlink_default_attr_free(&attr_item);
- return err;
- }
- if (cmd_item) {
- cmd_item->cmd = spec->cmd;
- cmd_item->run = spec->run;
- cmd_item->attr = attr_item;
- } else {
- devlink_default_attr_free(&attr_item);
+ for (i = 0; i < ARRAY_SIZE(devlink_default_cmds); i++) {
+ if (!strcmp(cmd_name, devlink_default_cmds[i].name)) {
+ err = devlink_default_cmds[i].attr_parse(str, &attr_item);
+ if (err) {
+ devlink_default_attr_free(&attr_item);
+ return err;
+ }
+ if (cmd_item) {
+ cmd_item->cmd = devlink_default_cmds[i].cmd;
+ cmd_item->run = devlink_default_cmds[i].run;
+ cmd_item->attr = attr_item;
+ } else {
+ devlink_default_attr_free(&attr_item);
+ }
+ return 0;
+ }
}
- return 0;
+ return -EINVAL;
}
static int __init
@@ -369,6 +401,10 @@ devlink_default_cmd_equal(const struct devlink_default_cmd_item *a,
if (a->cmd != b->cmd || a->attr.attr != b->attr.attr)
return false;
+ if (a->cmd == DEVLINK_CMD_PARAM_SET)
+ return !strcmp(a->attr.value.param.name,
+ b->attr.value.param.name);
+
return true;
}
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index e4e48ee2da5a..bde333c22f18 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -212,6 +212,9 @@ static inline int devlink_nl_put_u64(struct sk_buff *msg, int attrtype, u64 val)
int devlink_nl_put_nested_handle(struct sk_buff *msg, struct net *net,
struct devlink *devlink, int attrtype);
int devlink_nl_msg_reply_and_new(struct sk_buff **msg, struct genl_info *info);
+int devlink_param_set_from_string(struct devlink *devlink,
+ const char *param_name,
+ const char *value_str);
static inline bool devlink_nl_notify_need(struct devlink *devlink)
{
diff --git a/net/devlink/param.c b/net/devlink/param.c
index cf95268da5b0..d2604fe2eee5 100644
--- a/net/devlink/param.c
+++ b/net/devlink/param.c
@@ -4,6 +4,8 @@
* Copyright (c) 2016 Jiri Pirko <jiri@mellanox.com>
*/
+#include <linux/kstrtox.h>
+
#include "devl_internal.h"
static const struct devlink_param devlink_param_generic[] = {
@@ -551,6 +553,74 @@ devlink_param_value_get_from_info(const struct devlink_param *param,
return 0;
}
+static int
+devlink_param_value_get_from_str(const struct devlink_param *param,
+ const char *value_str,
+ union devlink_param_value *value)
+{
+ switch (param->type) {
+ case DEVLINK_PARAM_TYPE_U8:
+ return kstrtou8(value_str, 0, &value->vu8);
+ case DEVLINK_PARAM_TYPE_U16:
+ return kstrtou16(value_str, 0, &value->vu16);
+ case DEVLINK_PARAM_TYPE_U32:
+ return kstrtou32(value_str, 0, &value->vu32);
+ case DEVLINK_PARAM_TYPE_U64:
+ return kstrtou64(value_str, 0, &value->vu64);
+ case DEVLINK_PARAM_TYPE_STRING:
+ if (strscpy(value->vstr, value_str, sizeof(value->vstr)) < 0)
+ return -EINVAL;
+ return 0;
+ case DEVLINK_PARAM_TYPE_BOOL:
+ return kstrtobool(value_str, &value->vbool);
+ }
+
+ return -EINVAL;
+}
+
+int devlink_param_set_from_string(struct devlink *devlink,
+ const char *param_name,
+ const char *value_str)
+{
+ struct devlink_param_gset_ctx ctx;
+ struct devlink_param_item *param_item;
+ const struct devlink_param *param;
+ union devlink_param_value value;
+ int err;
+
+ devl_assert_locked(devlink);
+
+ param_item = devlink_param_find_by_name(&devlink->params, param_name);
+ if (!param_item)
+ return -EINVAL;
+ param = param_item->param;
+
+ if (!devlink_param_cmode_is_supported(param,
+ DEVLINK_PARAM_CMODE_RUNTIME))
+ return -EOPNOTSUPP;
+ if (!param->set)
+ return -EOPNOTSUPP;
+
+ err = devlink_param_value_get_from_str(param, value_str, &value);
+ if (err)
+ return err;
+
+ if (param->validate) {
+ err = param->validate(devlink, param->id, value, NULL);
+ if (err)
+ return err;
+ }
+
+ ctx.val = value;
+ ctx.cmode = DEVLINK_PARAM_CMODE_RUNTIME;
+ err = devlink_param_set(devlink, param, &ctx, NULL);
+ if (err)
+ return err;
+
+ devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_NEW);
+ return 0;
+}
+
static struct devlink_param_item *
devlink_param_get_from_info(struct xarray *params, struct genl_info *info)
{
--
2.34.1
^ permalink raw reply related
* [RFC net-next 2/4] devlink: Add eswitch mode boot default
From: Mark Bloch @ 2026-05-06 12:37 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jonathan Corbet, Shuah Khan, Jiri Pirko, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch,
Andrew Morton, Borislav Petkov (AMD), Randy Dunlap, Dave Hansen,
Christian Brauner, Petr Mladek, Peter Zijlstra (Intel),
Thomas Gleixner, Pawan Gupta, Dapeng Mi, Kees Cook, Marco Elver,
Eric Biggers, Li RongQing, Paul E. McKenney, linux-doc,
linux-kernel, netdev, linux-rdma
In-Reply-To: <20260506123739.1959770-1-mbloch@nvidia.com>
Add support for configuring the devlink eswitch mode from the
devlink= kernel command line parameter.
The supported syntax is:
devlink=[<handle>]:esw:mode:<mode>
where <mode> is one of legacy, switchdev or switchdev_inactive. The
default is applied through the existing eswitch_mode_set() devlink
operation, matching the userspace devlink eswitch set command.
Document the devlink= syntax and the eswitch mode default.
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
.../admin-guide/kernel-parameters.txt | 24 ++++
.../networking/devlink/devlink-defaults.rst | 99 +++++++++++++++
Documentation/networking/devlink/index.rst | 1 +
net/devlink/core.c | 114 ++++++++++++++++++
4 files changed, 238 insertions(+)
create mode 100644 Documentation/networking/devlink/devlink-defaults.rst
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 7834ee927310..150202882870 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1278,6 +1278,30 @@ Kernel parameters
dell_smm_hwmon.fan_max=
[HW] Maximum configurable fan speed.
+ devlink= [NET]
+ Format:
+ <entry>[,<entry>...]
+
+ <entry>:
+ [<handle>[,<handle>...]]:<cmd>:<cmd-options>
+
+ <handle>:
+ <bus-name>/<dev-name>
+
+ Configure default devlink settings for matching
+ devlink instances during device initialization.
+
+ Currently supported settings:
+ esw:mode:{ legacy | switchdev | switchdev_inactive }
+
+ Examples:
+ devlink=[pci/0000:08:00.0]:esw:mode:switchdev
+ devlink=[pci/0000:08:00.0,pci/0000:08:00.1]:esw:mode:legacy
+ devlink=[pci/0000:08:00.0]:esw:mode:switchdev,[pci/0000:08:00.1]:esw:mode:switchdev_inactive
+
+ See Documentation/networking/devlink/devlink-defaults.rst
+ for the full syntax and duplicate handling rules.
+
dfltcc= [HW,S390]
Format: { on | off | def_only | inf_only | always }
on: s390 zlib hardware support for compression on
diff --git a/Documentation/networking/devlink/devlink-defaults.rst b/Documentation/networking/devlink/devlink-defaults.rst
new file mode 100644
index 000000000000..7d6ccaddca86
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-defaults.rst
@@ -0,0 +1,99 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+================
+Devlink Defaults
+================
+
+Devlink defaults allow selected devlink settings to be provided on the
+kernel command line and applied to matching devlink instances during device
+initialization.
+
+The devlink device is selected by its devlink handle. For PCI devices this is
+the same handle shown by ``devlink dev show``, for example
+``pci/0000:08:00.0``.
+
+Kernel command line syntax
+==========================
+
+Defaults are specified with the ``devlink=`` kernel command line parameter.
+
+The general syntax is::
+
+ devlink=<default>[,<default>...]
+
+Each default has the following form::
+
+ [<handle-list>]:<cmd>:<cmd-options>
+
+``<handle-list>`` is one or more devlink handles::
+
+ <bus-name>/<dev-name>[,<bus-name>/<dev-name>...]
+
+All handles in the same ``[]`` list receive the same command setting.
+
+Multiple defaults may be specified by separating complete defaults with a
+comma after the value::
+
+ devlink=[pci/0000:08:00.0]:esw:mode:switchdev,[pci/0000:08:00.1]:esw:mode:legacy
+
+Syntax rules
+------------
+
+The following syntax rules apply:
+
+* Specify all defaults in one ``devlink=`` parameter. Repeated ``devlink=``
+ parameters are not accumulated.
+* The ``devlink=`` value is limited by the kernel command line size.
+* Whitespace is not allowed within the parameter value.
+* ``<bus-name>`` and ``<dev-name>`` must not be empty.
+* ``<bus-name>`` must not contain ``:``.
+* ``<dev-name>`` may contain ``:``. This allows PCI names such as
+ ``0000:08:00.0``.
+* Handles must not contain whitespace, ``[``, ``]`` or more than one ``/``.
+* A comma inside ``[]`` separates handles.
+* A comma after the ``<value>`` separates defaults.
+* Defaults for the same handle are applied in command-line order.
+* The same ``esw`` attribute may be specified only once for a given devlink
+ handle.
+* Duplicate entries for the same handle are rejected and all devlink defaults
+ are ignored.
+
+Supported defaults
+==================
+
+The supported command is ``esw``:
+
+.. list-table::
+ :widths: 10 25 35
+ :header-rows: 1
+
+ * - Command
+ - Options
+ - Values
+ * - ``esw``
+ - ``mode:<mode>``
+ - ``legacy``, ``switchdev``, ``switchdev_inactive``
+
+The ``esw:mode`` default corresponds to the userspace command::
+
+ devlink dev eswitch set <handle> mode <value>
+
+
+Examples
+========
+
+Set one PCI devlink instance to switchdev mode::
+
+ devlink=[pci/0000:08:00.0]:esw:mode:switchdev
+
+Set two PCI devlink instances to legacy mode::
+
+ devlink=[pci/0000:08:00.0,pci/0000:08:00.1]:esw:mode:legacy
+
+Set different modes for different PCI devlink instances::
+
+ devlink=[pci/0000:08:00.0]:esw:mode:switchdev,[pci/0000:08:00.1]:esw:mode:switchdev_inactive
+
+The following is invalid because the same handle receives ``esw:mode`` twice::
+
+ devlink=[pci/0000:08:00.0]:esw:mode:legacy,[pci/0000:08:00.0]:esw:mode:switchdev
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index f7ba7dcf477d..0d27a7008b14 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -56,6 +56,7 @@ general.
:maxdepth: 1
devlink-dpipe
+ devlink-defaults
devlink-eswitch-attr
devlink-flash
devlink-health
diff --git a/net/devlink/core.c b/net/devlink/core.c
index 2421a1f8dbb7..4b404135181c 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -73,9 +73,123 @@ devlink_default_attr_free(struct devlink_default_attr_item *attr)
kfree(attr->value.param.value);
}
+struct devlink_default_attr_spec {
+ const char *name;
+ enum devlink_attr attr;
+ int (*value_parse)(const char *value,
+ struct devlink_default_attr_item *attr_item);
+};
+
+static int __init
+devlink_default_attr_parse(char *str,
+ const struct devlink_default_attr_spec *attrs,
+ size_t attrs_count,
+ struct devlink_default_attr_item *attr_item)
+{
+ char *attr_name;
+ char *value;
+ size_t i;
+
+ attr_name = strsep(&str, ":");
+ if (!attr_name || !*attr_name || !str || !*str)
+ return -EINVAL;
+
+ value = str;
+ for (i = 0; i < attrs_count; i++) {
+ if (!strcmp(attr_name, attrs[i].name)) {
+ attr_item->attr = attrs[i].attr;
+ return attrs[i].value_parse(value, attr_item);
+ }
+ }
+
+ return -EINVAL;
+}
+
+static int __init
+devlink_default_esw_mode_to_value(const char *str,
+ enum devlink_eswitch_mode *mode)
+{
+ if (!strcmp(str, "legacy")) {
+ *mode = DEVLINK_ESWITCH_MODE_LEGACY;
+ return 0;
+ }
+ if (!strcmp(str, "switchdev")) {
+ *mode = DEVLINK_ESWITCH_MODE_SWITCHDEV;
+ return 0;
+ }
+ if (!strcmp(str, "switchdev_inactive")) {
+ *mode = DEVLINK_ESWITCH_MODE_SWITCHDEV_INACTIVE;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int __init
+devlink_default_esw_mode_parse(const char *str,
+ struct devlink_default_attr_item *attr_item)
+{
+ enum devlink_eswitch_mode mode;
+ int err;
+
+ err = devlink_default_esw_mode_to_value(str, &mode);
+ if (err)
+ return err;
+
+ attr_item->value.eswitch_mode = mode;
+ return 0;
+}
+
+static const struct devlink_default_attr_spec devlink_default_esw_attrs[] __initconst = {
+ { "mode", DEVLINK_ATTR_ESWITCH_MODE, devlink_default_esw_mode_parse },
+};
+
+static int __init
+devlink_default_esw_attr_parse(char *str,
+ struct devlink_default_attr_item *attr_item)
+{
+ return devlink_default_attr_parse(str, devlink_default_esw_attrs,
+ ARRAY_SIZE(devlink_default_esw_attrs),
+ attr_item);
+}
+
+static int
+devlink_default_eswitch_apply(struct devlink *devlink,
+ const struct devlink_default_attr_item *attr)
+{
+ const struct devlink_ops *ops = devlink->ops;
+
+ switch (attr->attr) {
+ case DEVLINK_ATTR_ESWITCH_MODE:
+ if (!ops->eswitch_mode_set)
+ return -EOPNOTSUPP;
+
+ return ops->eswitch_mode_set(devlink, attr->value.eswitch_mode,
+ NULL);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static const struct devlink_default_cmd_spec devlink_default_cmds[] __initconst = {
+ {
+ .name = "esw",
+ .cmd = DEVLINK_CMD_ESWITCH_SET,
+ .run = devlink_default_eswitch_apply,
+ .attr_parse = devlink_default_esw_attr_parse,
+ },
+};
+
static const struct devlink_default_cmd_spec *__init
devlink_default_cmd_spec_find(const char *name)
{
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE(devlink_default_cmds); i++) {
+ if (!strcmp(name, devlink_default_cmds[i].name))
+ return &devlink_default_cmds[i];
+ }
+
return NULL;
}
--
2.34.1
^ permalink raw reply related
* [RFC net-next 1/4] devlink: Add infrastructure for boot-time defaults
From: Mark Bloch @ 2026-05-06 12:37 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jonathan Corbet, Shuah Khan, Jiri Pirko, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch,
Andrew Morton, Borislav Petkov (AMD), Randy Dunlap, Dave Hansen,
Christian Brauner, Petr Mladek, Peter Zijlstra (Intel),
Thomas Gleixner, Pawan Gupta, Dapeng Mi, Kees Cook, Marco Elver,
Eric Biggers, Li RongQing, Paul E. McKenney, linux-doc,
linux-kernel, netdev, linux-rdma
In-Reply-To: <20260506123739.1959770-1-mbloch@nvidia.com>
Add generic devlink boot-default infrastructure driven by the
devlink= kernel command line parameter.
The parser stores defaults per devlink handle using the same
bus/device handle format exposed by devlink. Each handle keeps an
ordered list of parsed commands so that defaults can later be applied
in command-line order when the matching devlink instance is initialized.
This commit only adds the generic parsing, storage, duplicate handling
and devl_apply_defaults() API. Concrete default commands are added in
later commits.
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
---
include/net/devlink.h | 1 +
net/devlink/core.c | 441 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 442 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index bcd31de1f890..058654d6800f 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1622,6 +1622,7 @@ int devl_trylock(struct devlink *devlink);
void devl_unlock(struct devlink *devlink);
void devl_assert_locked(struct devlink *devlink);
bool devl_lock_is_held(struct devlink *devlink);
+int devl_apply_defaults(struct devlink *devlink);
DEFINE_GUARD(devl, struct devlink *, devl_lock(_T), devl_unlock(_T));
struct ib_device;
diff --git a/net/devlink/core.c b/net/devlink/core.c
index eeb6a71f5f56..2421a1f8dbb7 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -4,6 +4,11 @@
* Copyright (c) 2016 Jiri Pirko <jiri@mellanox.com>
*/
+#include <linux/ctype.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/string.h>
#include <net/genetlink.h>
#define CREATE_TRACE_POINTS
#include <trace/events/devlink.h>
@@ -16,6 +21,418 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(devlink_trap_report);
DEFINE_XARRAY_FLAGS(devlinks, XA_FLAGS_ALLOC);
+static char *devlink_default;
+static LIST_HEAD(devlink_default_nodes);
+
+struct devlink_default_attr_item {
+ enum devlink_attr attr;
+ union {
+ enum devlink_eswitch_mode eswitch_mode;
+ struct {
+ char *name;
+ char *value;
+ } param;
+ } value;
+};
+
+struct devlink_default_cmd_item {
+ struct list_head list;
+ enum devlink_command cmd;
+ int (*run)(struct devlink *devlink,
+ const struct devlink_default_attr_item *attr);
+ struct devlink_default_attr_item attr;
+};
+
+struct devlink_default_node {
+ struct list_head list;
+ char *bus_name;
+ char *dev_name;
+ struct list_head cmd_list;
+};
+
+struct devlink_default_cmd_spec {
+ const char *name;
+ enum devlink_command cmd;
+ int (*run)(struct devlink *devlink,
+ const struct devlink_default_attr_item *attr);
+ int (*attr_parse)(char *str,
+ struct devlink_default_attr_item *attr_item);
+};
+
+static int __init
+devlink_default_node_add(const char *bus_name, const char *dev_name,
+ const char *cmd);
+
+static void __init
+devlink_default_attr_free(struct devlink_default_attr_item *attr)
+{
+ if (attr->attr != DEVLINK_ATTR_PARAM)
+ return;
+
+ kfree(attr->value.param.name);
+ kfree(attr->value.param.value);
+}
+
+static const struct devlink_default_cmd_spec *__init
+devlink_default_cmd_spec_find(const char *name)
+{
+ return NULL;
+}
+
+static int __init
+devlink_default_cmd_parse(char *str,
+ struct devlink_default_cmd_item *cmd_item)
+{
+ const struct devlink_default_cmd_spec *spec;
+ struct devlink_default_attr_item attr_item = {};
+ char *cmd_name;
+ int err;
+
+ cmd_name = strsep(&str, ":");
+ if (!cmd_name || !*cmd_name || !str || !*str)
+ return -EINVAL;
+
+ spec = devlink_default_cmd_spec_find(cmd_name);
+ if (!spec)
+ return -EINVAL;
+
+ err = spec->attr_parse(str, &attr_item);
+ if (err) {
+ devlink_default_attr_free(&attr_item);
+ return err;
+ }
+ if (cmd_item) {
+ cmd_item->cmd = spec->cmd;
+ cmd_item->run = spec->run;
+ cmd_item->attr = attr_item;
+ } else {
+ devlink_default_attr_free(&attr_item);
+ }
+
+ return 0;
+}
+
+static int __init
+devlink_default_cmd_parse_copy(const char *str,
+ struct devlink_default_cmd_item *cmd_item)
+{
+ char *cmd;
+ int err;
+
+ cmd = kstrdup(str, GFP_KERNEL);
+ if (!cmd)
+ return -ENOMEM;
+
+ err = devlink_default_cmd_parse(cmd, cmd_item);
+ kfree(cmd);
+ return err;
+}
+
+static int __init
+devlink_default_handle_parse(char *handle, char **bus_name, char **dev_name)
+{
+ char *slash;
+ char *p;
+
+ if (!handle || !*handle)
+ return -EINVAL;
+
+ for (p = handle; *p; p++) {
+ if (isspace(*p))
+ return -EINVAL;
+ if (*p == '[' || *p == ']')
+ return -EINVAL;
+ }
+
+ slash = strchr(handle, '/');
+ if (!slash || slash == handle || !slash[1])
+ return -EINVAL;
+ if (strchr(slash + 1, '/'))
+ return -EINVAL;
+
+ *slash = '\0';
+ if (strchr(handle, ':'))
+ return -EINVAL;
+
+ *bus_name = handle;
+ *dev_name = slash + 1;
+ return 0;
+}
+
+static int __init
+devlink_default_entry_parse(char *entry, bool store)
+{
+ char *handles_end;
+ char *handles;
+ char *handle;
+ char *cmd;
+ int err;
+
+ if (!entry || *entry != '[')
+ return -EINVAL;
+
+ handles = entry + 1;
+ handles_end = strchr(handles, ']');
+ if (!handles_end || handles_end[1] != ':' || !handles_end[2])
+ return -EINVAL;
+
+ *handles_end = '\0';
+ cmd = handles_end + 2;
+ if (!*handles)
+ return -EINVAL;
+
+ while ((handle = strsep(&handles, ",")) != NULL) {
+ char *bus_name;
+ char *dev_name;
+
+ err = devlink_default_handle_parse(handle, &bus_name,
+ &dev_name);
+ if (err)
+ return err;
+
+ if (!store)
+ continue;
+
+ err = devlink_default_node_add(bus_name, dev_name, cmd);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+static void __init
+devlink_default_cmd_item_free(struct devlink_default_cmd_item *cmd)
+{
+ devlink_default_attr_free(&cmd->attr);
+ kfree(cmd);
+}
+
+static void __init devlink_default_node_free(struct devlink_default_node *node)
+{
+ struct devlink_default_cmd_item *cmd;
+ struct devlink_default_cmd_item *cmd_tmp;
+
+ list_for_each_entry_safe(cmd, cmd_tmp, &node->cmd_list, list) {
+ list_del(&cmd->list);
+ devlink_default_cmd_item_free(cmd);
+ }
+
+ kfree(node->bus_name);
+ kfree(node->dev_name);
+ kfree(node);
+}
+
+static void __init devlink_default_nodes_clear(void)
+{
+ struct devlink_default_node *node;
+ struct devlink_default_node *node_tmp;
+
+ list_for_each_entry_safe(node, node_tmp, &devlink_default_nodes, list) {
+ list_del(&node->list);
+ devlink_default_node_free(node);
+ }
+}
+
+static struct devlink_default_node *__init
+devlink_default_node_find(const char *bus_name, const char *dev_name)
+{
+ struct devlink_default_node *node;
+
+ list_for_each_entry(node, &devlink_default_nodes, list) {
+ if (!strcmp(node->bus_name, bus_name) &&
+ !strcmp(node->dev_name, dev_name))
+ return node;
+ }
+
+ return NULL;
+}
+
+static bool __init
+devlink_default_cmd_equal(const struct devlink_default_cmd_item *a,
+ const struct devlink_default_cmd_item *b)
+{
+ if (a->cmd != b->cmd || a->attr.attr != b->attr.attr)
+ return false;
+
+ return true;
+}
+
+static bool __init
+devlink_default_cmd_exists(struct devlink_default_node *node,
+ const struct devlink_default_cmd_item *cmd)
+{
+ struct devlink_default_cmd_item *cmd_item;
+
+ list_for_each_entry(cmd_item, &node->cmd_list, list) {
+ if (devlink_default_cmd_equal(cmd_item, cmd))
+ return true;
+ }
+
+ return false;
+}
+
+static int __init
+devlink_default_cmd_item_add(struct devlink_default_node *node,
+ const char *cmd_str)
+{
+ struct devlink_default_cmd_item *cmd;
+ int err;
+
+ cmd = kzalloc_obj(*cmd);
+ if (!cmd)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&cmd->list);
+ err = devlink_default_cmd_parse_copy(cmd_str, cmd);
+ if (err) {
+ devlink_default_cmd_item_free(cmd);
+ return err;
+ }
+
+ if (devlink_default_cmd_exists(node, cmd)) {
+ devlink_default_cmd_item_free(cmd);
+ return -EEXIST;
+ }
+
+ list_add_tail(&cmd->list, &node->cmd_list);
+ return 0;
+}
+
+static int __init
+devlink_default_node_add(const char *bus_name, const char *dev_name,
+ const char *cmd_str)
+{
+ struct devlink_default_node *node;
+ int err;
+
+ node = devlink_default_node_find(bus_name, dev_name);
+ if (node)
+ return devlink_default_cmd_item_add(node, cmd_str);
+
+ node = kzalloc_obj(*node);
+ if (!node)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&node->list);
+ INIT_LIST_HEAD(&node->cmd_list);
+ node->bus_name = kstrdup(bus_name, GFP_KERNEL);
+ node->dev_name = kstrdup(dev_name, GFP_KERNEL);
+ if (!node->bus_name || !node->dev_name) {
+ err = -ENOMEM;
+ goto err_free_node;
+ }
+
+ err = devlink_default_cmd_item_add(node, cmd_str);
+ if (err)
+ goto err_free_node;
+
+ list_add_tail(&node->list, &devlink_default_nodes);
+ return 0;
+
+err_free_node:
+ devlink_default_node_free(node);
+ return err;
+}
+
+static int __init devlink_default_parse(char *str, bool store)
+{
+ char *entry = str;
+ int err;
+
+ if (!str || !*str)
+ return -EINVAL;
+
+ while (entry) {
+ char *handles_end;
+ char *cmd_start;
+ char *entry_end;
+
+ if (*entry != '[') {
+ err = -EINVAL;
+ goto err_clear;
+ }
+
+ handles_end = strchr(entry + 1, ']');
+ if (!handles_end || handles_end[1] != ':') {
+ err = -EINVAL;
+ goto err_clear;
+ }
+
+ cmd_start = handles_end + 2;
+ entry_end = strchr(cmd_start, ',');
+ if (entry_end)
+ *entry_end = '\0';
+
+ err = devlink_default_entry_parse(entry, store);
+ if (err)
+ goto err_clear;
+ if (!entry_end)
+ return 0;
+
+ entry = entry_end + 1;
+ if (!*entry) {
+ err = -EINVAL;
+ goto err_clear;
+ }
+ }
+
+ return 0;
+
+err_clear:
+ if (store)
+ devlink_default_nodes_clear();
+ return err;
+}
+
+static int devlink_default_node_apply(struct devlink *devlink,
+ const struct devlink_default_node *node)
+{
+ const struct devlink_default_cmd_item *cmd;
+ int err;
+
+ list_for_each_entry(cmd, &node->cmd_list, list) {
+ err = cmd->run(devlink, &cmd->attr);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+/**
+ * devl_apply_defaults - Apply defaults matching the devlink instance
+ * @devlink: devlink
+ *
+ * The caller must hold the devlink instance lock.
+ */
+int devl_apply_defaults(struct devlink *devlink)
+{
+ const char *bus_name = devlink_bus_name(devlink);
+ const char *dev_name = devlink_dev_name(devlink);
+ struct devlink_default_node *node;
+
+ devl_assert_locked(devlink);
+
+ list_for_each_entry(node, &devlink_default_nodes, list) {
+ if (strcmp(node->bus_name, bus_name) ||
+ strcmp(node->dev_name, dev_name))
+ continue;
+
+ return devlink_default_node_apply(devlink, node);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(devl_apply_defaults);
+
+static int __init devlink_default_setup(char *str)
+{
+ devlink_default = str;
+ return 1;
+}
+__setup("devlink=", devlink_default_setup);
+
static struct devlink *devlinks_xa_get(unsigned long index)
{
struct devlink *devlink;
@@ -578,6 +995,27 @@ static int __init devlink_init(void)
{
int err;
+ if (devlink_default) {
+ char *def;
+
+ def = kstrdup(devlink_default, GFP_KERNEL);
+ if (!def) {
+ err = -ENOMEM;
+ goto out;
+ }
+ err = devlink_default_parse(def, true);
+ kfree(def);
+ if (err == -EEXIST) {
+ devlink_default = NULL;
+ pr_warn("devlink: duplicate defaults ignored\n");
+ } else if (err == -EINVAL) {
+ devlink_default = NULL;
+ pr_warn("devlink: invalid command line parameter ignored\n");
+ } else if (err) {
+ goto out;
+ }
+ }
+
err = register_pernet_subsys(&devlink_pernet_ops);
if (err)
goto out;
@@ -593,7 +1031,10 @@ static int __init devlink_init(void)
out_unreg_pernet_subsys:
unregister_pernet_subsys(&devlink_pernet_ops);
out:
+ if (err)
+ devlink_default_nodes_clear();
WARN_ON(err);
+
return err;
}
--
2.34.1
^ permalink raw reply related
* [RFC net-next 0/4] devlink: Add boot-time defaults
From: Mark Bloch @ 2026-05-06 12:37 UTC (permalink / raw)
To: Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn,
David S. Miller
Cc: Jonathan Corbet, Shuah Khan, Jiri Pirko, Simon Horman,
Saeed Mahameed, Leon Romanovsky, Tariq Toukan, Mark Bloch,
Andrew Morton, Borislav Petkov (AMD), Randy Dunlap, Dave Hansen,
Christian Brauner, Petr Mladek, Peter Zijlstra (Intel),
Thomas Gleixner, Pawan Gupta, Dapeng Mi, Kees Cook, Marco Elver,
Eric Biggers, Li RongQing, Paul E. McKenney, linux-doc,
linux-kernel, netdev, linux-rdma
This series adds a devlink= kernel command line parameter for applying
selected devlink settings during device initialization.
Following a discussion with Jakub[1], I am sending this RFC to get the
conversation moving. I started from Jakub's example/request and extended
it to cover requirements from production systems and configurations that
customers use.
One important caveat is that the parsing logic in this RFC was written
with AI assistance. I am also not sure whether the resulting syntax and
parser are too complex for a kernel command line interface. This is part
of why I am sending it as an RFC: to understand what direction and level
of complexity would be acceptable to people.
The implementation is intended to support the following properties:
- A system may have multiple devlink devices that usually need the same
configuration. For a configuration such as eswitch mode switchdev, a
user should be able to specify multiple devices to which that
configuration applies.
- There may be ordering dependencies between options. For example, in
mlx5, flow_steering_mode should be set before moving to switchdev.
With this in mind, defaults are applied per device in the left-to-right
order in which they appear on the command line.
The intent is to let deployments set devlink defaults before normal
userspace orchestration runs, while still using devlink concepts and
driver callbacks rather than adding driver-specific module parameters.
A default is scoped to one or more devlink handles, for example:
devlink=[pci/0000:08:00.0]:esw:mode:switchdev
devlink=[pci/0000:08:00.0]:param:flow_steering_mode:smfs
devlink=[pci/0000:08:00.0,pci/0000:08:00.1]:param:flow_steering_mode:hmfs,[pci/0000:08:00.0,pci/0000:08:00.1]:esw:mode:switchdev
The infrastructure stores parsed defaults per devlink handle and
applies them in command-line order when a matching devlink instance is
ready. Duplicate defaults for the same handle are rejected so the
resulting state is deterministic.
The first supported command is eswitch mode configuration. The second
is generic runtime devlink parameter setting. Parameter values are
parsed according to the registered devlink parameter type and are
applied only in runtime configuration mode.
mlx5 wires this into device initialization after the devlink instance
is registered and after mlx5 devlink operations and parameters are
available, so both eswitch mode defaults and runtime parameter
defaults can be applied to matching devlink devices.
Patch 1 adds the generic devlink boot-default parser, storage,
duplicate handling and devl_apply_defaults() API.
Patch 2 adds eswitch mode defaults and documents the devlink= syntax.
Patch 3 adds runtime devlink parameter defaults, including string to
devlink parameter value conversion.
Patch 4 calls devl_apply_defaults() from mlx5 device initialization.
[1] https://lore.kernel.org/all/20260502184153.4fd8d06f@kernel.org/
Mark Bloch (4):
devlink: Add infrastructure for boot-time defaults
devlink: Add eswitch mode boot default
devlink: Add runtime parameter boot defaults
net/mlx5: Apply devlink boot defaults during init
.../admin-guide/kernel-parameters.txt | 26 +
.../networking/devlink/devlink-defaults.rst | 115 ++++
Documentation/networking/devlink/index.rst | 1 +
.../net/ethernet/mellanox/mlx5/core/main.c | 2 +
include/net/devlink.h | 1 +
net/devlink/core.c | 591 ++++++++++++++++++
net/devlink/devl_internal.h | 3 +
net/devlink/param.c | 70 +++
8 files changed, 809 insertions(+)
create mode 100644 Documentation/networking/devlink/devlink-defaults.rst
base-commit: 7e0cccae6b45b12eaf71fc3ab8eb133bb50b28ad
--
2.34.1
^ permalink raw reply
* Re: [PATCH 01/14] kbuild: Bump minimum version of LLVM for building the kernel to 17.0.1
From: Daniel Pereira @ 2026-05-06 12:33 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Bill Wendling, Justin Stitt, Nick Desaulniers, linux-kernel, llvm,
linux-kbuild, Jonathan Corbet, Shuah Khan, linux-doc
In-Reply-To: <20260506062128.GA322298@ax162>
On Wed, May 6, 2026 at 3:21 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
>> Thanks but I think I can just update the version number in this patch
>> when I send v2, as the update should happen atomically. If you patch it
>> separately, it might not be true depending on when my change is merged.
>
>> --
>> Cheers,
>> Nathan
Hi Nathan,
Thanks for your reply. I still needed to adjust the changes.rst file
for the Portuguese translation (pt_BR), as I found it was quite
outdated compared to the current English document.
Thanks,
Daniel Pereira
^ permalink raw reply
* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Nicolas Frattaroli @ 2026-05-06 12:28 UTC (permalink / raw)
To: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Boris Brezillon, Steven Price, Liviu Dudau,
Daniel Almeida, Alice Ryhl, Matthias Brugger,
AngeloGioacchino Del Regno, Ketil Johnsen
Cc: dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin, Ketil Johnsen
In-Reply-To: <20260505140516.1372388-5-ketil.johnsen@arm.com>
Thanks for sending this series! A few quick notes in-line.
On Tuesday, 5 May 2026 16:05:10 Central European Summer Time Ketil Johnsen wrote:
> From: Florent Tomasin <florent.tomasin@arm.com>
>
> This patch allows Panthor to allocate buffer objects from a
> protected heap. The Panthor driver should be seen as a consumer
> of the heap and not an exporter.
>
> Protected memory buffers needed by the Panthor driver:
> - On CSF FW load, the Panthor driver must allocate a protected
> buffer object to hold data to use by the FW when in protected
> mode. This protected buffer object is owned by the device
> and does not belong to a process.
> - On CSG creation, the Panthor driver must allocate a protected
> suspend buffer object for the FW to store data when suspending
> the CSG while in protected mode. The kernel owns this allocation
> and does not allow user space mapping. The format of the data
> in this buffer is only known by the FW and does not need to be
> shared with other entities.
>
> The driver will retrieve the protected heap using the name of the
> heap provided to the driver as module parameter.
>
> If the heap is not yet available, the panthor driver will defer
> the probe until created. It is an integration error to provide
> a heap name that does not exist or is never created.
>
> Panthor is calling the DMA heap allocation function
> and obtains a DMA buffer from it. This buffer is then
> registered to GEM and imported.
>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
> ---
> Documentation/gpu/panthor.rst | 47 +++++++++++++++
> drivers/gpu/drm/panthor/Kconfig | 1 +
> drivers/gpu/drm/panthor/panthor_device.c | 28 ++++++++-
> drivers/gpu/drm/panthor/panthor_device.h | 6 ++
> drivers/gpu/drm/panthor/panthor_fw.c | 29 ++++++++-
> drivers/gpu/drm/panthor/panthor_fw.h | 2 +
> drivers/gpu/drm/panthor/panthor_gem.c | 77 ++++++++++++++++++++++--
> drivers/gpu/drm/panthor/panthor_gem.h | 16 ++++-
> drivers/gpu/drm/panthor/panthor_heap.c | 2 +
> drivers/gpu/drm/panthor/panthor_sched.c | 11 +++-
> 10 files changed, 208 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/gpu/panthor.rst b/Documentation/gpu/panthor.rst
> index 7a841741278fb..be20eadea6dd5 100644
> --- a/Documentation/gpu/panthor.rst
> +++ b/Documentation/gpu/panthor.rst
> @@ -54,3 +54,50 @@ sync object arrays and heap chunks. Because they are all allocated and pinned
> at creation time, only `panthor-resident-memory` is necessary to tell us their
> size. `panthor-active-memory` shows the size of kernel BO's associated with
> VM's and groups currently being scheduled for execution by the GPU.
> +
> +Panthor Protected Memory Integration
> +=====================================
> +
> +Panthor requires the platform to provide a protected DMA HEAP.
> +This DMA heap must be identifiable via a string name.
> +The name is defined by the system integrator, it could be hard coded
> +in the heap driver, defined by a module parameter of the heap driver
> +or else.
> +
> +.. code-block:: none
> +
> + User
> + ┌─────────────────────────────┐
> + | Application |
> + └─────────────▲───────────────┘
> + | | |
> + | DMA-BUF | | Protected
> + | | | Job Submission
> + --------|---------|----------|---------
> + Kernel | | |
> + | | |
> + | | DMA-BUF |
> + ┌───────▼─────────────┐ ┌─▼───────┐
> + | DMA PROTECTED HEAP |◄───| Panthor |
> + | (Vendor specific) | | |
> + └─────────────────────┘ └─────────┘
> + | |
> + --------|--------------------|---------
> + HW | |
> + | |
> + ┌───────▼───────────────┐ ┌─▼───┐
> + | Trusted FW | | |
> + | Protected Memory ◄──► GPU |
> + └───────────────────────┘ └─────┘
> +
> +To configure Panthor to use the protected memory heap, pass the protected memory
> +heap string name as module parameter of the Panthor module.
> +
> +Example:
> +
> + .. code-block:: shell
> +
> + insmod panthor.ko protected_heap_name=“vendor_protected_heap"
> +
> +If `protected_heap_name` module parameter is not provided, Panthor will not support
> +protected job execution.
> diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
> index 911e7f4810c39..fb0bad9a0fd2b 100644
> --- a/drivers/gpu/drm/panthor/Kconfig
> +++ b/drivers/gpu/drm/panthor/Kconfig
> @@ -7,6 +7,7 @@ config DRM_PANTHOR
> depends on !GENERIC_ATOMIC64 # for IOMMU_IO_PGTABLE_LPAE
> depends on MMU
> select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + select DMABUF_HEAPS
> select DRM_EXEC
> select DRM_GPUVM
> select DRM_SCHED
> diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> index bc62a498a8a84..3a5cdfa99e5fe 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -5,7 +5,9 @@
> /* Copyright 2025 ARM Limited. All rights reserved. */
>
> #include <linux/clk.h>
> +#include <linux/dma-heap.h>
> #include <linux/mm.h>
> +#include <linux/of.h>
Can be dropped, none of the added code in this file requires it.
> #include <linux/platform_device.h>
> #include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> @@ -27,6 +29,10 @@
> #include "panthor_regs.h"
> #include "panthor_sched.h"
>
> +MODULE_PARM_DESC(protected_heap_name, "DMA heap name, from which to allocate protected buffers");
> +static char *protected_heap_name;
> +module_param(protected_heap_name, charp, 0444);
> +
> static int panthor_gpu_coherency_init(struct panthor_device *ptdev)
> {
> BUILD_BUG_ON(GPU_COHERENCY_NONE != DRM_PANTHOR_GPU_COHERENCY_NONE);
> @@ -127,6 +133,9 @@ void panthor_device_unplug(struct panthor_device *ptdev)
> panthor_gpu_unplug(ptdev);
> panthor_pwr_unplug(ptdev);
>
> + if (ptdev->protm.heap)
> + dma_heap_put(ptdev->protm.heap);
> +
> pm_runtime_dont_use_autosuspend(ptdev->base.dev);
> pm_runtime_put_sync_suspend(ptdev->base.dev);
>
> @@ -277,9 +286,21 @@ int panthor_device_init(struct panthor_device *ptdev)
> return ret;
> }
>
> + /* If a protected heap name is specified but not found, defer the probe until created */
> + if (protected_heap_name && strlen(protected_heap_name)) {
> + ptdev->protm.heap = dma_heap_find(protected_heap_name);
> + if (!ptdev->protm.heap) {
> + drm_warn(&ptdev->base,
> + "Protected heap \'%s\' not (yet) available - deferring probe",
> + protected_heap_name);
The escaping of the single quotes here is redundant, and I think this
is better as a debug message rather than a drm_warn: probe deferral
is normal.
Though I'm wondering whether we're open-coding dependency handling here,
I guess there's no way for any core to order things for us because the
dependency is on a name and the name is from a driver-specific module
parameter, so there's no generic solution to this. This second paragraph
is just me ruminating though and not an actionable request for changes.
> + ret = -EPROBE_DEFER;
> + goto err_rpm_put;
> + }
> + }
> +
> ret = panthor_hw_init(ptdev);
> if (ret)
> - goto err_rpm_put;
> + goto err_dma_heap_put;
>
> ret = panthor_pwr_init(ptdev);
> if (ret)
> @@ -343,6 +364,11 @@ int panthor_device_init(struct panthor_device *ptdev)
>
> err_rpm_put:
> pm_runtime_put_sync_suspend(ptdev->base.dev);
> +
> +err_dma_heap_put:
> + if (ptdev->protm.heap)
> + dma_heap_put(ptdev->protm.heap);
> +
This is ordered wrong. Getting the dma heap happens after getting rpm,
so the unwind should put the dma heap before putting rpm. Right now,
a failure of panthor_hw_init would leave rpm enabled.
As Boris already mentioned in his review though, using devres helpers
would get rid of this manual put on error or driver remove entirely,
which is preferable.
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h
> index 5cba272f9b4de..d51fec97fc5fa 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.h
> +++ b/drivers/gpu/drm/panthor/panthor_device.h
> @@ -7,6 +7,7 @@
> #define __PANTHOR_DEVICE_H__
>
> #include <linux/atomic.h>
> +#include <linux/dma-heap.h>
> #include <linux/io-pgtable.h>
> #include <linux/regulator/consumer.h>
> #include <linux/pm_runtime.h>
> @@ -329,6 +330,11 @@ struct panthor_device {
> struct list_head node;
> } gems;
> #endif
> + /** @protm: Protected mode related data. */
> + struct {
> + /** @heap: Pointer to the protected heap */
> + struct dma_heap *heap;
> + } protm;
> };
>
> struct panthor_gpu_usage {
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 0d07a133dc3af..1aba29b9779b6 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -500,6 +500,7 @@ panthor_fw_alloc_queue_iface_mem(struct panthor_device *ptdev,
>
> mem = panthor_kernel_bo_create(ptdev, ptdev->fw->vm, SZ_8K,
> DRM_PANTHOR_BO_NO_MMAP,
> + 0,
> DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC |
> DRM_PANTHOR_VM_BIND_OP_MAP_UNCACHED,
> PANTHOR_VM_KERNEL_AUTO_VA,
> @@ -534,6 +535,26 @@ panthor_fw_alloc_suspend_buf_mem(struct panthor_device *ptdev, size_t size)
> {
> return panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev), size,
> DRM_PANTHOR_BO_NO_MMAP,
> + 0,
> + DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
> + PANTHOR_VM_KERNEL_AUTO_VA,
> + "suspend_buf");
Looks like we're effectively renaming this from "FW suspend buffer"
to "suspend_buf", and calling the protm suspend buf "FW suspend buffer".
This seems a little confusing, to the point where the diff algorithm
also had a hard time. Naming comes down to a matter of taste, but I
want to make sure the rename was intentional here.
> +}
> +
> +/**
> + * panthor_fw_alloc_protm_suspend_buf_mem() - Allocate a protm suspend buffer
> + * for a command stream group.
> + * @ptdev: Device.
> + * @size: Size of the protm suspend buffer.
> + *
> + * Return: A valid pointer in case of success, an ERR_PTR() otherwise.
> + */
> +struct panthor_kernel_bo *
> +panthor_fw_alloc_protm_suspend_buf_mem(struct panthor_device *ptdev, size_t size)
> +{
> + return panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev), size,
> + DRM_PANTHOR_BO_NO_MMAP,
> + DRM_PANTHOR_KBO_PROTECTED_HEAP,
> DRM_PANTHOR_VM_BIND_OP_MAP_NOEXEC,
> PANTHOR_VM_KERNEL_AUTO_VA,
> "FW suspend buffer");
> @@ -547,6 +568,7 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> ssize_t vm_pgsz = panthor_vm_page_size(ptdev->fw->vm);
> struct panthor_fw_binary_section_entry_hdr hdr;
> struct panthor_fw_section *section;
> + u32 kbo_flags = 0;
> u32 section_size;
> u32 name_len;
> int ret;
> @@ -585,10 +607,13 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> return -EINVAL;
> }
>
> - if (hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) {
> + if ((hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) && !ptdev->protm.heap) {
> drm_warn(&ptdev->base,
> "Firmware protected mode entry is not supported, ignoring");
> return 0;
> + } else if ((hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) && ptdev->protm.heap) {
> + drm_info(&ptdev->base, "Firmware protected mode entry supported");
> + kbo_flags = DRM_PANTHOR_KBO_PROTECTED_HEAP;
Instead of the duplicated check in both branches of the condition,
nesting it may be less clunky:
if ((hdr.flags & CSF_FW_BINARY_IFACE_ENTRY_PROT) {
if (!ptdev->protm.heap) {
drm_warn(&ptdev->base,
"Firmware protected mode entry is not supported, ignoring");
return 0;
}
drm_info(&ptdev->base, "Firmware protected mode entry supported");
kbo_flags = DRM_PANTHOR_KBO_PROTECTED_HEAP;
}
That being said, we might want to rethink the warning/info entirely. If
it's normal behaviour for a platform to load this fw section, even if
the platform doesn't support protm, as I suspect is the case due to the
`return 0`, then the warning has always been a bit too noisy.
I think info level that also gives some identifier for what section was
ignored (e.g. fw offset start/end) would be fine. The other branch, i.e.
"Firmware protected mode entry supported", may be best something that
gets printed along with other details about what the GPU supports, so
that each protected section does not print this over and over. It feels
wrong to do it in panthor_hw_info_init since that's printing features of
the hardware rather than of panthor's configuration, so maybe just do it
in panthor_device_init after ptdev->protm.heap is non-NULL.
> }
>
> if (hdr.va.start == CSF_MCU_SHARED_REGION_START &&
> @@ -653,7 +678,7 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
>
> section->mem = panthor_kernel_bo_create(ptdev, panthor_fw_vm(ptdev),
> section_size,
> - DRM_PANTHOR_BO_NO_MMAP,
> + DRM_PANTHOR_BO_NO_MMAP, kbo_flags,
> vm_map_flags, va, "FW section");
> if (IS_ERR(section->mem))
> return PTR_ERR(section->mem);
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.h b/drivers/gpu/drm/panthor/panthor_fw.h
> index fbdc21469ba32..0cf3761abf789 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.h
> +++ b/drivers/gpu/drm/panthor/panthor_fw.h
> @@ -509,6 +509,8 @@ panthor_fw_alloc_queue_iface_mem(struct panthor_device *ptdev,
> u32 *input_fw_va, u32 *output_fw_va);
> struct panthor_kernel_bo *
> panthor_fw_alloc_suspend_buf_mem(struct panthor_device *ptdev, size_t size);
> +struct panthor_kernel_bo *
> +panthor_fw_alloc_protm_suspend_buf_mem(struct panthor_device *ptdev, size_t size);
>
> struct panthor_vm *panthor_fw_vm(struct panthor_device *ptdev);
>
> diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
> index 13295d7a593df..08fe4a5e43817 100644
> --- a/drivers/gpu/drm/panthor/panthor_gem.c
> +++ b/drivers/gpu/drm/panthor/panthor_gem.c
> @@ -20,12 +20,17 @@
> #include <drm/drm_print.h>
> #include <drm/panthor_drm.h>
>
> +#include <uapi/linux/dma-heap.h>
> +
> #include "panthor_device.h"
> #include "panthor_drv.h"
> #include "panthor_fw.h"
> #include "panthor_gem.h"
> #include "panthor_mmu.h"
>
> +MODULE_IMPORT_NS("DMA_BUF");
> +MODULE_IMPORT_NS("DMA_BUF_HEAP");
> +
> void panthor_gem_init(struct panthor_device *ptdev)
> {
> int err;
> @@ -466,7 +471,6 @@ static void panthor_gem_free_object(struct drm_gem_object *obj)
> }
>
> drm_gem_object_release(obj);
> -
Unrelated whitespace change (though I do like it), don't know how we
handle "too small for its own commit but also function isn't touched
by anything else in this commit" type whitespace cleanups.
> kfree(bo);
> drm_gem_object_put(vm_root_gem);
> }
> @@ -1026,6 +1030,7 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
> }
>
> panthor_gem_debugfs_set_usage_flags(bo, usage_flags);
> +
Unrelated whitespace change (though again, I do like it)
> return bo;
>
> err_put:
> @@ -1033,6 +1038,54 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
> return ERR_PTR(ret);
> }
>
> +static struct panthor_gem_object *
> +panthor_gem_create_protected(struct panthor_device *ptdev, size_t size,
> + uint32_t flags, struct panthor_vm *exclusive_vm,
> + u32 usage_flags)
> +{
> + struct dma_buf *dma_bo = NULL;
> + struct drm_gem_object *gem_obj;
> + struct panthor_gem_object *bo;
> + int ret;
> +
> + if (!ptdev->protm.heap)
> + return ERR_PTR(-EINVAL);
> +
> + if (flags != DRM_PANTHOR_BO_NO_MMAP)
> + return ERR_PTR(-EINVAL);
> +
> + if (!exclusive_vm)
> + return ERR_PTR(-EINVAL);
> +
> + dma_bo = dma_heap_buffer_alloc(ptdev->protm.heap, size, DMA_HEAP_VALID_FD_FLAGS,
> + DMA_HEAP_VALID_HEAP_FLAGS);
> + if (IS_ERR(dma_bo))
> + return ERR_PTR(PTR_ERR(dma_bo));
> +
> + gem_obj = drm_gem_prime_import(&ptdev->base, dma_bo);
I agree with Boris that putting the dma_buf here is the cleanest
solution.
Adding a cleanup.h DEFINE_FREE helper for dmabufs would be a longer-
term refactor with its own pros and cons, but would allow us to get
rid of the explicit put entirely by adorning the local with a __free
attribute.
> + if (IS_ERR(gem_obj)) {
> + ret = PTR_ERR(gem_obj);
> + goto err_free_dma_bo;
> + }
> +
> + bo = to_panthor_bo(gem_obj);
> + bo->flags = flags;
> +
> + panthor_gem_debugfs_set_usage_flags(bo, usage_flags);
> +
> + bo->exclusive_vm_root_gem = panthor_vm_root_gem(exclusive_vm);
> + drm_gem_object_get(bo->exclusive_vm_root_gem);
> + bo->base.resv = bo->exclusive_vm_root_gem->resv;
> +
> + return bo;
> +
> +err_free_dma_bo:
> + if (dma_bo)
> + dma_buf_put(dma_bo);
> +
> + return ERR_PTR(ret);
> +}
> +
> struct drm_gem_object *
> panthor_gem_prime_import_sg_table(struct drm_device *dev,
> struct dma_buf_attachment *attach,
Kind regards,
Nicolas Frattaroli
^ permalink raw reply
* [PATCH 2/2] cgroup/dmem: introduce a peak file
From: Thadeu Lima de Souza Cascardo @ 2026-05-06 11:58 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný, Michal Hocko,
Roman Gushchin, Shakeel Butt, Muchun Song, Andrew Morton,
Jonathan Corbet, Shuah Khan, Maarten Lankhorst, Maxime Ripard,
Natalie Vock, Tvrtko Ursulin
Cc: cgroups, linux-kernel, linux-mm, linux-doc, dri-devel,
Thadeu Lima de Souza Cascardo, kernel-dev
In-Reply-To: <20260506-dmem_peak-v1-0-8d803eb3449c@igalia.com>
Just like we have memory.peak, introduce a dmem.peak, which uses the
page_counter support for that.
It can be written to in order to reset the peak, but different from
memory.peak, which expects any write, dmem.peak expects the region name to
be written to it. That region peak is the one that is reset.
That requires ofp_peak to carry a pointer to the pool that was reset.
Writing a different region name will reset the different region and make
the original region peak get back to its non-reset value.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
Documentation/admin-guide/cgroup-v2.rst | 10 +++
include/linux/cgroup-defs.h | 1 +
kernel/cgroup/dmem.c | 132 ++++++++++++++++++++++++++++++--
3 files changed, 137 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 6efd0095ed99..3ba7ab3a36b3 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -2808,6 +2808,16 @@ DMEM Interface Files
The semantics are the same as for the memory cgroup controller, and are
calculated in the same way.
+ dmem.peak
+ A readwrite nested-keyed file that exists on non-root cgroups.
+
+ The max memory usage recorded for the cgroup and its descendants since
+ either the creation of the cgroup or the most recent reset for that FD.
+
+ A write of a region name to this file resets it to the current memory
+ usage for subsequent reads through the same file descriptor for that
+ region.
+
dmem.capacity
A read-only file that describes maximum region capacity.
It only exists on the root cgroup. Not all memory can be
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index a85044cb0553..b536054bd916 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -874,6 +874,7 @@ extern bool cgroup_enable_per_threadgroup_rwsem;
struct cgroup_of_peak {
unsigned long value;
struct list_head list;
+ struct dmem_cgroup_pool_state *pool;
};
void of_peak_reset(struct cgroup_of_peak *ofp, struct page_counter *pc,
diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
index 1ab1fb47f271..afa380c9839b 100644
--- a/kernel/cgroup/dmem.c
+++ b/kernel/cgroup/dmem.c
@@ -57,6 +57,9 @@ struct dmemcg_state {
struct cgroup_subsys_state css;
struct list_head pools;
+
+ /** @peaks_lock: Protects access to the pools' peaks lists */
+ spinlock_t peaks_lock;
};
struct dmem_cgroup_pool_state {
@@ -72,6 +75,10 @@ struct dmem_cgroup_pool_state {
struct rcu_head rcu;
struct page_counter cnt;
+
+ /* Protected by the dmemcg_state peaks_lock */
+ struct list_head peaks;
+
struct dmem_cgroup_pool_state *parent;
refcount_t ref;
@@ -162,26 +169,45 @@ set_resource_max(struct dmem_cgroup_pool_state *pool, u64 val)
page_counter_set_max(&pool->cnt, val);
}
-static u64 get_resource_low(struct dmem_cgroup_pool_state *pool)
+static u64 get_resource_low(struct seq_file *sf, struct dmem_cgroup_pool_state *pool)
{
return pool ? READ_ONCE(pool->cnt.low) : 0;
}
-static u64 get_resource_min(struct dmem_cgroup_pool_state *pool)
+static u64 get_resource_min(struct seq_file *sf, struct dmem_cgroup_pool_state *pool)
{
return pool ? READ_ONCE(pool->cnt.min) : 0;
}
-static u64 get_resource_max(struct dmem_cgroup_pool_state *pool)
+static u64 get_resource_max(struct seq_file *sf, struct dmem_cgroup_pool_state *pool)
{
return pool ? READ_ONCE(pool->cnt.max) : PAGE_COUNTER_MAX;
}
-static u64 get_resource_current(struct dmem_cgroup_pool_state *pool)
+static u64 get_resource_current(struct seq_file *sf, struct dmem_cgroup_pool_state *pool)
{
return pool ? page_counter_read(&pool->cnt) : 0;
}
+static u64 get_resource_peak(struct seq_file *sf, struct dmem_cgroup_pool_state *pool)
+{
+ struct cgroup_of_peak *ofp = of_peak(sf->private);
+ u64 fd_peak, peak;
+ struct dmem_cgroup_pool_state *of_pool;
+
+ if (!pool)
+ return 0;
+
+ of_pool = READ_ONCE(ofp->pool);
+
+ fd_peak = READ_ONCE(ofp->value);
+ if (of_pool != pool || fd_peak == OFP_PEAK_UNSET)
+ peak = pool->cnt.watermark;
+ else
+ peak = max(fd_peak, READ_ONCE(pool->cnt.local_watermark));
+ return peak;
+}
+
static void reset_all_resource_limits(struct dmem_cgroup_pool_state *rpool)
{
set_resource_min(rpool, 0);
@@ -227,6 +253,7 @@ dmemcs_alloc(struct cgroup_subsys_state *parent_css)
return ERR_PTR(-ENOMEM);
INIT_LIST_HEAD(&dmemcs->pools);
+ spin_lock_init(&dmemcs->peaks_lock);
return &dmemcs->css;
}
@@ -377,6 +404,7 @@ alloc_pool_single(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region
ppool ? &ppool->cnt : NULL, true);
reset_all_resource_limits(pool);
refcount_set(&pool->ref, 1);
+ INIT_LIST_HEAD(&pool->peaks);
kref_get(®ion->ref);
if (ppool && !pool->parent) {
pool->parent = ppool;
@@ -784,7 +812,7 @@ static ssize_t dmemcg_limit_write(struct kernfs_open_file *of,
}
static int dmemcg_limit_show(struct seq_file *sf, void *v,
- u64 (*fn)(struct dmem_cgroup_pool_state *))
+ u64 (*fn)(struct seq_file *, struct dmem_cgroup_pool_state *))
{
struct dmemcg_state *dmemcs = css_to_dmemcs(seq_css(sf));
struct dmem_cgroup_region *region;
@@ -796,7 +824,7 @@ static int dmemcg_limit_show(struct seq_file *sf, void *v,
seq_puts(sf, region->name);
- val = fn(pool);
+ val = fn(sf, pool);
if (val < PAGE_COUNTER_MAX)
seq_printf(sf, " %lld\n", val);
else
@@ -807,6 +835,90 @@ static int dmemcg_limit_show(struct seq_file *sf, void *v,
return 0;
}
+static int dmem_cgroup_region_peak_open(struct kernfs_open_file *of)
+{
+ struct cgroup_of_peak *ofp = of_peak(of);
+
+ ofp->value = OFP_PEAK_UNSET;
+
+ return 0;
+}
+
+static void dmem_cgroup_region_peak_remove(struct cgroup_of_peak *ofp)
+{
+ struct dmem_cgroup_pool_state *pool;
+ struct dmemcg_state *dmemcs;
+
+ pool = xchg(&ofp->pool, NULL);
+ if (!pool)
+ return;
+
+ dmemcs = pool->cs;
+
+ spin_lock(&dmemcs->peaks_lock);
+ list_del(&ofp->list);
+ spin_unlock(&dmemcs->peaks_lock);
+
+ WRITE_ONCE(ofp->value, OFP_PEAK_UNSET);
+
+ dmemcg_pool_put(pool);
+}
+
+static void dmem_cgroup_region_peak_release(struct kernfs_open_file *of)
+{
+ struct cgroup_of_peak *ofp = of_peak(of);
+
+ if (ofp->value == OFP_PEAK_UNSET) {
+ /* fast path (no writes on this fd) */
+ return;
+ }
+
+ dmem_cgroup_region_peak_remove(ofp);
+}
+
+static ssize_t dmem_cgroup_region_peak_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(of_css(of));
+ struct cgroup_of_peak *ofp = of_peak(of);
+ struct dmem_cgroup_pool_state *pool = NULL;
+ struct dmem_cgroup_region *region;
+ int err = 0;
+
+ buf = strstrip(buf);
+ if (!buf[0])
+ return -EINVAL;
+
+ rcu_read_lock();
+ region = dmemcg_get_region_by_name(buf);
+ rcu_read_unlock();
+
+ if (!region)
+ return -EINVAL;
+
+ pool = get_cg_pool_unlocked(dmemcs, region);
+ if (IS_ERR(pool)) {
+ err = PTR_ERR(pool);
+ goto out_put;
+ }
+
+ dmem_cgroup_region_peak_remove(ofp);
+
+ xchg(&ofp->pool, pool);
+ spin_lock(&dmemcs->peaks_lock);
+ of_peak_reset(ofp, &pool->cnt, &pool->peaks);
+ spin_unlock(&dmemcs->peaks_lock);
+
+out_put:
+ kref_put(®ion->ref, dmemcg_free_region);
+ return err ?: nbytes;
+}
+
+static int dmem_cgroup_region_peak_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_peak);
+}
+
static int dmem_cgroup_region_current_show(struct seq_file *sf, void *v)
{
return dmemcg_limit_show(sf, v, get_resource_current);
@@ -855,6 +967,14 @@ static struct cftype files[] = {
.name = "current",
.seq_show = dmem_cgroup_region_current_show,
},
+ {
+ .name = "peak",
+ .open = dmem_cgroup_region_peak_open,
+ .release = dmem_cgroup_region_peak_release,
+ .write = dmem_cgroup_region_peak_write,
+ .seq_show = dmem_cgroup_region_peak_show,
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
{
.name = "min",
.write = dmem_cgroup_region_min_write,
--
2.47.3
^ permalink raw reply related
* [PATCH 1/2] mm/page_counter: decouple peak_reset from peak_write
From: Thadeu Lima de Souza Cascardo @ 2026-05-06 11:58 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný, Michal Hocko,
Roman Gushchin, Shakeel Butt, Muchun Song, Andrew Morton,
Jonathan Corbet, Shuah Khan, Maarten Lankhorst, Maxime Ripard,
Natalie Vock, Tvrtko Ursulin
Cc: cgroups, linux-kernel, linux-mm, linux-doc, dri-devel,
Thadeu Lima de Souza Cascardo, kernel-dev
In-Reply-To: <20260506-dmem_peak-v1-0-8d803eb3449c@igalia.com>
Create a new function of_peak_reset that resets the page_counter peak for a
given writer. This should allow it to be reused by other cgroups.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
include/linux/cgroup-defs.h | 6 ++++++
kernel/cgroup/cgroup.c | 32 ++++++++++++++++++++++++++++++++
mm/memcontrol.c | 42 ++++++++----------------------------------
3 files changed, 46 insertions(+), 34 deletions(-)
diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index f42563739d2e..a85044cb0553 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -22,6 +22,7 @@
#include <linux/workqueue.h>
#include <linux/bpf-cgroup-defs.h>
#include <linux/psi_types.h>
+#include <linux/page_counter.h>
#ifdef CONFIG_CGROUPS
@@ -868,11 +869,16 @@ struct cgroup_subsys {
extern struct percpu_rw_semaphore cgroup_threadgroup_rwsem;
extern bool cgroup_enable_per_threadgroup_rwsem;
+#define OFP_PEAK_UNSET (((-1UL)))
+
struct cgroup_of_peak {
unsigned long value;
struct list_head list;
};
+void of_peak_reset(struct cgroup_of_peak *ofp, struct page_counter *pc,
+ struct list_head *watchers);
+
/**
* cgroup_threadgroup_change_begin - threadgroup exclusion for cgroups
* @tsk: target task
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 45c0b1ed687a..9b98a5cccf0e 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1981,6 +1981,38 @@ struct cgroup_of_peak *of_peak(struct kernfs_open_file *of)
return &ctx->peak;
}
+/**
+ * of_peak_reset - reset peak
+ * @ofp: open file context
+ * @pc: counter
+ * @watchers: list of other open file contexts
+ *
+ * This function updates all contexts in @watchers to the new usage of @pc.
+ * If @ofp is not in the list yet, that is, if its value is
+ * %OFP_PEAK_UNSET, it is added to @watchers list.
+ *
+ * A lock must be used to protect @watchers.
+ */
+void of_peak_reset(struct cgroup_of_peak *ofp, struct page_counter *pc,
+ struct list_head *watchers)
+{
+ unsigned long usage;
+ struct cgroup_of_peak *peer_ctx;
+
+ usage = page_counter_read(pc);
+ WRITE_ONCE(pc->local_watermark, usage);
+
+ list_for_each_entry(peer_ctx, watchers, list)
+ if (usage > peer_ctx->value)
+ WRITE_ONCE(peer_ctx->value, usage);
+
+ /* initial write, register watcher */
+ if (ofp->value == OFP_PEAK_UNSET)
+ list_add(&ofp->list, watchers);
+
+ WRITE_ONCE(ofp->value, usage);
+}
+
static void apply_cgroup_root_flags(unsigned int root_flags)
{
if (current->nsproxy->cgroup_ns == &init_cgroup_ns) {
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c03d4787d466..8754927070d3 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4517,8 +4517,6 @@ static u64 memory_current_read(struct cgroup_subsys_state *css,
return (u64)page_counter_read(&memcg->memory) * PAGE_SIZE;
}
-#define OFP_PEAK_UNSET (((-1UL)))
-
static int peak_show(struct seq_file *sf, void *v, struct page_counter *pc)
{
struct cgroup_of_peak *ofp = of_peak(sf->private);
@@ -4563,45 +4561,18 @@ static void peak_release(struct kernfs_open_file *of)
spin_unlock(&memcg->peaks_lock);
}
-static ssize_t peak_write(struct kernfs_open_file *of, char *buf, size_t nbytes,
- loff_t off, struct page_counter *pc,
- struct list_head *watchers)
+static ssize_t memory_peak_write(struct kernfs_open_file *of, char *buf,
+ size_t nbytes, loff_t off)
{
- unsigned long usage;
- struct cgroup_of_peak *peer_ctx;
struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
struct cgroup_of_peak *ofp = of_peak(of);
spin_lock(&memcg->peaks_lock);
-
- usage = page_counter_read(pc);
- WRITE_ONCE(pc->local_watermark, usage);
-
- list_for_each_entry(peer_ctx, watchers, list)
- if (usage > peer_ctx->value)
- WRITE_ONCE(peer_ctx->value, usage);
-
- /* initial write, register watcher */
- if (ofp->value == OFP_PEAK_UNSET)
- list_add(&ofp->list, watchers);
-
- WRITE_ONCE(ofp->value, usage);
+ of_peak_reset(ofp, &memcg->memory, &memcg->memory_peaks);
spin_unlock(&memcg->peaks_lock);
-
return nbytes;
}
-static ssize_t memory_peak_write(struct kernfs_open_file *of, char *buf,
- size_t nbytes, loff_t off)
-{
- struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
-
- return peak_write(of, buf, nbytes, off, &memcg->memory,
- &memcg->memory_peaks);
-}
-
-#undef OFP_PEAK_UNSET
-
static int memory_min_show(struct seq_file *m, void *v)
{
return seq_puts_memcg_tunable(m,
@@ -5611,9 +5582,12 @@ static ssize_t swap_peak_write(struct kernfs_open_file *of, char *buf,
size_t nbytes, loff_t off)
{
struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of));
+ struct cgroup_of_peak *ofp = of_peak(of);
- return peak_write(of, buf, nbytes, off, &memcg->swap,
- &memcg->swap_peaks);
+ spin_lock(&memcg->peaks_lock);
+ of_peak_reset(ofp, &memcg->swap, &memcg->swap_peaks);
+ spin_unlock(&memcg->peaks_lock);
+ return nbytes;
}
static int swap_high_show(struct seq_file *m, void *v)
--
2.47.3
^ permalink raw reply related
* [PATCH 0/2] cgroup/dmem: introduce a peak file
From: Thadeu Lima de Souza Cascardo @ 2026-05-06 11:58 UTC (permalink / raw)
To: Tejun Heo, Johannes Weiner, Michal Koutný, Michal Hocko,
Roman Gushchin, Shakeel Butt, Muchun Song, Andrew Morton,
Jonathan Corbet, Shuah Khan, Maarten Lankhorst, Maxime Ripard,
Natalie Vock, Tvrtko Ursulin
Cc: cgroups, linux-kernel, linux-mm, linux-doc, dri-devel,
Thadeu Lima de Souza Cascardo, kernel-dev
Just like we have memory.peak, introduce a dmem.peak, which uses the
page_counter support for that.
It can be written to in order to reset the peak, but different from
memory.peak, which expects any write, dmem.peak expects the region name to
be written to it. That region peak is the one that is reset.
That requires ofp_peak to carry a pointer to the pool that was reset.
Writing a different region name will reset the different region and make
the original region peak get back to its non-reset value.
While at it, we reuse a helper from memcontrol, which we moved to
kernel/cgroup/cgroup.c.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
Thadeu Lima de Souza Cascardo (2):
mm/page_counter: decouple peak_reset from peak_write
cgroup/dmem: introduce a peak file
Documentation/admin-guide/cgroup-v2.rst | 10 +++
include/linux/cgroup-defs.h | 7 ++
kernel/cgroup/cgroup.c | 32 ++++++++
kernel/cgroup/dmem.c | 132 ++++++++++++++++++++++++++++++--
mm/memcontrol.c | 42 ++--------
5 files changed, 183 insertions(+), 40 deletions(-)
---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260409-dmem_peak-3abc1be95072
Best regards,
--
Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
^ permalink raw reply
* [PATCH] docs: kernel-doc: python: strip __counted_by_ptr macro
From: Tudor Ambarus @ 2026-05-06 11:04 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Kees Cook, Gustavo A. R. Silva
Cc: linux-kernel, linux-doc, linux-hardening, peter.griffin,
andre.draszik, willmcvicker, jyescas, krzk, kernel-team,
Tudor Ambarus
The `__counted_by_ptr` macro was recently introduced [1] to extend
bounds checking semantics to standard dynamically allocated pointers.
However, the new Python implementation of kernel-doc does not currently
recognize it as a compiler attribute. When kernel-doc encounters a
struct member annotated with this macro, it fails to parse the variable
name correctly, resulting in false-positive warnings like:
Warning: ... struct member '__counted_by_ptr(cmdcnt' not described
Add `__counted_by_ptr` to the `struct_xforms` regex list so it gets
safely stripped out during the parsing phase, mirroring the existing
behavior for `__counted_by`. Update the corresponding unit tests.
Link: https://git.kernel.org/torvalds/c/150a04d817d8 [1]
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
tools/lib/python/kdoc/xforms_lists.py | 1 +
tools/unittests/test_cmatch.py | 1 +
2 files changed, 2 insertions(+)
diff --git a/tools/lib/python/kdoc/xforms_lists.py b/tools/lib/python/kdoc/xforms_lists.py
index f6ea9efb11ae..118156ea8cd2 100644
--- a/tools/lib/python/kdoc/xforms_lists.py
+++ b/tools/lib/python/kdoc/xforms_lists.py
@@ -29,6 +29,7 @@ class CTransforms:
(CMatch("__aligned"), ""),
(CMatch("__counted_by"), ""),
(CMatch("__counted_by_(le|be)"), ""),
+ (CMatch("__counted_by_ptr"), ""),
(CMatch("__guarded_by"), ""),
(CMatch("__pt_guarded_by"), ""),
(CMatch("__packed"), ""),
diff --git a/tools/unittests/test_cmatch.py b/tools/unittests/test_cmatch.py
index 7b996f83784d..109141cd2ab8 100755
--- a/tools/unittests/test_cmatch.py
+++ b/tools/unittests/test_cmatch.py
@@ -320,6 +320,7 @@ class TestSubWithLocalXforms(TestCaseDiff):
(CMatch('__aligned'), ' '),
(CMatch('__counted_by'), ' '),
(CMatch('__counted_by_(le|be)'), ' '),
+ (CMatch('__counted_by_ptr'), ' '),
(CMatch('__guarded_by'), ' '),
(CMatch('__pt_guarded_by'), ' '),
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260506-kdoc-__counted_by_ptr-1e206f3f1dc1
Best regards,
--
Tudor Ambarus <tudor.ambarus@linaro.org>
^ permalink raw reply related
* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Boris Brezillon @ 2026-05-06 10:50 UTC (permalink / raw)
To: Maxime Ripard
Cc: Ketil Johnsen, David Airlie, Simona Vetter, Maarten Lankhorst,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin
In-Reply-To: <20260506-energetic-azure-pig-2b6ec4@houat>
On Wed, 6 May 2026 12:08:24 +0200
Maxime Ripard <mripard@kernel.org> wrote:
> Hi,
>
> On Tue, May 05, 2026 at 04:05:10PM +0200, Ketil Johnsen wrote:
> > From: Florent Tomasin <florent.tomasin@arm.com>
> >
> > This patch allows Panthor to allocate buffer objects from a
> > protected heap. The Panthor driver should be seen as a consumer
> > of the heap and not an exporter.
> >
> > Protected memory buffers needed by the Panthor driver:
> > - On CSF FW load, the Panthor driver must allocate a protected
> > buffer object to hold data to use by the FW when in protected
> > mode. This protected buffer object is owned by the device
> > and does not belong to a process.
> > - On CSG creation, the Panthor driver must allocate a protected
> > suspend buffer object for the FW to store data when suspending
> > the CSG while in protected mode. The kernel owns this allocation
> > and does not allow user space mapping. The format of the data
> > in this buffer is only known by the FW and does not need to be
> > shared with other entities.
> >
> > The driver will retrieve the protected heap using the name of the
> > heap provided to the driver as module parameter.
>
> I know it's what dma_heap_find asks for, but I wonder if it wouldn't be
> better in the device tree and lookup through the device node? heaps are
> going to have a node anyway, right?
I'm not too sure. Take the PROTMEM (name="protected,xxxx") dma_heaps
instantiated by optee for instance, I don't think the originating
tee_device comes from a device node, nor is the underlying heap
described as a device node. The reserved memory pool this protected heap
comes from is most likely defined somewhere as reserved memory in the
DT, but there's nothing to correlate this range of reserved mem to some
sub-range that the TEE implementation is carving out to provide
protected memory.
>
> This would allow you to have a default that works and not mess to much
> with the kernel parameters that aren't always easy to change for
> end-users.
I guess we can have a default list of heaps that we know provide
protected memory for GPU rendering if that helps. Right now this list
would contain only "protected,trusted-ui" :D. The other option would be
to make this list a panthor Kconfig option and not expose it as a module
param.
^ permalink raw reply
* Re: [PATCH RFC v3 9/9] docs: iio: add documentation for ad9910 driver
From: Rodrigo Alencar @ 2026-05-06 10:41 UTC (permalink / raw)
To: Jonathan Cameron, Rodrigo Alencar
Cc: Rodrigo Alencar via B4 Relay, rodrigo.alencar, linux-iio,
devicetree, linux-kernel, linux-doc, linux-hardening,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Jonathan Corbet, Shuah Khan, Kees Cook,
Gustavo A. R. Silva
In-Reply-To: <20260505180827.2972124e@jic23-huawei>
On 26/05/05 06:08PM, Jonathan Cameron wrote:
> On Mon, 27 Apr 2026 15:35:21 +0100
> Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
>
> > On 26/04/27 10:46AM, Jonathan Cameron wrote:
> > > On Sun, 26 Apr 2026 21:42:15 +0100
> > > Rodrigo Alencar <455.rodrigo.alencar@gmail.com> wrote:
> > >
> > > > On 26/04/26 02:10PM, Jonathan Cameron wrote:
> > > > > On Fri, 17 Apr 2026 09:17:38 +0100
> > > > > Rodrigo Alencar via B4 Relay <devnull+rodrigo.alencar.analog.com@kernel.org> wrote:
> > > > >
> > > > > > From: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > > > >
> > > > > > Add documentation for the AD9910 DDS IIO driver, which describes channels,
> > > > > > DDS modes, attributes and ABI usage examples.
> > > > > >
> > > > > > Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
> > > > >
> > > > > Hi Rodrigo,
> > > > >
> > > > > I think this is getting close to something workable subject to some tweaks
> > > > > to not make the priority thing visible and use rate of change parameters
> > > > > so /Sec rather than steps.
> > > >
> > > > I am not sure about this one. Getting the value into units per seconds will
> > > > increase the range of values by a lot, e.g., for the frequency case the step
> > > > size can range from a few Hz up to the entire supported range (hundreds of
> > > > MHz), and if you consider that one would often have the sampling_frequency
> > > > at 250 MHz... an attribute frequency_roc could have an order of 10^17 Hz/s,
> > > > and I am not sure how practical is that, although it can have a physical meaning,
> > > > like a "chirp slope".
> > >
> > > That scaling is indeed a bit of a pain though it will go in a 64 bit int
> > > however, seems likely we'll get higher frequency devices one day that will
> > > limb even faster.
> > >
> > > Maybe wait and see if anyone else has input on this.
Maybe we add this _roc ABI for this device and figure out scaling issues on new devices
later.
> > > >
> > > > >
> > > > > Given this defines the ABI for a whole class of new devices that are
> > > > > rather complex, one concern is whether whatever we define here is general
> > > > > enough to be useful.
> > > > >
> > > > > Do you have any other DDS in your queue to upstream? Maybe it's worth
> > > > > sanity checking the ABI against them to see if it is fit for purpose?
> > > >
> > > > Not really, still the only DDS. Other DDS of the same family have a similar
> > > > Digital Ramp Generator with controls over ramp limits, rates and step.
> > >
> > > There are two in staging that have been there a very long time...
> > > ad9832 and ad9834. I haven't looked at how they correspond to this.
> >
> > It seems they can benefit of this child channel concept, as they define
> > multiple frequency and phase configuration for the same physical DAC.
> > I see the following in the most complicated one:
> > - out_altvoltage0_frequency0
> > - out_altvoltage0_frequency1
> > - out_altvoltage0_frequency_scale
> > - out_altvoltage0_phase0
> > - out_altvoltage0_phase1
> > - out__altvoltage0_phase_scale
> > - out_altvoltage0_pincontrol_en
> > - out_altvoltage0_frequencysymbol
> > - out_altvoltage0_phasesymbol
> > - out_altvoltage0_out_enable
> > - out_altvoltage0_out1_enable
> > - out_altvoltage0_out0_wavetype
> > - out_altvoltage0_out1_wavetype
> > - out_altvoltage0_out0_wavetype_available
> > - out_altvoltage0_out1_wavetype_available
> >
> > less complicated, no RAM, no DRG nor parallel port. In terms of common stuff I can see
> > frequency_scale and pinctrl_en.
>
> My main concern here is IIRC these are very much PSK / FSK devices with expectation
> of symbol being externally controlled. (maybe I remembered that wrong ;)
> So the symbol relationship would need to map to child channels in some fashion.
With multiple channels, I'd say that is already figured with the standard "enable" ABI,
As the "symbol" to select is a choice of one of the channels to enable. Like it is
being done here for the single tone profiles. Being externally controlled is a user
problem, being its responsability to make sure that expectations on the values are
aligned during some specific actions. For instance, this very driver, expects the selected profile
to be trully selected by the profile pins so that RAM loading works. Also, there is another
weird case, where the profile needs to be selected to make sure a read of the profile register
works (which is another reason to use cached reads). Apart from those cases, I suppose
profile pin control can be "detached" from the driver.
>
> >
> > > We should think hard about whether to bring them inline with this
> > > and out of staging, or just delete them.
> > >
> > > >
> > > > ...
> > > >
> > > > > > +DDS modes
> > > > > > +=========
> > > > > > +
> > > > > > +The AD9910 supports multiple modes of operation that can be configured
> > > > > > +independently or in combination. Such modes and their corresponding IIO channels
> > > > > > +are described in this section. The following tables are extracted from the
> > > > > > +AD9910 datasheet and summarizes the control parameters for each mode and their
> > > > > > +priority when multiple sources are enabled simultaneously:
> > > > >
> > > > > Maybe add a bit on what priority means. Does it mean that only the highest
> > > > > priority one is acted on? If so why do we need to expose that others are
> > > > > enabled? Just report only the highest priority one as enabled.
> > > > >
> > > > > I can see the hardware needs to do priority so it knows where to go when
> > > > > a given source is disabled but from a software point of view that
> > > > > can be controlled by us enabling that next item (and the driver does
> > > > > things in the right order to get the appropriate transition)
> > > > >
> > > > > That may mean that if all modes are disabled, we have to disable any output
> > > > > but seems doable.
> > > >
> > > > That is a bit complicated, as you can see, this part has modes that target
> > > > one DDS parameter or multiple (destinations: phase, frequency, amplitude).
> > > > Also, multiple modes can coexist, when they target different parameters/destination.
> > > > At the same time, RAM mode complicates everything because even though it targets
> > > > one specific parameter, once it is enabled influences the base mode for the
> > > > other parameters because single-tone is off. I have ordered the mode channels so
> > > > that higher index have higher priority, so that can be a bit clearer.
> > > >
> > > > Right now, all the controls are provided, what might be missing is a way
> > > > to query which level of those priorities is currently active, but those levels
> > > > are not the same thing as the controls. If we turn the priority levels into the
> > > > controls themselves it would be a different ABI and it would get a lot messy.
> > > > That is why I am dumping this priority table in this document! =(
> > >
> > > I understand (at least some) of the hardware complexity but I don't like
> > > the fact this is effectively exposing it to userspace. + I really don't want
> > > more ABI to indicate whether a mode is actively doing anything or not.
> > > Whilst I agree the code will be more complex, having clarity on what is
> > > enabled at any given time is definitely something we ant to aim for.
> >
> > This part seems special, so I would not bother with the fact that userspace
> > needs to figure some things out.
>
> I'm kind of fine for that being needed for complex usecases but if I want
> to do something simple I'd like that to be standard and intuitive.
> Obviously gets blurry wrt to what counts as simple.
>
> >
> > Since there is this priority, there is a difference on enable vs active,
> > and things are different for each DDS parameter. Take the single tone example
> > when it is enabled, should that mean that it is active on frequency, phase
> > or amplitude? that granularity could be exposed like that if we create dedicated
> > channels for each parameter on every mode possible, which turns the ABI into
> > a greater mess.
>
> I did wonder if we should separate those but agree it's tricky.
>
> >
> > > Can we work out a transition diagram? That might make it easier to
> > > tell whether it's possible to map it as single enables at a time and
> > > incorporate weird corners like the RAM one. Maybe not needed if the
> > > RAM one is the only real oddity and otherwise it's just going up
> > > and down the priority lists.
> >
> > I still think it is not that simple, an enable is just a control for
> > a specific mode-destination pair.
> >
> > > One complexity I can see with single enables is that they'd need
> > > to be separate for each of frequency, phase and amplitude to reflect
> > > the transitions that can occur.
> >
> > As mentioned above, that would be messy to interface with...
> >
> > > Also the fun of profiles, where those profile pins are basically picking
> > > symbols - could be used for multi level PSK or FSK for example if wired
> > > up to an external symbol source. I'd be a bit surprised if those are
> > > always wired up to a host CPU.
> >
> > Yes, some users may want to control the profile pins through an FPGA.
> >
> > > *sigh* I'm talking my self around to needing ABI to indicate a channel
> > > is active. The symbol stuff gets us some of the way there (and would
> > > work for the tones) but doesn't cover the added complexity of RAM etc.
> > > So 'maybe' new ABI for _isactive or something like that?
> >
> > active_params or active_targets? listing out DDS parameters that is currently
> > active for that mode/channel? like printing "frequency phase amplitude", so
> > that would mean the mode is driving all of them. That would be a read-only
> > status.
>
> Would have to be simple per channel sysfs attribute that would say
> is this particular channel actually contributing to the current output.
> Where it can contribute in multiple ways we'll need separate attributes.
>
> Anything else is going to break the rules on one thing per each sysfs
> attribute.
> out_altvoltageXXX_phase_active
> out_altvoltageXXX_freq_active
> or something like that.
Yes, that could be done. This would be just informative, and I suppose that
any userspace software that knows how to control the part would not even
look at those. Probably it would just be used while debugging.
> >
> > > Perhaps the boundary we put on this is the ABI should be such that
> > > simple choices such as enabling a single tone, or single RAM mode
> > > setting are intuitive.
> > >
> > > Why do we only have one ram channel? I'd kind of expect the firmware
> > > to fill all 8 RAM profiles because of that 'external' profile pins
> > > use case.
> >
> > Most of the RAM configuration is now comming from the firmware, so there
> > is no much info to display/configure in multiple channels. The per-profile
> > configs require the weird ABI like operating modes and address start/end.
> > By removing those, I have mostly "global" stuff.
> If folk are doing profile control from external pins I'd kind of expect to
> see some description of what they are controlling even if it's read only.
>
> Maybe we don't do this now but we should make sure there is space in the ABI
> by thinking what it might look like.
Right now, I am not seeing anything generic in RAM control appart from a
"sampling_frequency". A lot of it seems device-specific:
- Destination (freq/phase/amplitude/polar)
- Internal profile control config (sequenced modes I had before)
- Per-profile config:
- Start address
- End address
- Address step rate (sampling freq)
- RAM mode:
- direct switch
- ramp-up
- bidirectional
- bidirectional continuous
- ramp-up continuous
- No-dwell high bit (specific to ramp-up mode)
- Zero-crossing bit (specific to direct switch mode)
...
> > > > > > +Digital ramp generator (DRG)
> > > > > > +----------------------------
> > > > > > +
> > > > > > +The DRG produces linear frequency, phase or amplitude sweeps using dedicated
> > > > > > +hardware. It is controlled through three channels: a parent control channel
> > > > > > +(``digital_ramp_generator``) and two child ramp channels
> > > > > > +(``digital_ramp_up``, ``digital_ramp_down``). DRG destination is set when
> > > > > > +ramp attributes are written, i.e. writing to ``frequency`` or ``frequency_step``
> > > > > > +sets the destination to frequency.
> > > > > > +
> > > > > > +Control channel attributes
> > > > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > > > +
> > > > > > +.. flat-table::
> > > > > > + :header-rows: 1
> > > > > > +
> > > > > > + * - Attribute
> > > > > > + - Unit
> > > > > > + - Description
> > > > > > +
> > > > > > + * - ``en``
> > > > > > + - boolean
> > > > > > + - Enable/disable the DRG.
> > > > > > +
> > > > > > +Ramp channel attributes
> > > > > > +^^^^^^^^^^^^^^^^^^^^^^^^
> > > > > > +
> > > > > > +The ``digital_ramp_up`` and ``digital_ramp_down`` channels share the same
> > > > > > +attribute set but configure ascending and descending ramp parameters
> > > > > > +independently:
> > > > > > +
> > > > > > +.. flat-table::
> > > > > > + :header-rows: 1
> > > > > > +
> > > > > > + * - Attribute
> > > > > > + - Unit
> > > > > > + - Description
> > > > > > +
> > > > > > + * - ``en``
> > > > > > + - boolean
> > > > > > + - Enable/disable the ramp no-dwell behavior. Enabling both creates a
> > > > > > + bidirectional continuous ramp (Triangular pattern). Other configurations
> > > > > > + creates a single-shot ramp at the trasition of the DRCTL pin: ramp-up
> > > > >
> > > > > transition
> > > > >
> > > > > > + only, ramp-down only or bidirectional with dwell at the limits.
> > > > >
> > > > > Feels a little unintuitive to use the generic enable for this.
> > > > > We might need a specific control for this one.
> > > >
> > > > How about dwell_en, but it might not sound that generic. I used "enable" because:
> > > > - no-dwell high means a ramp-up pattern (only enabling the ramp-up channel)
> > > > - no-dwell low means a ramp-down pattern (only enabling the ramp-down channel)
> > > > - both no-dwell is a continuous ramp that goes up and down. (both enabled)
> > > > The last case is a bit off though, when both are disabled we get the normal mode, which
> > > > is also a ramps up and down, but dwelling in the limits.
> > > >
> > > > > > +
> > > > > > + * - ``frequency``
> > > > > > + - Hz
> > > > > > + - Frequency ramp limit. Range [0, SYSCLK/2).
> > > > > > +
> > > > > > + * - ``phase``
> > > > > > + - rad
> > > > > > + - Phase ramp limit. Range [0, 2*pi).
> > >
> > > Looking at this again, how do we set the DRG mode? E.g. if it effects
> > > only phase?
> >
> > You mean the destination? I removed the destination ABI. so now destination is
> > set when we write to either frequency, phase or scale properties.
> > * writing to frequency or frequency_step sets the destination to frequency
> > * writing to phase or phase_step sets the destination to phase
> > * writing to scale or scale_step sets the destination to amplitude
>
> So it's last write that sets it. We definitely need a way to know which
> one is active if we get multiple writes. What do the others return if they
> were set but something else has been set since?
All those values represent the same set of registers with different scaling for
frequency, phase or scale units. I can get the other values to return -EBUSY
when reading a destination attribute that was not set.
> >
> > The DRG mode (dwell mode) is now controlled with the enable bits in the ramp
> > up/down channels
> >
> > > > > > +
> > > > > > + * - ``scale``
> > > > > > + - fractional
> > > > > > + - Amplitude scale ramp limit. Range [0, 1).
> > > > > > +
> > > > > > + * - ``sampling_frequency``
> > > > > > + - Hz
> > > > > > + - Ramp clock rate: SYSCLK / (4 * divider).
> > > > > > +
> > > > > > + * - ``frequency_step``
> > > > > > + - Hz
> > > > > > + - Per-tick frequency increment/decrement. Range [0, SYSCLK/2).
> > > > >
> > > > > So this was the bit I referred to earlier. Normally we do
> > > > > rate of change measurements for this stuff rather than what happens on
> > > > > each tick (based on how we handle things like ROC events)
> > > > >
> > > > > So could we make these
> > > > > ``frequency_roc`` units HZ/Sec
> > > > > etc? Then from the mix configured would need to work out the optimum
> > > > > tick to deliver it.
> > > > >
> > > > > I suppose it's possible that someone might want a stepped frequency
> > > > > though which would break this approach? Does anyone actually do that?
> > > > > If so we'd need to keep the samping_frequency but then control _roc
> > > > > with that in mind.
> > > >
> > > > yeah... frequency steps would make sense when the user controls when to
> > > > perform the updates, or when it comes from certain events.
> > >
> > > You've lost me here. How can they do that? Some external clocking
> > > or event?
> >
> > That would depend on what the user does. This part has this DRHOLD pin
> > which can freeze the ramp. If the user sets this HIGH and creates pulses
> > it is able to control the stepping of the RAMP manually. But I assume
> > that no one would do that... such application is unknown to me.
>
> They'd have to sync that with the clock driving the ramp. I guess that's
> possible - but nasty. I vote we pretend this usecase doesn't exist for now ;)
>
> > > > > > +
> > > > > > +Output shift keying (OSK)
> > > > > This is a new one on me...
> > > > > > +-------------------------
> > > > > > +
> > > > > > +OSK controls the output amplitude envelope, allowing the output to be ramped
> > > > > > +on/off rather than switched abruptly.
> > > > >
> > > > > > +
> > > > > > +.. flat-table::
> > > > > > + :header-rows: 1
> > > > > > +
> > > > > > + * - Attribute
> > > > > > + - Unit
> > > > > > + - Description
> > > > > > +
> > > > > > + * - ``en``
> > > > > > + - boolean
> > > > > > + - Enable/disable OSK.
> > > > > > +
> > > > > > + * - ``scale``
> > > > > > + - fractional
> > > > > > + - Target amplitude for the OSK ramp. 14-bit ASF field. Range [0, 1).
> > > > > > +
> > > > > > + * - ``sampling_frequency``
> > > > > > + - Hz
> > > > > > + - OSK ramp rate: SYSCLK / (4 * divider).
> > > > > > +
> > > > > > + * - ``pinctrl_en``
> > > > > > + - boolean
> > > > > > + - Enable manual external pin control. When enabled, the OSK pin directly
> > > > > > + gates the output on/off instead of using the automatic ramp.
> > > > >
> > > > > I wonder if we should split the various OSK modes into different channels given
> > > > > only some properties apply to each of automatic and manual modes. Also I think
> > > > > automatic mode is meaningless without pinctrl_en (so that can be replaced
> > > > > by simply enabling that mode). I have no idea if anyone cares about pin ctrl
> > > > > with manual mode or not? That one seems even more odd.
> > > >
> > > > OSK is either in manual or auto:
> > > > * In manual mode the OSK pin enables and disables the output based on its level.
> > > > * In auto, the OSK pin controls the direction the amplitude updates.
> > > >
> > > > If we enable RAM mode, and other modes do not target amplitude, the only way to
> > > > manually configure the amplitude in software (i.e. without using an OSK gpio)
> > > > is going manual mode (scale_step == 0), disable this pinctrl_en and then set the
> > > > scale property (ASF register). That is the only reason I added this property.
> > >
> > > Ah. Maybe we hide that away and make the amplitude a property of RAM channel?
> >
> > And what if a user is in fact willing to use the OSK pin?
>
> Sigh. I don't have a good answer, but I don't like the weird 'special' nature
> of this attribute. I guess sometimes there isn't a good answer to be found.
>
> OSK + RAM is odd enough that I'm not that bothered if we have to go weird
> here.
I will remove this then. I suppose that adding another possible value for
scale_roc allows us to go around this, i.e., if scale_roc is such that the
amplitude step is a value equal or greater than the amplitude itself (or even the
max amplitude value) I could assume that this would be the manual mode with pinctrl
enable as it would be like an automatic mode with no ramp, but an immediate on-off
transition. Then scale_roc == 0 means manual mode with pinctrl disabled.
Will try to develop on this....
> >
> > > It can do this magic under the hood. I don't mind the attributes for OSK changing
> > > if this trick is in use (they won't be active anyway).
> >
> > OSK has the highest priority of all, but it only acts on the amplitude.
> >
> > > > > > +
> > > > > > + * - ``scale_step``
> > > > > > + - fractional
> > > > > > + - Automatic OSK amplitude step. Writing non-zero enables automatic OSK
> > > > > > + and sets the per-tick increment. Writing ``0`` disables it. Rounded to
> > > > > > + nearest hardware step: 0.000061, 0.000122, 0.000244 or 0.000488.
> > > > >
> > > > > Similar thing about rate of change of amplitude fitting better with current ABI
> > > > > than step does.
> > > >
> > > > ok... and this one is still missing the correspondent available attr.
> > >
> > > Available is a bit tricky when there is an inverse relationship involved in the maths
> > > as what do we put the step as. Maybe we should add a note on that to the ABI
> > > docs. [min step max] where step gives the minimum step that due to non linearity
> > > may not be applicable between discrete values that may be taken away from that
> > > minimum granularity base value. If that occurs the driver will round to the
> > > nearest possible value.
--
Kind regards,
Rodrigo Alencar
^ permalink raw reply
* Re: [PATCH 5/8] drm/panthor: Minor scheduler refactoring
From: Boris Brezillon @ 2026-05-06 10:33 UTC (permalink / raw)
To: Ketil Johnsen
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Jonathan Corbet, Shuah Khan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin
In-Reply-To: <20260505140516.1372388-6-ketil.johnsen@arm.com>
On Tue, 5 May 2026 16:05:11 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:
> From: Florent Tomasin <florent.tomasin@arm.com>
>
> Refactor parts of the group scheduling logic into new helper functions.
> This will simplify addition of the protected mode feature.
>
> Remove redundant assignments of csg_slot.
>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> Co-developed-by: Ketil Johnsen <ketil.johnsen@arm.com>
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 135 +++++++++++++++---------
> 1 file changed, 86 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5ee386338005c..987072bd867c4 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -1934,6 +1934,12 @@ static void csgs_upd_ctx_init(struct panthor_csg_slots_upd_ctx *ctx)
> memset(ctx, 0, sizeof(*ctx));
> }
>
> +static void csgs_upd_ctx_ring_doorbell(struct panthor_csg_slots_upd_ctx *ctx,
> + u32 csg_id)
> +{
> + ctx->update_mask |= BIT(csg_id);
> +}
> +
> static void csgs_upd_ctx_queue_reqs(struct panthor_device *ptdev,
> struct panthor_csg_slots_upd_ctx *ctx,
> u32 csg_id, u32 value, u32 mask)
> @@ -1944,7 +1950,8 @@ static void csgs_upd_ctx_queue_reqs(struct panthor_device *ptdev,
>
> ctx->requests[csg_id].value = (ctx->requests[csg_id].value & ~mask) | (value & mask);
> ctx->requests[csg_id].mask |= mask;
> - ctx->update_mask |= BIT(csg_id);
> +
> + csgs_upd_ctx_ring_doorbell(ctx, csg_id);
> }
>
> static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> @@ -1961,8 +1968,12 @@ static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> while (update_slots) {
> struct panthor_fw_csg_iface *csg_iface;
> u32 csg_id = ffs(update_slots) - 1;
> + u32 req_mask = ctx->requests[csg_id].mask;
>
> update_slots &= ~BIT(csg_id);
> + if (!req_mask)
> + continue;
Looks like something that should be in patch 7, where you update the
doorbell_req register, and then call csgs_upd_ctx_ring_doorbell(),
meaning req_mask can be zero. The other option would be to teach
panthor_csg_slots_upd_ctx about CS doorbells, and let
csgs_upd_ctx_apply_locked() toggle the doorbell_req.
> +
> csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> panthor_fw_update_reqs(csg_iface, req,
> ctx->requests[csg_id].value,
> @@ -1979,6 +1990,9 @@ static int csgs_upd_ctx_apply_locked(struct panthor_device *ptdev,
> int ret;
>
> update_slots &= ~BIT(csg_id);
> + if (!req_mask)
> + continue;
> +
> csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
>
> ret = panthor_fw_csg_wait_acks(ptdev, csg_id, req_mask, &acked, 100);
^ permalink raw reply
* Re: [PATCH v8 1/4] bug/kunit: Core support for suppressing warning backtraces
From: Albert Esteve @ 2026-05-06 10:11 UTC (permalink / raw)
To: David Gow
Cc: Arnd Bergmann, Brendan Higgins, Rae Moar, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jonathan Corbet, Shuah Khan, Andrew Morton, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-kernel,
linux-arch, linux-kselftest, kunit-dev, dri-devel, workflows,
linux-riscv, linux-doc, peterz, Alessandro Carminati,
Guenter Roeck, Kees Cook
In-Reply-To: <c5c28f16-3940-4960-84e6-33f5479a5450@davidgow.net>
On Wed, May 6, 2026 at 11:40 AM David Gow <david@davidgow.net> wrote:
>
> Le 04/05/2026 à 3:41 PM, Albert Esteve a écrit :
> > From: Alessandro Carminati <acarmina@redhat.com>
> >
> > Some unit tests intentionally trigger warning backtraces by passing bad
> > parameters to kernel API functions. Such unit tests typically check the
> > return value from such calls, not the existence of the warning backtrace.
> >
> > Such intentionally generated warning backtraces are neither desirable
> > nor useful for a number of reasons:
> > - They can result in overlooked real problems.
> > - A warning that suddenly starts to show up in unit tests needs to be
> > investigated and has to be marked to be ignored, for example by
> > adjusting filter scripts. Such filters are ad hoc because there is
> > no real standard format for warnings. On top of that, such filter
> > scripts would require constant maintenance.
> >
> > Solve the problem by providing a means to suppress warning backtraces
> > originating from the current kthread while executing test code. Since
> > each KUnit test runs in its own kthread, this effectively scopes
> > suppression to the test that enabled it. Limit changes to generic code
> > to the absolute minimum.
> >
> > Implementation details:
> > Suppression is integrated into the existing KUnit hooks infrastructure
> > in test-bug.h, reusing the kunit_running static branch for zero
> > overhead when no tests are running.
> >
> > Suppression is checked at three points in the warning path:
> > - In warn_slowpath_fmt(), the check runs before any output, fully
> > suppressing both message and backtrace. This covers architectures
> > without __WARN_FLAGS.
> > - In __warn_printk(), the check suppresses the warning message text.
> > This covers architectures that define __WARN_FLAGS but not their own
> > __WARN_printf (arm64, loongarch, parisc, powerpc, riscv, sh), where
> > the message is printed before the trap enters __report_bug().
> > - In __report_bug(), the check runs before __warn() is called,
> > suppressing the backtrace and stack dump.
> >
> > To avoid double-counting on architectures where both __warn_printk()
> > and __report_bug() run for the same warning, kunit_is_suppressed_warning()
> > takes a bool parameter: true to increment the suppression counter
> > (used in warn_slowpath_fmt and __report_bug), false to check only
> > (used in __warn_printk).
> >
> > The suppression state is dynamically allocated via kunit_kzalloc() and
> > tied to the KUnit test lifecycle via kunit_add_action(), ensuring
> > automatic cleanup at test exit. Writer-side access to the global
> > suppression list is serialized with a spinlock; readers use RCU.
> >
> > Three API forms are provided:
> > - kunit_warning_suppress(test) { ... }: scoped, uses __cleanup for
> > automatic teardown on scope exit, kunit_add_action() as safety net
> > for abnormal exits (e.g. kthread_exit from failed assertions).
> > Suppression handle is only accessible inside the block.
> > - KUNIT_START/END_SUPPRESSED_WARNING(test): manual macros for larger
> > blocks or when warning counts need to be checked after suppression
> > ends. Limited to one pair per scope.
> > - kunit_start/end_suppress_warning(test): direct functions returning
> > an explicit handle, for retaining the handle within the test,
> > or for cross-function usage.
> >
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> > Reviewed-by: Kees Cook <kees@kernel.org>
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
>
> This looks pretty good to me, thanks.
>
> Reviewed-by: David Gow <david@davidgow.net>
>
> It's maybe slightly over-the-top to now have three different ways of
> enabling warning suppression: I'd probably personally get rid of
> KUNIT_START/END_SUPPRESSED_WARNING() if we had to lose one. But if
> there's a real reason to prefer keeping all three, it's not actually a
> problem to do so.
Thanks for the review!
I think the three forms earn their keep: the scoped form is the go-to
for most cases, but the macros avoid indentation without requiring
users to manage a raw pointer. I initially removed the macros and
added them back later. Direct calls to the functions will be less
frequent, used only when you need the handle.
That said, if it becomes a maintenance burden, the macros are the
easiest to drop since they're thin wrappers. Let me know if you prefer
them to be dropped, and I will send a v9 with that and the
`KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT` additions to the drm test
patch.
BR,
Albert.
>
> Regardless, this series is looking pretty ready to me. Let me know if
> you're planning a v9, otherwise we'll take this when you're ready.
>
> Cheers,
> -- David
>
> > include/kunit/test-bug.h | 25 +++++++++
> > include/kunit/test.h | 138 +++++++++++++++++++++++++++++++++++++++++++++++
> > kernel/panic.c | 15 +++++-
> > lib/bug.c | 10 ++++
> > lib/kunit/Makefile | 3 +-
> > lib/kunit/bug.c | 115 +++++++++++++++++++++++++++++++++++++++
> > lib/kunit/hooks-impl.h | 2 +
> > 7 files changed, 305 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
> > index 47aa8f21ccce8..6237e48ceadfd 100644
> > --- a/include/kunit/test-bug.h
> > +++ b/include/kunit/test-bug.h
> > @@ -23,6 +23,7 @@ DECLARE_STATIC_KEY_FALSE(kunit_running);
> > extern struct kunit_hooks_table {
> > __printf(3, 4) void (*fail_current_test)(const char*, int, const char*, ...);
> > void *(*get_static_stub_address)(struct kunit *test, void *real_fn_addr);
> > + bool (*is_suppressed_warning)(bool count);
> > } kunit_hooks;
> >
> > /**
> > @@ -60,9 +61,33 @@ static inline struct kunit *kunit_get_current_test(void)
> > } \
> > } while (0)
> >
> > +/**
> > + * kunit_is_suppressed_warning() - Check if warnings are being suppressed
> > + * by the current KUnit test.
> > + * @count: if true, increment the suppression counter on match.
> > + *
> > + * Returns true if the current task has active warning suppression.
> > + * Uses the kunit_running static branch for zero overhead when no tests run.
> > + *
> > + * A single WARN*() may traverse multiple call sites in the warning path
> > + * (e.g., __warn_printk() and __report_bug()). Pass @count = true at the
> > + * primary suppression point to count each warning exactly once, and
> > + * @count = false at secondary points to suppress output without
> > + * inflating the count.
> > + */
> > +static inline bool kunit_is_suppressed_warning(bool count)
> > +{
> > + if (!static_branch_unlikely(&kunit_running))
> > + return false;
> > +
> > + return kunit_hooks.is_suppressed_warning &&
> > + kunit_hooks.is_suppressed_warning(count);
> > +}
> > +
> > #else
> >
> > static inline struct kunit *kunit_get_current_test(void) { return NULL; }
> > +static inline bool kunit_is_suppressed_warning(bool count) { return false; }
> >
> > #define kunit_fail_current_test(fmt, ...) do {} while (0)
> >
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index 9cd1594ab697d..f278ec028019c 100644
> > --- a/include/kunit/test.h
> > +++ b/include/kunit/test.h
> > @@ -1795,4 +1795,142 @@ do { \
> > // include resource.h themselves if they need it.
> > #include <kunit/resource.h>
> >
> > +/*
> > + * Warning backtrace suppression API.
> > + *
> > + * Suppresses WARN*() backtraces on the current task while active. Three forms
> > + * are provided, in order of convenience:
> > + *
> > + * - Scoped: kunit_warning_suppress(test) { ... }
> > + * Suppression is active for the duration of the block. On normal exit,
> > + * the for-loop increment deactivates suppression. On early exit (break,
> > + * return, goto), the __cleanup attribute fires. On kthread_exit() (e.g.,
> > + * a failed KUnit assertion), kunit_add_action() cleans up at test
> > + * teardown. The suppression handle is only accessible inside the block,
> > + * so warning counts must be checked before the block exits.
> > + *
> > + * - Manual macros: KUNIT_[START|END]_SUPPRESSED_WARNING(test)
> > + * Suppression spans an explicit range in the same scope. kunit_add_action()
> > + * guarantees cleanup even if KUNIT_END_SUPPRESSED_WARNING() is not reached.
> > + * Prefer this form when suppressing warnings across a large block where
> > + * extra indentation is undesirable, or when the warning count needs to be
> > + * checked after suppression ends. Limited to one pair per scope.
> > + *
> > + * - Direct: kunit_start_suppress_warning() / kunit_end_suppress_warning()
> > + * The underlying functions, returning an explicit handle pointer. Use
> > + * when the handle needs to be retained (e.g., for post-suppression
> > + * count checks) or passed across helper functions.
> > + */
> > +struct kunit_suppressed_warning;
> > +
> > +struct kunit_suppressed_warning *
> > +kunit_start_suppress_warning(struct kunit *test);
> > +void kunit_end_suppress_warning(struct kunit *test,
> > + struct kunit_suppressed_warning *w);
> > +int kunit_suppressed_warning_count(struct kunit_suppressed_warning *w);
> > +void __kunit_suppress_auto_cleanup(struct kunit_suppressed_warning **wp);
> > +bool kunit_has_active_suppress_warning(void);
> > +
> > +/**
> > + * kunit_warning_suppress() - Suppress WARN*() backtraces for the duration
> > + * of a block.
> > + * @test: The test context object.
> > + *
> > + * Scoped form of the suppression API. Suppression starts when the block is
> > + * entered and ends automatically when the block exits through any path. See
> > + * the section comment above for the cleanup guarantees on each exit path.
> > + * Fails the test if suppression is already active; nesting is not supported.
> > + *
> > + * The warning count can be checked inside the block via
> > + * KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(). The handle is not accessible
> > + * after the block exits.
> > + *
> > + * Example::
> > + *
> > + * kunit_warning_suppress(test) {
> > + * trigger_warning();
> > + * KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> > + * }
> > + */
> > +#define kunit_warning_suppress(test) \
> > + for (struct kunit_suppressed_warning *__kunit_suppress \
> > + __cleanup(__kunit_suppress_auto_cleanup) = \
> > + kunit_start_suppress_warning(test); \
> > + __kunit_suppress; \
> > + kunit_end_suppress_warning(test, __kunit_suppress), \
> > + __kunit_suppress = NULL)
> > +
> > +/**
> > + * KUNIT_START_SUPPRESSED_WARNING() - Begin suppressing WARN*() backtraces.
> > + * @test: The test context object.
> > + *
> > + * Manual form of the suppression API. Must be paired with
> > + * KUNIT_END_SUPPRESSED_WARNING() in the same scope. See the section comment
> > + * above for cleanup guarantees. Fails the test if suppression is already
> > + * active; nesting is not supported. Limited to one pair per scope; use
> > + * sequential kunit_warning_suppress() blocks or the direct function API
> > + * when more than one suppression region is needed.
> > + *
> > + * Example::
> > + *
> > + * KUNIT_START_SUPPRESSED_WARNING(test);
> > + * trigger_code_that_should_warn_once();
> > + * KUNIT_END_SUPPRESSED_WARNING(test);
> > + * KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> > + */
> > +#define KUNIT_START_SUPPRESSED_WARNING(test) \
> > + struct kunit_suppressed_warning *__kunit_suppress = \
> > + kunit_start_suppress_warning(test)
> > +
> > +/**
> > + * KUNIT_END_SUPPRESSED_WARNING() - End suppressing WARN*() backtraces.
> > + * @test: The test context object.
> > + *
> > + * Deactivates suppression started by KUNIT_START_SUPPRESSED_WARNING().
> > + * The warning count remains readable via KUNIT_SUPPRESSED_WARNING_COUNT()
> > + * after this call.
> > + */
> > +#define KUNIT_END_SUPPRESSED_WARNING(test) \
> > + kunit_end_suppress_warning(test, __kunit_suppress)
> > +
> > +/**
> > + * KUNIT_SUPPRESSED_WARNING_COUNT() - Returns the suppressed warning count.
> > + *
> > + * Returns the number of WARN*() calls suppressed since the current
> > + * suppression block started, or 0 if the handle is NULL. Usable inside a
> > + * kunit_warning_suppress() block or after KUNIT_END_SUPPRESSED_WARNING().
> > + */
> > +#define KUNIT_SUPPRESSED_WARNING_COUNT() \
> > + kunit_suppressed_warning_count(__kunit_suppress)
> > +
> > +/**
> > + * KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT() - Sets an expectation that the
> > + * suppressed warning count equals
> > + * @expected.
> > + * @test: The test context object.
> > + * @expected: an expression that evaluates to the expected warning count.
> > + *
> > + * Sets an expectation that the number of suppressed WARN*() calls equals
> > + * @expected. This is semantically equivalent to
> > + * KUNIT_EXPECT_EQ(@test, KUNIT_SUPPRESSED_WARNING_COUNT(), @expected).
> > + * See KUNIT_EXPECT_EQ() for more information.
> > + */
> > +#define KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, expected) \
> > + KUNIT_EXPECT_EQ(test, KUNIT_SUPPRESSED_WARNING_COUNT(), expected)
> > +
> > +/**
> > + * KUNIT_ASSERT_SUPPRESSED_WARNING_COUNT() - Sets an assertion that the
> > + * suppressed warning count equals
> > + * @expected.
> > + * @test: The test context object.
> > + * @expected: an expression that evaluates to the expected warning count.
> > + *
> > + * Sets an assertion that the number of suppressed WARN*() calls equals
> > + * @expected. This is the same as KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(),
> > + * except it causes an assertion failure (see KUNIT_ASSERT_TRUE()) when the
> > + * assertion is not met.
> > + */
> > +#define KUNIT_ASSERT_SUPPRESSED_WARNING_COUNT(test, expected) \
> > + KUNIT_ASSERT_EQ(test, KUNIT_SUPPRESSED_WARNING_COUNT(), expected)
> > +
> > #endif /* _KUNIT_TEST_H */
> > diff --git a/kernel/panic.c b/kernel/panic.c
> > index c78600212b6c1..697d8ca054bef 100644
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -39,6 +39,7 @@
> > #include <linux/sys_info.h>
> > #include <trace/events/error_report.h>
> > #include <asm/sections.h>
> > +#include <kunit/test-bug.h>
> >
> > #define PANIC_TIMER_STEP 100
> > #define PANIC_BLINK_SPD 18
> > @@ -1080,9 +1081,14 @@ void __warn(const char *file, int line, void *caller, unsigned taint,
> > void warn_slowpath_fmt(const char *file, int line, unsigned taint,
> > const char *fmt, ...)
> > {
> > - bool rcu = warn_rcu_enter();
> > + bool rcu;
> > struct warn_args args;
> >
> > + if (kunit_is_suppressed_warning(true))
> > + return;
> > +
> > + rcu = warn_rcu_enter();
> > +
> > pr_warn(CUT_HERE);
> >
> > if (!fmt) {
> > @@ -1102,9 +1108,14 @@ EXPORT_SYMBOL(warn_slowpath_fmt);
> > #else
> > void __warn_printk(const char *fmt, ...)
> > {
> > - bool rcu = warn_rcu_enter();
> > + bool rcu;
> > va_list args;
> >
> > + if (kunit_is_suppressed_warning(false))
> > + return;
> > +
> > + rcu = warn_rcu_enter();
> > +
> > pr_warn(CUT_HERE);
> >
> > va_start(args, fmt);
> > diff --git a/lib/bug.c b/lib/bug.c
> > index 623c467a8b76c..a5cebde554ed8 100644
> > --- a/lib/bug.c
> > +++ b/lib/bug.c
> > @@ -48,6 +48,7 @@
> > #include <linux/rculist.h>
> > #include <linux/ftrace.h>
> > #include <linux/context_tracking.h>
> > +#include <kunit/test-bug.h>
> >
> > extern struct bug_entry __start___bug_table[], __stop___bug_table[];
> >
> > @@ -223,6 +224,15 @@ static enum bug_trap_type __report_bug(struct bug_entry *bug, unsigned long buga
> > no_cut = bug->flags & BUGFLAG_NO_CUT_HERE;
> > has_args = bug->flags & BUGFLAG_ARGS;
> >
> > +#ifdef CONFIG_KUNIT
> > + /*
> > + * Before the once logic so suppressed warnings do not consume
> > + * the single-fire budget of WARN_ON_ONCE().
> > + */
> > + if (warning && kunit_is_suppressed_warning(true))
> > + return BUG_TRAP_TYPE_WARN;
> > +#endif
> > +
> > if (warning && once) {
> > if (done)
> > return BUG_TRAP_TYPE_WARN;
> > diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> > index 656f1fa35abcc..4592f9d0aa8dd 100644
> > --- a/lib/kunit/Makefile
> > +++ b/lib/kunit/Makefile
> > @@ -10,7 +10,8 @@ kunit-objs += test.o \
> > executor.o \
> > attributes.o \
> > device.o \
> > - platform.o
> > + platform.o \
> > + bug.o
> >
> > ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
> > kunit-objs += debugfs.o
> > diff --git a/lib/kunit/bug.c b/lib/kunit/bug.c
> > new file mode 100644
> > index 0000000000000..b0b6778d7399a
> > --- /dev/null
> > +++ b/lib/kunit/bug.c
> > @@ -0,0 +1,115 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * KUnit helpers for backtrace suppression
> > + *
> > + * Copyright (C) 2025 Alessandro Carminati <acarmina@redhat.com>
> > + * Copyright (C) 2024 Guenter Roeck <linux@roeck-us.net>
> > + */
> > +
> > +#include <kunit/resource.h>
> > +#include <linux/export.h>
> > +#include <linux/rculist.h>
> > +#include <linux/sched.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include "hooks-impl.h"
> > +
> > +struct kunit_suppressed_warning {
> > + struct list_head node;
> > + struct task_struct *task;
> > + struct kunit *test;
> > + int counter;
> > +};
> > +
> > +static LIST_HEAD(suppressed_warnings);
> > +static DEFINE_SPINLOCK(suppressed_warnings_lock);
> > +
> > +static void kunit_suppress_warning_remove(struct kunit_suppressed_warning *w)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&suppressed_warnings_lock, flags);
> > + list_del_rcu(&w->node);
> > + spin_unlock_irqrestore(&suppressed_warnings_lock, flags);
> > + synchronize_rcu(); /* Wait for readers to finish */
> > +}
> > +
> > +KUNIT_DEFINE_ACTION_WRAPPER(kunit_suppress_warning_cleanup,
> > + kunit_suppress_warning_remove,
> > + struct kunit_suppressed_warning *);
> > +
> > +bool kunit_has_active_suppress_warning(void)
> > +{
> > + return __kunit_is_suppressed_warning_impl(false);
> > +}
> > +EXPORT_SYMBOL_GPL(kunit_has_active_suppress_warning);
> > +
> > +struct kunit_suppressed_warning *
> > +kunit_start_suppress_warning(struct kunit *test)
> > +{
> > + struct kunit_suppressed_warning *w;
> > + unsigned long flags;
> > + int ret;
> > +
> > + if (kunit_has_active_suppress_warning()) {
> > + KUNIT_FAIL(test, "Another suppression block is already active");
> > + return NULL;
> > + }
> > +
> > + w = kunit_kzalloc(test, sizeof(*w), GFP_KERNEL);
> > + if (!w)
> > + return NULL;
> > +
> > + w->task = current;
> > + w->test = test;
> > +
> > + spin_lock_irqsave(&suppressed_warnings_lock, flags);
> > + list_add_rcu(&w->node, &suppressed_warnings);
> > + spin_unlock_irqrestore(&suppressed_warnings_lock, flags);
> > +
> > + ret = kunit_add_action_or_reset(test,
> > + kunit_suppress_warning_cleanup, w);
> > + if (ret)
> > + return NULL;
> > +
> > + return w;
> > +}
> > +EXPORT_SYMBOL_GPL(kunit_start_suppress_warning);
> > +
> > +void kunit_end_suppress_warning(struct kunit *test,
> > + struct kunit_suppressed_warning *w)
> > +{
> > + if (!w)
> > + return;
> > + kunit_release_action(test, kunit_suppress_warning_cleanup, w);
> > +}
> > +EXPORT_SYMBOL_GPL(kunit_end_suppress_warning);
> > +
> > +void __kunit_suppress_auto_cleanup(struct kunit_suppressed_warning **wp)
> > +{
> > + if (*wp)
> > + kunit_end_suppress_warning((*wp)->test, *wp);
> > +}
> > +EXPORT_SYMBOL_GPL(__kunit_suppress_auto_cleanup);
> > +
> > +int kunit_suppressed_warning_count(struct kunit_suppressed_warning *w)
> > +{
> > + return w ? w->counter : 0;
> > +}
> > +EXPORT_SYMBOL_GPL(kunit_suppressed_warning_count);
> > +
> > +bool __kunit_is_suppressed_warning_impl(bool count)
> > +{
> > + struct kunit_suppressed_warning *w;
> > +
> > + guard(rcu)();
> > + list_for_each_entry_rcu(w, &suppressed_warnings, node) {
> > + if (w->task == current) {
> > + if (count)
> > + w->counter++;
> > + return true;
> > + }
> > + }
> > +
> > + return false;
> > +}
> > diff --git a/lib/kunit/hooks-impl.h b/lib/kunit/hooks-impl.h
> > index 4e71b2d0143ba..d8720f2616925 100644
> > --- a/lib/kunit/hooks-impl.h
> > +++ b/lib/kunit/hooks-impl.h
> > @@ -19,6 +19,7 @@ void __printf(3, 4) __kunit_fail_current_test_impl(const char *file,
> > int line,
> > const char *fmt, ...);
> > void *__kunit_get_static_stub_address_impl(struct kunit *test, void *real_fn_addr);
> > +bool __kunit_is_suppressed_warning_impl(bool count);
> >
> > /* Code to set all of the function pointers. */
> > static inline void kunit_install_hooks(void)
> > @@ -26,6 +27,7 @@ static inline void kunit_install_hooks(void)
> > /* Install the KUnit hook functions. */
> > kunit_hooks.fail_current_test = __kunit_fail_current_test_impl;
> > kunit_hooks.get_static_stub_address = __kunit_get_static_stub_address_impl;
> > + kunit_hooks.is_suppressed_warning = __kunit_is_suppressed_warning_impl;
> > }
> >
> > #endif /* _KUNIT_HOOKS_IMPL_H */
> >
>
^ permalink raw reply
* Re: [PATCH 4/8] drm/panthor: Add support for protected memory allocation in panthor
From: Maxime Ripard @ 2026-05-06 10:08 UTC (permalink / raw)
To: Ketil Johnsen
Cc: David Airlie, Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
Jonathan Corbet, Shuah Khan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Boris Brezillon, Steven Price, Liviu Dudau, Daniel Almeida,
Alice Ryhl, Matthias Brugger, AngeloGioacchino Del Regno,
dri-devel, linux-doc, linux-kernel, linux-media, linaro-mm-sig,
linux-arm-kernel, linux-mediatek, Florent Tomasin
In-Reply-To: <20260505140516.1372388-5-ketil.johnsen@arm.com>
[-- Attachment #1: Type: text/plain, Size: 1434 bytes --]
Hi,
On Tue, May 05, 2026 at 04:05:10PM +0200, Ketil Johnsen wrote:
> From: Florent Tomasin <florent.tomasin@arm.com>
>
> This patch allows Panthor to allocate buffer objects from a
> protected heap. The Panthor driver should be seen as a consumer
> of the heap and not an exporter.
>
> Protected memory buffers needed by the Panthor driver:
> - On CSF FW load, the Panthor driver must allocate a protected
> buffer object to hold data to use by the FW when in protected
> mode. This protected buffer object is owned by the device
> and does not belong to a process.
> - On CSG creation, the Panthor driver must allocate a protected
> suspend buffer object for the FW to store data when suspending
> the CSG while in protected mode. The kernel owns this allocation
> and does not allow user space mapping. The format of the data
> in this buffer is only known by the FW and does not need to be
> shared with other entities.
>
> The driver will retrieve the protected heap using the name of the
> heap provided to the driver as module parameter.
I know it's what dma_heap_find asks for, but I wonder if it wouldn't be
better in the device tree and lookup through the device node? heaps are
going to have a node anyway, right?
This would allow you to have a default that works and not mess to much
with the kernel parameters that aren't always easy to change for
end-users.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH v4 10/27] mtd: spi-nor: swp: Create a helper that writes SR, CR and checks
From: Miquel Raynal @ 2026-05-06 9:54 UTC (permalink / raw)
To: Michael Walle
Cc: Pratyush Yadav, Takahiro Kuwano, Richard Weinberger,
Vignesh Raghavendra, Jonathan Corbet, Sean Anderson,
Thomas Petazzoni, Steam Lin, linux-mtd, linux-kernel, linux-doc
In-Reply-To: <DIBH3CCTRHPW.1J4LOJQAJ50XE@kernel.org>
On 06/05/2026 at 11:06:22 +02, "Michael Walle" <mwalle@kernel.org> wrote:
> On Tue May 5, 2026 at 6:05 PM CEST, Pratyush Yadav wrote:
>> On Fri, Apr 03 2026, Miquel Raynal wrote:
>>
>>> There are many helpers already to either read and/or write SR and/or CR,
>>> as well as sometimes check the returned values. In order to be able to
>>> switch from a 1 byte status register to a 2 bytes status register while
>>> keeping the same level of verification, let's introduce a new helper
>>> that writes them both (atomically) and then reads them back (separated)
>>> to compare the values.
>>>
>>> In case 2 bytes registers are not supported, we still have the usual
>>> fallback available in the helper being exported to the rest of the core.
>>>
>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>>
>> I'm confused. Doesn't spi_nor_write_16bit_sr_and_check() do the same
>> thing? How are these two different?
>
> So I've never come around to finish reviewing this series due to
> personal reasons, but here are my remarks. Personally, I really
> don't like all these multiple helpers doing almost the same thing.
> But it is what is is for now.
>
> Back when reviewing this series, I've digged into this and it mostly
> evolve around how to enable the QE bit, that is defined in the 15th
> SFDP DWORD. One could see how we could consolidate all the status
> register handling in one function which are then called by the
> different (specified) quad_enable helpers.
I already had a look, it doesn't seem so straightforward. But I will
look into it deeper, I am willing to improve things. There will anyway
be a wide variety of helpers because there is a wide variety of QER
possibilities. What we can do though, is to decouple status register
writing and QE bit masking.
However, I would like to point that this is totally orthogonal to the
whole (almost 30 patch long) locking cleanup and CMP feature series, so
I do not plan to change this particular implementation in
v5. Reorganising these helpers should be done in its own follow-up
series.
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH v8 3/4] drm: Suppress intentional warning backtraces in scaling unit tests
From: Maxime Ripard @ 2026-05-06 9:48 UTC (permalink / raw)
To: David Gow
Cc: Albert Esteve, Arnd Bergmann, Brendan Higgins, Rae Moar,
Maarten Lankhorst, Thomas Zimmermann, David Airlie, Simona Vetter,
Jonathan Corbet, Shuah Khan, Andrew Morton, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, linux-kernel,
linux-arch, linux-kselftest, kunit-dev, dri-devel, workflows,
linux-riscv, linux-doc, peterz, Guenter Roeck,
Linux Kernel Functional Testing, Dan Carpenter, Maíra Canal,
Alessandro Carminati, Simona Vetter
In-Reply-To: <47d78cad-0839-4602-9cb3-d1f672739e7e@davidgow.net>
[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]
On Wed, May 06, 2026 at 05:38:50PM +0800, David Gow wrote:
> Le 04/05/2026 à 3:41 PM, Albert Esteve a écrit :
> > From: Guenter Roeck <linux@roeck-us.net>
> >
> > The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
> > intentionally trigger warning backtraces by providing bad parameters to
> > the tested functions. What is tested is the return value, not the existence
> > of a warning backtrace. Suppress the backtraces to avoid clogging the
> > kernel log and distraction from real problems.
> >
> > Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> > Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Acked-by: Maíra Canal <mcanal@igalia.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: David Airlie <airlied@gmail.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> > Acked-by: David Gow <david@davidgow.net>
> > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > ---
>
> I'm happy with this either with or without the extra check for the warning
> count.
>
> Acked-by: David Gow <david@davidgow.net>
>
> We'll take this patch in the kunit tree along with the rest of the series
> once everyone's happy.
If there's no need for a new version, feel free to merge these patches,
I'm totally fine with it being addressed as a follow-up.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply
* Re: [PATCH v8 4/4] kunit: Add documentation for warning backtrace suppression API
From: David Gow @ 2026-05-06 9:38 UTC (permalink / raw)
To: Albert Esteve, Arnd Bergmann, Brendan Higgins, Rae Moar,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jonathan Corbet, Shuah Khan, Andrew Morton,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-kernel, linux-arch, linux-kselftest, kunit-dev, dri-devel,
workflows, linux-riscv, linux-doc, peterz, Guenter Roeck,
Linux Kernel Functional Testing, Dan Carpenter,
Alessandro Carminati, Kees Cook
In-Reply-To: <20260504-kunit_add_support-v8-4-3e5957cdd235@redhat.com>
Le 04/05/2026 à 3:41 PM, Albert Esteve a écrit :
> From: Guenter Roeck <linux@roeck-us.net>
>
> Document API functions for suppressing warning backtraces.
>
> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: David Gow <davidgow@google.com>
> Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> Reviewed-by: David Gow <david@davidgow.net>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
This is great, thanks!
Reviewed-by: David Gow <david@davidgow.net>
Cheers,
-- David
> Documentation/dev-tools/kunit/usage.rst | 63 ++++++++++++++++++++++++++++++++-
> 1 file changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
> index ebd06f5ea4550..25724f7e72969 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -157,6 +157,67 @@ Alternatively, one can take full control over the error message by using
> if (some_setup_function())
> KUNIT_FAIL(test, "Failed to setup thing for testing");
>
> +Suppressing warning backtraces
> +------------------------------
> +
> +Some unit tests trigger warning backtraces either intentionally or as a side
> +effect. Such backtraces are normally undesirable since they distract from
> +the actual test and may result in the impression that there is a problem.
> +
> +Backtraces can be suppressed with **task-scoped suppression**: while
> +suppression is active on the current task, the backtrace and stack dump from
> +``WARN*()``, ``WARN_ON*()``, and related macros on that task are suppressed.
> +Three API forms are available, in order of convenience.
> +
> +- Scoped suppression is the simplest form. Wrap the code that triggers
> + warnings in a ``kunit_warning_suppress()`` block:
> +
> +.. code-block:: c
> +
> + static void some_test(struct kunit *test)
> + {
> + kunit_warning_suppress(test) {
> + trigger_backtrace();
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> + }
> + }
> +
> +.. note::
> + The warning count must be checked inside the block; the suppression handle
> + is not accessible after the block exits.
> +
> +- Manual macros are useful when the suppressed region is large enough that
> + extra indentation is undesirable, or when the warning count needs to be
> + checked after suppression ends. ``KUNIT_START_SUPPRESSED_WARNING()`` must
> + appear before ``KUNIT_END_SUPPRESSED_WARNING()`` in the same scope.
> + Limited to one pair per scope.
> +
> +.. code-block:: c
> +
> + static void some_test(struct kunit *test)
> + {
> + KUNIT_START_SUPPRESSED_WARNING(test);
> + trigger_backtrace();
> + KUNIT_END_SUPPRESSED_WARNING(test);
> +
> + KUNIT_EXPECT_SUPPRESSED_WARNING_COUNT(test, 1);
> + }
> +
> +- Direct functions return an explicit handle pointer. Use them when the handle
> + needs to be retained or passed across helper functions:
> +
> +.. code-block:: c
> +
> + static void some_test(struct kunit *test)
> + {
> + struct kunit_suppressed_warning *w;
> +
> + w = kunit_start_suppress_warning(test);
> + trigger_backtrace();
> + kunit_end_suppress_warning(test, w);
> +
> + KUNIT_EXPECT_EQ(test, kunit_suppressed_warning_count(w), 1);
> + }
>
> Test Suites
> ~~~~~~~~~~~
> @@ -1211,4 +1272,4 @@ For example:
> dev_managed_string = devm_kstrdup(fake_device, "Hello, World!");
>
> // Everything is cleaned up automatically when the test ends.
> - }
> \ No newline at end of file
> + }
>
^ permalink raw reply
* Re: [PATCH v8 3/4] drm: Suppress intentional warning backtraces in scaling unit tests
From: David Gow @ 2026-05-06 9:38 UTC (permalink / raw)
To: Albert Esteve, Arnd Bergmann, Brendan Higgins, Rae Moar,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jonathan Corbet, Shuah Khan, Andrew Morton,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: linux-kernel, linux-arch, linux-kselftest, kunit-dev, dri-devel,
workflows, linux-riscv, linux-doc, peterz, Guenter Roeck,
Linux Kernel Functional Testing, Dan Carpenter, Maíra Canal,
Alessandro Carminati, Simona Vetter
In-Reply-To: <20260504-kunit_add_support-v8-3-3e5957cdd235@redhat.com>
Le 04/05/2026 à 3:41 PM, Albert Esteve a écrit :
> From: Guenter Roeck <linux@roeck-us.net>
>
> The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
> intentionally trigger warning backtraces by providing bad parameters to
> the tested functions. What is tested is the return value, not the existence
> of a warning backtrace. Suppress the backtraces to avoid clogging the
> kernel log and distraction from real problems.
>
> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
> Acked-by: Maíra Canal <mcanal@igalia.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Alessandro Carminati <acarmina@redhat.com>
> Acked-by: David Gow <david@davidgow.net>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
I'm happy with this either with or without the extra check for the
warning count.
Acked-by: David Gow <david@davidgow.net>
We'll take this patch in the kunit tree along with the rest of the
series once everyone's happy.
Cheers,
-- David
> drivers/gpu/drm/tests/drm_rect_test.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_rect_test.c b/drivers/gpu/drm/tests/drm_rect_test.c
> index 17e1f34b76101..818e16e80c8f9 100644
> --- a/drivers/gpu/drm/tests/drm_rect_test.c
> +++ b/drivers/gpu/drm/tests/drm_rect_test.c
> @@ -409,8 +409,16 @@ static void drm_test_rect_calc_hscale(struct kunit *test)
> const struct drm_rect_scale_case *params = test->param_value;
> int scaling_factor;
>
> - scaling_factor = drm_rect_calc_hscale(¶ms->src, ¶ms->dst,
> - params->min_range, params->max_range);
> + /*
> + * drm_rect_calc_hscale() generates a warning backtrace whenever bad
> + * parameters are passed to it. This affects all unit tests with an
> + * error code in expected_scaling_factor.
> + */
> + kunit_warning_suppress(test) {
> + scaling_factor = drm_rect_calc_hscale(¶ms->src, ¶ms->dst,
> + params->min_range,
> + params->max_range);
> + }
>
> KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
> }
> @@ -420,8 +428,15 @@ static void drm_test_rect_calc_vscale(struct kunit *test)
> const struct drm_rect_scale_case *params = test->param_value;
> int scaling_factor;
>
> - scaling_factor = drm_rect_calc_vscale(¶ms->src, ¶ms->dst,
> - params->min_range, params->max_range);
> + /*
> + * drm_rect_calc_vscale() generates a warning backtrace whenever bad
> + * parameters are passed to it. This affects all unit tests with an
> + * error code in expected_scaling_factor.
> + */
> + kunit_warning_suppress(test) {
> + scaling_factor = drm_rect_calc_vscale(¶ms->src, ¶ms->dst,
> + params->min_range, params->max_range);
> + }
>
> KUNIT_EXPECT_EQ(test, scaling_factor, params->expected_scaling_factor);
> }
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox