* [PATCH v2 01/14] drbd: Open-code transfer log list walk
2026-06-09 6:13 [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Kaitao Cheng
@ 2026-06-09 6:13 ` Kaitao Cheng
2026-06-09 6:53 ` sashiko-bot
2026-06-09 6:13 ` [PATCH v2 02/14] firewire: core: Open-code topology " Kaitao Cheng
` (5 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:13 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. That is the desired behaviour for the
common case, but this transfer log walk temporarily drops
resource->req_lock and revalidates the cursor before continuing.
Keep the loop open-coded so the next request is derived after the body
has completed and after the cursor has been adjusted. This preserves the
existing traversal semantics and prepares the code for the list iterator
update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/block/drbd/drbd_debugfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/block/drbd/drbd_debugfs.c b/drivers/block/drbd/drbd_debugfs.c
index 12460b584bcb..e90cead90e9d 100644
--- a/drivers/block/drbd/drbd_debugfs.c
+++ b/drivers/block/drbd/drbd_debugfs.c
@@ -308,7 +308,9 @@ static void seq_print_resource_transfer_log_summary(struct seq_file *m,
seq_puts(m, "n\tdevice\tvnr\t" RQ_HDR);
spin_lock_irq(&resource->req_lock);
- list_for_each_entry(req, &connection->transfer_log, tl_requests) {
+ for (req = list_first_entry(&connection->transfer_log, typeof(*req), tl_requests);
+ !list_entry_is_head(req, &connection->transfer_log, tl_requests);
+ req = list_next_entry(req, tl_requests)) {
unsigned int tmp = 0;
unsigned int s;
++count;
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 01/14] drbd: Open-code transfer log list walk
2026-06-09 6:13 ` [PATCH v2 01/14] drbd: Open-code transfer log list walk Kaitao Cheng
@ 2026-06-09 6:53 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:53 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Dropping `req_lock` during transfer log iteration allows the current request to be moved to a different list, causing out-of-bounds reads and an infinite loop.
--
commit eaa3f881c5752fd80fb9f58149972d66629f2d5b
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
drbd: Open-code transfer log list walk
This commit open-codes the transfer log list walk in drbd_debugfs.c to
preserve the existing traversal semantics where the cursor is revalidated
after temporarily dropping a lock, in preparation for an upcoming change
to list_for_each_entry().
> diff --git a/drivers/block/drbd/drbd_debugfs.c b/drivers/block/drbd/drbd_debugfs.c
> index 12460b584bcb9..e90cead90e9db 100644
> --- a/drivers/block/drbd/drbd_debugfs.c
> +++ b/drivers/block/drbd/drbd_debugfs.c
> @@ -308,7 +308,9 @@ static void seq_print_resource_transfer_log_summary(struct seq_file *m,
>
> seq_puts(m, "n\tdevice\tvnr\t" RQ_HDR);
> spin_lock_irq(&resource->req_lock);
> - list_for_each_entry(req, &connection->transfer_log, tl_requests) {
> + for (req = list_first_entry(&connection->transfer_log, typeof(*req), tl_requests);
> + !list_entry_is_head(req, &connection->transfer_log, tl_requests);
> + req = list_next_entry(req, tl_requests)) {
[Severity: High]
This is a pre-existing issue, but can this loop run infinitely or read
out-of-bounds memory when resource->req_lock is dropped inside the loop body?
Later in seq_print_resource_transfer_log_summary(), the lock is dropped
to yield:
spin_unlock_irq(&resource->req_lock);
cond_resched();
spin_lock_irq(&resource->req_lock);
req_next = list_next_entry(req, tl_requests);
While the lock is dropped, if another thread calls drbd_restart_request() in
drivers/block/drbd/drbd_main.c, the current req can be moved from
connection->transfer_log to the global retry.writes list via list_move_tail():
drivers/block/drbd/drbd_main.c:drbd_restart_request() {
...
spin_lock_irqsave(&retry.lock, flags);
list_move_tail(&req->tl_requests, &retry.writes);
spin_unlock_irqrestore(&retry.lock, flags);
...
}
When the lock is reacquired, req remains valid because of the kref_get() call,
but list_next_entry() will now follow retry.writes instead of transfer_log.
Because the loop terminates by checking against &connection->transfer_log,
traversing retry.writes bypasses the break condition.
Furthermore, if do_retry() concurrently splices retry.writes onto a local stack
list, can this traversal continue into unmapped or overwritten stack memory?
> unsigned int tmp = 0;
> unsigned int s;
> ++count;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 02/14] firewire: core: Open-code topology list walk
2026-06-09 6:13 [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Kaitao Cheng
2026-06-09 6:13 ` [PATCH v2 01/14] drbd: Open-code transfer log list walk Kaitao Cheng
@ 2026-06-09 6:13 ` Kaitao Cheng
2026-06-09 6:53 ` sashiko-bot
2026-06-09 6:25 ` [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks Kaitao Cheng
` (4 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:13 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. for_each_fw_node() intentionally appends
newly discovered child nodes to the temporary walk list while the list is
being traversed.
Keep the loop open-coded so the next node is looked up only after
children have been appended. This preserves the current breadth-first
traversal semantics and prepares the code for the list iterator update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/firewire/core-topology.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index bb2d2db30795..df2ac0dab106 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -272,7 +272,9 @@ static void for_each_fw_node(struct fw_card *card, struct fw_node *root,
fw_node_get(root);
list_add_tail(&root->link, &list);
parent = NULL;
- list_for_each_entry(node, &list, link) {
+ for (node = list_first_entry(&list, typeof(*node), link);
+ !list_entry_is_head(node, &list, link);
+ node = list_next_entry(node, link)) {
node->color = card->color;
for (i = 0; i < node->port_count; i++) {
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 02/14] firewire: core: Open-code topology list walk
2026-06-09 6:13 ` [PATCH v2 02/14] firewire: core: Open-code topology " Kaitao Cheng
@ 2026-06-09 6:53 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:53 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Memory leak in `build_tree()` where dynamically allocated `fw_node` subtrees on the local `stack` list are leaked when returning `NULL` on error paths.
- [High] Unconditional dereference of `card->root_node` in `fw_core_handle_bus_reset()` leading to a kernel panic if `build_tree()` fails on the first bus reset.
--
commit 814723ea200ca545cf7dcf5b3f733df13cc3c3f2
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
firewire: core: Open-code topology list walk
This commit open-codes the topology list walk in for_each_fw_node() to
defer looking up the next node until after children are appended. It
preserves breadth-first traversal semantics while preparing the code
for a future list iterator update.
> diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
> index bb2d2db30795e..df2ac0dab1060 100644
> --- a/drivers/firewire/core-topology.c
> +++ b/drivers/firewire/core-topology.c
[ ... ]
drivers/firewire/core-topology.c:build_tree() {
...
if (child_port_count > stack_depth) {
fw_err(card, "topology stack underflow\n");
return NULL;
}
...
}
[Severity: High]
This is a pre-existing issue, but does this code leak the dynamically
allocated fw_node subtrees on the local stack list when returning NULL?
Since the stack list goes out of scope on this error path, it seems the
attached nodes are abandoned without being freed.
drivers/firewire/core-topology.c:fw_core_handle_bus_reset() {
...
scoped_guard(spinlock, &card->topology_map.lock) {
update_topology_map(card->topology_map.buffer, sizeof(card->topology_map.buffer),
card->root_node->node_id, self_ids, self_id_count);
}
...
}
[Severity: High]
This is a pre-existing issue, but can this sequence cause a panic?
If build_tree() fails and returns NULL during the first bus reset,
card->root_node would be left as NULL. Would passing
card->root_node->node_id unconditionally to update_topology_map() then
result in a NULL pointer dereference?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=2
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks
2026-06-09 6:13 [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Kaitao Cheng
2026-06-09 6:13 ` [PATCH v2 01/14] drbd: Open-code transfer log list walk Kaitao Cheng
2026-06-09 6:13 ` [PATCH v2 02/14] firewire: core: Open-code topology " Kaitao Cheng
@ 2026-06-09 6:25 ` Kaitao Cheng
2026-06-09 6:25 ` [PATCH v2 04/14] drm/i915/gt: Open-code active timeline walk Kaitao Cheng
` (5 more replies)
2026-06-09 6:38 ` [PATCH v2 10/14] spi: tegra210-quad: " Kaitao Cheng
` (3 subsequent siblings)
6 siblings, 6 replies; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:25 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry_from() and
list_for_each_entry_reverse() cache the next or previous element before
entering the loop body. The bridge enable and disable ordering code
adjusts its cursor to skip ranges that have already been handled.
Keep those walks open-coded so the loop step observes the cursor
selected by the body. This preserves the existing bridge ordering
semantics and prepares the code for the list iterator update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/gpu/drm/drm_bridge.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index d6f512b73389..a538aabc4e0b 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -868,7 +868,8 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
encoder = bridge->encoder;
- list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) {
+ for (; !list_entry_is_head(bridge, &encoder->bridge_chain, chain_node);
+ bridge = list_next_entry(bridge, chain_node)) {
limit = NULL;
if (!list_is_last(&bridge->chain_node, &encoder->bridge_chain)) {
@@ -962,7 +963,9 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge,
encoder = bridge->encoder;
- list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
+ for (iter = list_last_entry(&encoder->bridge_chain, typeof(*iter), chain_node);
+ !list_entry_is_head(iter, &encoder->bridge_chain, chain_node);
+ iter = list_prev_entry(iter, chain_node)) {
if (iter->pre_enable_prev_first) {
next = iter;
limit = bridge;
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 04/14] drm/i915/gt: Open-code active timeline walk
2026-06-09 6:25 ` [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks Kaitao Cheng
@ 2026-06-09 6:25 ` Kaitao Cheng
2026-06-09 7:00 ` Andy Shevchenko
2026-06-09 6:25 ` [PATCH v2 05/14] drm/i915: Open-code DFS dependency list walk Kaitao Cheng
` (4 subsequent siblings)
5 siblings, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:25 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. __intel_gt_unset_wedged() drops
timelines->lock while waiting on a fence and then restarts the walk from
the list head after the lock is reacquired.
Keep the loop open-coded so the next timeline is selected after that
restart logic has run. This preserves the existing lock-drop traversal
semantics and prepares the code for the list iterator update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/gpu/drm/i915/gt/intel_reset.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index adff482a6c9c..fe0d87e248a7 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1077,7 +1077,9 @@ static bool __intel_gt_unset_wedged(struct intel_gt *gt)
* No more can be submitted until we reset the wedged bit.
*/
spin_lock(&timelines->lock);
- list_for_each_entry(tl, &timelines->active_list, link) {
+ for (tl = list_first_entry(&timelines->active_list, typeof(*tl), link);
+ !list_entry_is_head(tl, &timelines->active_list, link);
+ tl = list_next_entry(tl, link)) {
struct dma_fence *fence;
fence = i915_active_fence_get(&tl->last_request);
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 04/14] drm/i915/gt: Open-code active timeline walk
2026-06-09 6:25 ` [PATCH v2 04/14] drm/i915/gt: Open-code active timeline walk Kaitao Cheng
@ 2026-06-09 7:00 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2026-06-09 7:00 UTC (permalink / raw)
To: Kaitao Cheng
Cc: Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Matthew Auld, Matthew Brost, Waiman Long,
drbd-dev, linux-block, linux1394-devel, dri-devel, intel-gfx,
linux-spi, linux-stm32, linux-arm-kernel, linux-tegra,
linux-sound, linux-kernel, Andrew Morton, Randy Dunlap,
Christian Brauner, David Howells, Luca Ceresoli, Kaitao Cheng
On Tue, Jun 09, 2026 at 02:25:16PM +0800, Kaitao Cheng wrote:
>
> A later change will make list_for_each_entry() cache the next element
> before entering the loop body. __intel_gt_unset_wedged() drops
> timelines->lock while waiting on a fence and then restarts the walk from
> the list head after the lock is reacquired.
>
> Keep the loop open-coded so the next timeline is selected after that
> restart logic has run. This preserves the existing lock-drop traversal
> semantics and prepares the code for the list iterator update.
...
> spin_lock(&timelines->lock);
> - list_for_each_entry(tl, &timelines->active_list, link) {
> + for (tl = list_first_entry(&timelines->active_list, typeof(*tl), link);
> + !list_entry_is_head(tl, &timelines->active_list, link);
> + tl = list_next_entry(tl, link)) {
Yeah, these cases should rather be converted to do {} while or while-loop.
This will make the intention clearer and reduces the possibility that someone
mistakenly changes these back to use list_for_each_entry().
See, for example, deferred_probe_work_func() implementation.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 05/14] drm/i915: Open-code DFS dependency list walk
2026-06-09 6:25 ` [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks Kaitao Cheng
2026-06-09 6:25 ` [PATCH v2 04/14] drm/i915/gt: Open-code active timeline walk Kaitao Cheng
@ 2026-06-09 6:25 ` Kaitao Cheng
2026-06-09 6:25 ` [PATCH v2 06/14] drm/ttm: Open-code reservation " Kaitao Cheng
` (3 subsequent siblings)
5 siblings, 0 replies; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:25 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. __i915_schedule() builds its DFS work list
while walking it by moving newly discovered dependencies to the tail.
Keep the DFS walk open-coded so the next dependency is resolved after any
tail moves performed by the body. This preserves the existing traversal
semantics and prepares the code for the list iterator update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/gpu/drm/i915/i915_scheduler.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index aec1342402ca..da1f60282df8 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -190,7 +190,9 @@ static void __i915_schedule(struct i915_sched_node *node,
* end result is a topological list of requests in reverse order, the
* last element in the list is the request we must execute first.
*/
- list_for_each_entry(dep, &dfs, dfs_link) {
+ for (dep = list_first_entry(&dfs, typeof(*dep), dfs_link);
+ !list_entry_is_head(dep, &dfs, dfs_link);
+ dep = list_next_entry(dep, dfs_link)) {
struct i915_sched_node *node = dep->signaler;
/* If we are already flying, we know we have no signalers */
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 06/14] drm/ttm: Open-code reservation list walk
2026-06-09 6:25 ` [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks Kaitao Cheng
2026-06-09 6:25 ` [PATCH v2 04/14] drm/i915/gt: Open-code active timeline walk Kaitao Cheng
2026-06-09 6:25 ` [PATCH v2 05/14] drm/i915: Open-code DFS dependency list walk Kaitao Cheng
@ 2026-06-09 6:25 ` Kaitao Cheng
2026-06-09 6:51 ` sashiko-bot
2026-06-09 6:25 ` [PATCH v2 07/14] spi: fsi: Open-code message transfer walk Kaitao Cheng
` (2 subsequent siblings)
5 siblings, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:25 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. ttm_eu_reserve_buffers() may move the
current validation buffer to the duplicates list and then rewinds the
cursor before continuing.
Keep the reservation walk open-coded so the loop step uses the cursor
selected by that duplicate handling. This preserves the existing
traversal semantics and prepares the code for the list iterator update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/gpu/drm/ttm/ttm_execbuf_util.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
index bc7a83a9fe44..8072f07d5557 100644
--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -86,7 +86,9 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
if (ticket)
ww_acquire_init(ticket, &reservation_ww_class);
- list_for_each_entry(entry, list, head) {
+ for (entry = list_first_entry(list, typeof(*entry), head);
+ !list_entry_is_head(entry, list, head);
+ entry = list_next_entry(entry, head)) {
struct ttm_buffer_object *bo = entry->bo;
unsigned int num_fences;
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 06/14] drm/ttm: Open-code reservation list walk
2026-06-09 6:25 ` [PATCH v2 06/14] drm/ttm: Open-code reservation " Kaitao Cheng
@ 2026-06-09 6:51 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:51 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing lock release when `dma_resv_reserve_fences()` fails, leaving the current buffer object permanently locked.
--
commit 4a941c2e22e18c4c60dbeb3162fc85117e45e7a8
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
drm/ttm: Open-code reservation list walk
This patch modifies ttm_eu_reserve_buffers() to open-code the
list_for_each_entry() loop. This prepares the code for future list iterator
updates while preserving the existing traversal semantics where the cursor
can be rewound during duplicate handling.
> diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> index bc7a83a9fe443..8072f07d55579 100644
> --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> @@ -86,7 +86,9 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
> if (ticket)
> ww_acquire_init(ticket, &reservation_ww_class);
>
> - list_for_each_entry(entry, list, head) {
> + for (entry = list_first_entry(list, typeof(*entry), head);
> + !list_entry_is_head(entry, list, head);
> + entry = list_next_entry(entry, head)) {
[Severity: High]
This is a pre-existing issue, but does the error path in this loop leak
the lock on entry->bo when dma_resv_reserve_fences() fails?
If ttm_bo_reserve() successfully acquires the lock on the current entry->bo,
and then dma_resv_reserve_fences() fails (for example, returning -ENOMEM),
the code cleans up using ttm_eu_backoff_reservation_reverse():
ttm_eu_reserve_buffers() {
...
if (!ret) {
ret = dma_resv_reserve_fences(bo->base.resv, num_fences);
if (!ret)
continue;
}
/* uh oh, we lost out, drop every reservation and try... */
ttm_eu_backoff_reservation_reverse(list, entry);
...
}
However, ttm_eu_backoff_reservation_reverse() explicitly skips unlocking the
current entry itself because it uses list_for_each_entry_continue_reverse().
The code then bypasses the slowpath because ret is -ENOMEM (not -EDEADLK)
and returns without unlocking entry->bo:
ttm_eu_reserve_buffers() {
...
if (ret == -EDEADLK) {
ret = ttm_bo_reserve_slowpath(bo, intr, ticket);
}
...
if (unlikely(ret != 0)) {
if (ticket) {
ww_acquire_done(ticket);
ww_acquire_fini(ticket);
}
return ret;
}
...
}
Could this sequence leave the ww_mutex for entry->bo permanently locked?
> struct ttm_buffer_object *bo = entry->bo;
> unsigned int num_fences;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=6
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 07/14] spi: fsi: Open-code message transfer walk
2026-06-09 6:25 ` [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks Kaitao Cheng
` (2 preceding siblings ...)
2026-06-09 6:25 ` [PATCH v2 06/14] drm/ttm: Open-code reservation " Kaitao Cheng
@ 2026-06-09 6:25 ` Kaitao Cheng
2026-06-09 7:02 ` Andy Shevchenko
2026-06-09 6:25 ` [PATCH v2 08/14] spi: stm32-ospi: " Kaitao Cheng
2026-06-09 6:25 ` [PATCH v2 09/14] spi: stm32-qspi: " Kaitao Cheng
5 siblings, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:25 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. fsi_spi_transfer_one_message() can combine
the current transfer with the following transfer and then advance the
cursor to that consumed entry.
Keep the transfer walk open-coded so the loop step observes that cursor
update and skips the consumed transfer. This preserves the existing
message sequencing semantics and prepares the code for the list iterator
update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/spi/spi-fsi.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c
index f6a75f0184c4..44999f00f5f6 100644
--- a/drivers/spi/spi-fsi.c
+++ b/drivers/spi/spi-fsi.c
@@ -434,7 +434,10 @@ static int fsi_spi_transfer_one_message(struct spi_controller *ctlr,
if (rc)
goto error;
- list_for_each_entry(transfer, &mesg->transfers, transfer_list) {
+ for (transfer = list_first_entry(&mesg->transfers,
+ typeof(*transfer), transfer_list);
+ !list_entry_is_head(transfer, &mesg->transfers, transfer_list);
+ transfer = list_next_entry(transfer, transfer_list)) {
struct fsi_spi_sequence seq;
struct spi_transfer *next = NULL;
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 07/14] spi: fsi: Open-code message transfer walk
2026-06-09 6:25 ` [PATCH v2 07/14] spi: fsi: Open-code message transfer walk Kaitao Cheng
@ 2026-06-09 7:02 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2026-06-09 7:02 UTC (permalink / raw)
To: Kaitao Cheng
Cc: Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Matthew Auld, Matthew Brost, Waiman Long,
drbd-dev, linux-block, linux1394-devel, dri-devel, intel-gfx,
linux-spi, linux-stm32, linux-arm-kernel, linux-tegra,
linux-sound, linux-kernel, Andrew Morton, Randy Dunlap,
Christian Brauner, David Howells, Luca Ceresoli, Kaitao Cheng
On Tue, Jun 09, 2026 at 02:25:19PM +0800, Kaitao Cheng wrote:
>
> A later change will make list_for_each_entry() cache the next element
> before entering the loop body. fsi_spi_transfer_one_message() can combine
> the current transfer with the following transfer and then advance the
> cursor to that consumed entry.
>
> Keep the transfer walk open-coded so the loop step observes that cursor
> update and skips the consumed transfer. This preserves the existing
> message sequencing semantics and prepares the code for the list iterator
> update.
...
> - list_for_each_entry(transfer, &mesg->transfers, transfer_list) {
> + for (transfer = list_first_entry(&mesg->transfers,
> + typeof(*transfer), transfer_list);
You can keep this on a single line for more logical split.
for (transfer = list_first_entry(&mesg->transfers, typeof(*transfer), transfer_list);
it's under relaxed limits for the line length.
> + !list_entry_is_head(transfer, &mesg->transfers, transfer_list);
> + transfer = list_next_entry(transfer, transfer_list)) {
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 08/14] spi: stm32-ospi: Open-code message transfer walk
2026-06-09 6:25 ` [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks Kaitao Cheng
` (3 preceding siblings ...)
2026-06-09 6:25 ` [PATCH v2 07/14] spi: fsi: Open-code message transfer walk Kaitao Cheng
@ 2026-06-09 6:25 ` Kaitao Cheng
2026-06-09 6:57 ` sashiko-bot
2026-06-09 6:25 ` [PATCH v2 09/14] spi: stm32-qspi: " Kaitao Cheng
5 siblings, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:25 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. stm32_ospi_transfer_one_message() can
consume the following transfer as part of the current operation and then
advance the loop cursor to that entry.
Keep the transfer walk open-coded so the loop step observes that cursor
update and skips the consumed transfer. This preserves the existing
message sequencing semantics and prepares the code for the list iterator
update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/spi/spi-stm32-ospi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-stm32-ospi.c b/drivers/spi/spi-stm32-ospi.c
index 4461c6e24b9e..4dc2b56b4c20 100644
--- a/drivers/spi/spi-stm32-ospi.c
+++ b/drivers/spi/spi-stm32-ospi.c
@@ -675,7 +675,9 @@ static int stm32_ospi_transfer_one_message(struct spi_controller *ctrl,
gpiod_set_value_cansleep(cs_gpiod, true);
- list_for_each_entry(transfer, &msg->transfers, transfer_list) {
+ for (transfer = list_first_entry(&msg->transfers, typeof(*transfer), transfer_list);
+ !list_entry_is_head(transfer, &msg->transfers, transfer_list);
+ transfer = list_next_entry(transfer, transfer_list)) {
u8 dummy_bytes = 0;
memset(&op, 0, sizeof(op));
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 08/14] spi: stm32-ospi: Open-code message transfer walk
2026-06-09 6:25 ` [PATCH v2 08/14] spi: stm32-ospi: " Kaitao Cheng
@ 2026-06-09 6:57 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:57 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] A valid SPI message containing a transfer with length > 0 but both `tx_buf` and `rx_buf` set to NULL will cause a kernel crash when the driver dereferences the NULL buffer during FIFO writes.
- [High] Early error returns in `stm32_ospi_transfer_one_message` fail to finalize the SPI message, resulting in a permanent hang for callers.
--
commit 77e013496da647c31add0f5085a07d31c7c250b2
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
spi: stm32-ospi: Open-code message transfer walk
This commit modifies stm32_ospi_transfer_one_message() to open-code the
list iteration over message transfers. It prepares the loop to observe
cursor updates explicitly, anticipating a later list iterator update.
> diff --git a/drivers/spi/spi-stm32-ospi.c b/drivers/spi/spi-stm32-ospi.c
> index 3757f6ba8fc61..fe92bb06251be 100644
> --- a/drivers/spi/spi-stm32-ospi.c
> +++ b/drivers/spi/spi-stm32-ospi.c
> @@ -674,7 +674,9 @@ static int stm32_ospi_transfer_one_message(struct spi_controller *ctrl,
[Severity: High]
This is a pre-existing issue, but do the early error returns in
stm32_ospi_transfer_one_message() fail to finalize the SPI message?
If cs_gpiod is null or pm_runtime_resume_and_get() fails, the function
returns early:
if (!cs_gpiod)
return -EOPNOTSUPP;
ret = pm_runtime_resume_and_get(ospi->dev);
if (ret < 0)
return ret;
Since these paths bypass spi_finalize_current_message(), does this leave
the SPI core waiting indefinitely and cause a deadlock for callers?
[Severity: High]
This is also a pre-existing issue, but can this driver crash if an SPI
message contains a dummy transfer with both tx_buf and rx_buf set to NULL?
The SPI core allows dummy transfers with both buffers set to NULL. If a
transfer reaches stm32_ospi_transfer_one_message() without the dummy_data
flag set, the driver defaults to a write operation and assigns the NULL
tx_buf to the operation:
} else {
ospi->fmode = CR_FMODE_INDW;
op.data.buswidth = transfer->tx_nbits;
op.data.dir = SPI_MEM_DATA_OUT;
op.data.buf.out = transfer->tx_buf;
}
This eventually leads to stm32_ospi_send() and stm32_ospi_xfer(). Because
DMA mapping of a NULL pointer fails, it falls back to PIO mode by calling
stm32_ospi_poll().
Inside stm32_ospi_poll(), the stm32_ospi_write_fifo() callback is invoked,
which dereferences the NULL pointer directly:
switch (len) {
case sizeof(u32):
writel_relaxed(*((u32 *)val), addr);
break;
Could this trigger a kernel panic and allow a local denial of service?
>
> gpiod_set_value_cansleep(cs_gpiod, true);
>
> - list_for_each_entry(transfer, &msg->transfers, transfer_list) {
> + for (transfer = list_first_entry(&msg->transfers, typeof(*transfer), transfer_list);
> + !list_entry_is_head(transfer, &msg->transfers, transfer_list);
> + transfer = list_next_entry(transfer, transfer_list)) {
> u8 dummy_bytes = 0;
>
> memset(&op, 0, sizeof(op));
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=8
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 09/14] spi: stm32-qspi: Open-code message transfer walk
2026-06-09 6:25 ` [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks Kaitao Cheng
` (4 preceding siblings ...)
2026-06-09 6:25 ` [PATCH v2 08/14] spi: stm32-ospi: " Kaitao Cheng
@ 2026-06-09 6:25 ` Kaitao Cheng
2026-06-09 6:55 ` sashiko-bot
5 siblings, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:25 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. stm32_qspi_transfer_one_message() can
consume the following transfer as part of the current operation and then
advance the loop cursor to that entry.
Keep the transfer walk open-coded so the loop step observes that cursor
update and skips the consumed transfer. This preserves the existing
message sequencing semantics and prepares the code for the list iterator
update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/spi/spi-stm32-qspi.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c
index df1bbacec90a..27d82a578c9f 100644
--- a/drivers/spi/spi-stm32-qspi.c
+++ b/drivers/spi/spi-stm32-qspi.c
@@ -577,7 +577,10 @@ static int stm32_qspi_transfer_one_message(struct spi_controller *ctrl,
gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), true);
- list_for_each_entry(transfer, &msg->transfers, transfer_list) {
+ for (transfer = list_first_entry(&msg->transfers,
+ typeof(*transfer), transfer_list);
+ !list_entry_is_head(transfer, &msg->transfers, transfer_list);
+ transfer = list_next_entry(transfer, transfer_list)) {
u8 dummy_bytes = 0;
memset(&op, 0, sizeof(op));
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 09/14] spi: stm32-qspi: Open-code message transfer walk
2026-06-09 6:25 ` [PATCH v2 09/14] spi: stm32-qspi: " Kaitao Cheng
@ 2026-06-09 6:55 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:55 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Early returns in `stm32_qspi_transfer_one_message` fail to call `spi_finalize_current_message()`, leaking the SPI message and hanging the caller.
- [High] The driver mishandles SPI transfers where both `tx_buf` and `rx_buf` are NULL but `len > 0`, causing a NULL pointer dereference.
--
commit 0feb4c5cf513d6c2af724ffe0a636c0f0b92a880
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
spi: stm32-qspi: Open-code message transfer walk
A later change will make list_for_each_entry() cache the next element
before entering the loop body. stm32_qspi_transfer_one_message() can
consume the following transfer as part of the current operation and then
advance the loop cursor to that entry.
Keep the transfer walk open-coded so the loop step observes that cursor
update and skips the consumed transfer. This preserves the existing
message sequencing semantics and prepares the code for the list iterator
update.
> diff --git a/drivers/spi/spi-stm32-qspi.c b/drivers/spi/spi-stm32-qspi.c
> index ea69fe25686f3..e264fee2bd2d7 100644
> --- a/drivers/spi/spi-stm32-qspi.c
> +++ b/drivers/spi/spi-stm32-qspi.c
> @@ -576,7 +576,10 @@ static int stm32_qspi_transfer_one_message(struct spi_controller *ctrl,
>
> gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), true);
>
> - list_for_each_entry(transfer, &msg->transfers, transfer_list) {
> + for (transfer = list_first_entry(&msg->transfers,
> + typeof(*transfer), transfer_list);
> + !list_entry_is_head(transfer, &msg->transfers, transfer_list);
> + transfer = list_next_entry(transfer, transfer_list)) {
> u8 dummy_bytes = 0;
>
> memset(&op, 0, sizeof(op));
[Severity: High]
This is a pre-existing issue, but I noticed early returns in
stm32_qspi_transfer_one_message() might fail to call
spi_finalize_current_message() and leak the SPI message:
if (!spi_get_csgpiod(spi, 0))
return -EOPNOTSUPP;
ret = pm_runtime_resume_and_get(qspi->dev);
if (ret < 0)
return ret;
Could these early returns cause spi_sync() to hang indefinitely on
wait_for_completion() and permanently lock the SPI bus? Should we arrange for
spi_finalize_current_message() to be called in these error paths?
[Severity: High]
This isn't a bug introduced by this patch, but there might be an issue with
how the driver handles SPI transfers where both tx_buf and rx_buf are NULL
but len > 0. In stm32_qspi_transfer_one_message():
} else {
qspi->fmode = CCR_FMODE_INDW;
op.data.buswidth = transfer->tx_nbits;
op.data.dir = SPI_MEM_DATA_OUT;
op.data.buf.out = transfer->tx_buf;
}
If a local user sends a message via spidev with tx_buf = 0, rx_buf = 0, and
len > 0, the driver assumes it is a transmit operation and assigns
op.data.buf.out to NULL.
Does the operation then fall back to stm32_qspi_tx_poll() which loops over
len, and since the transmit FIFO is empty, immediately satisfies the FIFO
Threshold Flag?
If so, could this result in dereferencing the NULL buf pointer in
stm32_qspi_tx_poll():
fifo(buf, qspi->io_base + QSPI_DR, step);
len -= step;
buf += step;
and triggering a kernel panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=9
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 10/14] spi: tegra210-quad: Open-code message transfer walk
2026-06-09 6:13 [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Kaitao Cheng
` (2 preceding siblings ...)
2026-06-09 6:25 ` [PATCH v2 03/14] drm/bridge: Open-code bridge chain list walks Kaitao Cheng
@ 2026-06-09 6:38 ` Kaitao Cheng
2026-06-09 6:38 ` [PATCH v2 11/14] locking/locktorture: Open-code ww mutex list walk Kaitao Cheng
` (2 more replies)
2026-06-09 6:41 ` [PATCH v2 13/14] ASoC: dapm: Open-code widget invalidation walk Kaitao Cheng
` (2 subsequent siblings)
6 siblings, 3 replies; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:38 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. tegra_qspi_non_combined_seq_xfer() can
consume the following transfer as part of the current operation and then
advance the loop cursor to that entry.
Keep the transfer walk open-coded so the loop step observes that cursor
update and skips the consumed transfer. This preserves the existing
message sequencing semantics and prepares the code for the list iterator
update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
drivers/spi/spi-tegra210-quad.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
index db28dd556484..42dd5cf53c67 100644
--- a/drivers/spi/spi-tegra210-quad.c
+++ b/drivers/spi/spi-tegra210-quad.c
@@ -1302,7 +1302,9 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
if (tqspi->soc_data->supports_tpm)
val &= ~QSPI_TPM_WAIT_POLL_EN;
tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
- list_for_each_entry(transfer, &msg->transfers, transfer_list) {
+ for (transfer = list_first_entry(&msg->transfers, typeof(*transfer), transfer_list);
+ !list_entry_is_head(transfer, &msg->transfers, transfer_list);
+ transfer = list_next_entry(transfer, transfer_list)) {
struct spi_transfer *xfer = transfer;
u8 dummy_bytes = 0;
u32 cmd1;
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 11/14] locking/locktorture: Open-code ww mutex list walk
2026-06-09 6:38 ` [PATCH v2 10/14] spi: tegra210-quad: " Kaitao Cheng
@ 2026-06-09 6:38 ` Kaitao Cheng
2026-06-09 6:54 ` sashiko-bot
2026-06-09 6:38 ` [PATCH v2 12/14] locking/ww_mutex: Open-code stress reorder " Kaitao Cheng
2026-06-09 6:57 ` [PATCH v2 10/14] spi: tegra210-quad: Open-code message transfer walk sashiko-bot
2 siblings, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:38 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. The ww-mutex torture path can move list
entries while it resolves a wound/wait conflict and then continue from
the adjusted cursor.
Keep the list walk open-coded so the loop step observes the cursor
selected by the body. This preserves the existing stress-test traversal
semantics and prepares the code for the list iterator update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
kernel/locking/locktorture.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index e618bcf75e2d..0eb75e9bccaa 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -644,7 +644,9 @@ __acquires(torture_ww_mutex_2)
ww_acquire_init(ctx, &torture_ww_class);
- list_for_each_entry(ll, &list, link) {
+ for (ll = list_first_entry(&list, typeof(*ll), link);
+ !list_entry_is_head(ll, &list, link);
+ ll = list_next_entry(ll, link)) {
int err;
err = ww_mutex_lock(ll->lock, ctx);
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 11/14] locking/locktorture: Open-code ww mutex list walk
2026-06-09 6:38 ` [PATCH v2 11/14] locking/locktorture: Open-code ww mutex list walk Kaitao Cheng
@ 2026-06-09 6:54 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:54 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The WW mutex documentation promotes an idiomatic pattern that relies on `list_for_each_entry()` not caching the next element, which is broken by the upcoming list iterator update.
--
commit 803c4bd20ba4e9475a66f9600c81114e4079f384
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
locking/locktorture: Open-code ww mutex list walk
This commit open-codes the ww mutex list walk in locktorture. It prepares
the code for an upcoming change that makes list_for_each_entry() cache the
next element before entering the loop body, which preserves the existing
dynamic traversal semantics.
> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> index e618bcf75e2d8..0eb75e9bccaa6 100644
> --- a/kernel/locking/locktorture.c
> +++ b/kernel/locking/locktorture.c
> @@ -644,7 +644,9 @@ __acquires(torture_ww_mutex_2)
>
> ww_acquire_init(ctx, &torture_ww_class);
>
> - list_for_each_entry(ll, &list, link) {
> + for (ll = list_first_entry(&list, typeof(*ll), link);
> + !list_entry_is_head(ll, &list, link);
> + ll = list_next_entry(ll, link)) {
> int err;
>
> err = ww_mutex_lock(ll->lock, ctx);
[Severity: Medium]
Since this patch updates locktorture to avoid regressions in the ww-mutex
list walk under the new list iterator semantics, does
Documentation/locking/ww-mutex-design.rst also need to be updated?
The documentation for "Method 2" explicitly encourages using
list_for_each_entry() for dynamic list reordering:
Documentation/locking/ww-mutex-design.rst:lock_objs() {
...
/* we lost out in a seqno race, lock and retry.. */
ww_mutex_lock_slow(&entry->obj->lock, ctx);
/*
* Move buf to head of the list, this will point
* buf->next to the first unlocked entry,
* restarting the for loop.
*/
list_del(&entry->head);
list_add(&entry->head, list);
...
}
If a developer follows this documented pattern with the new list iterator
semantics, moving the list entry to the head of the list will no longer
restart the loop. Instead, the loop will continue from the cached next
element, skipping over unlocked entries and silently failing to acquire
all required locks. Could leaving this unchanged cause regressions in
drivers that copy this pattern?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=11
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 12/14] locking/ww_mutex: Open-code stress reorder list walk
2026-06-09 6:38 ` [PATCH v2 10/14] spi: tegra210-quad: " Kaitao Cheng
2026-06-09 6:38 ` [PATCH v2 11/14] locking/locktorture: Open-code ww mutex list walk Kaitao Cheng
@ 2026-06-09 6:38 ` Kaitao Cheng
2026-06-09 6:57 ` [PATCH v2 10/14] spi: tegra210-quad: Open-code message transfer walk sashiko-bot
2 siblings, 0 replies; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:38 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. stress_reorder_work() can move list
entries while handling wound/wait locking conflicts and then continue
from the adjusted cursor.
Keep the list walk open-coded so the loop step observes the cursor
selected by the body. This preserves the existing stress-test traversal
semantics and prepares the code for the list iterator update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
kernel/locking/test-ww_mutex.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c
index 838d631544ed..08a6ab5ac041 100644
--- a/kernel/locking/test-ww_mutex.c
+++ b/kernel/locking/test-ww_mutex.c
@@ -519,7 +519,9 @@ static void stress_reorder_work(struct work_struct *work)
do {
ww_acquire_init(&ctx, stress->class);
- list_for_each_entry(ll, &locks, link) {
+ for (ll = list_first_entry(&locks, typeof(*ll), link);
+ !list_entry_is_head(ll, &locks, link);
+ ll = list_next_entry(ll, link)) {
err = ww_mutex_lock(ll->lock, &ctx);
if (!err)
continue;
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 10/14] spi: tegra210-quad: Open-code message transfer walk
2026-06-09 6:38 ` [PATCH v2 10/14] spi: tegra210-quad: " Kaitao Cheng
2026-06-09 6:38 ` [PATCH v2 11/14] locking/locktorture: Open-code ww mutex list walk Kaitao Cheng
2026-06-09 6:38 ` [PATCH v2 12/14] locking/ww_mutex: Open-code stress reorder " Kaitao Cheng
@ 2026-06-09 6:57 ` sashiko-bot
2 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:57 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Hardware Chip Select (CS) is left asserted indefinitely if the final message transfer is absorbed as a dummy transfer.
- [High] DMA memory mapping leak on error paths in `tegra_qspi_start_dma_based_transfer`.
- [High] DMA memory mapping leak upon transfer timeout or hardware error.
- [High] DMA maps memory beyond the end of the SPI transfer buffer when using packed DMA transfers.
- [High] Unconditional DMA unmap on error path in `tegra_qspi_dma_map_xfer`.
--
commit dedc4ce6598360fa2a4de3d4413aaecfda3051a4
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
spi: tegra210-quad: Open-code message transfer walk
This commit converts the list_for_each_entry() loop in
tegra_qspi_non_combined_seq_xfer() to an open-coded for-loop. This
preserves semantics when consuming the next transfer in the loop body,
preparing the code for future list iterator updates.
> diff --git a/drivers/spi/spi-tegra210-quad.c b/drivers/spi/spi-tegra210-quad.c
> index 588a929a97850..102562893d227 100644
> --- a/drivers/spi/spi-tegra210-quad.c
> +++ b/drivers/spi/spi-tegra210-quad.c
> @@ -1309,7 +1309,9 @@ static int tegra_qspi_non_combined_seq_xfer(struct tegra_qspi *tqspi,
> if (tqspi->soc_data->supports_tpm)
> val &= ~QSPI_TPM_WAIT_POLL_EN;
> tegra_qspi_writel(tqspi, val, QSPI_GLOBAL_CONFIG);
> - list_for_each_entry(transfer, &msg->transfers, transfer_list) {
> + for (transfer = list_first_entry(&msg->transfers, typeof(*transfer), transfer_list);
> + !list_entry_is_head(transfer, &msg->transfers, transfer_list);
> + transfer = list_next_entry(transfer, transfer_list)) {
> struct spi_transfer *xfer = transfer;
> u8 dummy_bytes = 0;
> u32 cmd1;
[Severity: High]
This is a pre-existing issue, but does this logic fail to deactivate the
chip select if the final transfer is absorbed as a dummy transfer?
In tegra_qspi_non_combined_seq_xfer(), if the final SPI transfer in the
message is a dummy transfer, the loop pointer advances to it, effectively
consuming it into the current xfer:
if (next_xfer->dummy_data) {
...
transfer = next_xfer;
}
When checking if CS should be deactivated at the end of the loop body,
the code uses xfer instead of the updated transfer pointer:
if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
if (!xfer->cs_change) {
tegra_qspi_transfer_end(spi);
...
}
} else if (xfer->cs_change) {
...
Since xfer is the second-to-last transfer, list_is_last() evaluates to
false. The code falls through to the else branch, and if cs_change is
false, tegra_qspi_transfer_end() is skipped.
The loop then terminates, leaving the hardware chip select asserted
indefinitely. Could this corrupt subsequent bus communications?
[Severity: High]
This is a pre-existing issue, but do we leak DMA memory mappings if a
transfer times out or encounters a hardware error?
Looking at tegra_qspi_non_combined_seq_xfer(), if the transfer times out,
tegra_qspi_handle_timeout() confirms the timeout, stops the DMA engine
channels, and jumps to complete_xfer:
if (tqspi->is_curr_dma_xfer)
tegra_qspi_dma_stop(tqspi);
...
goto complete_xfer;
Similarly, if a hardware error is detected, we also jump to complete_xfer:
if (tqspi->tx_status || tqspi->rx_status) {
...
goto complete_xfer;
}
In both paths, the code exits without calling tegra_qspi_dma_unmap_xfer().
Are these DMA mappings permanently leaked?
[Severity: High]
This isn't a bug introduced by this patch, but does
tegra_qspi_start_dma_based_transfer() leak DMA memory mappings on its
error paths?
When tqspi->is_packed is true, tegra_qspi_dma_map_xfer() successfully
maps the TX and RX DMA buffers.
If any subsequent DMA setup operations fail (like dmaengine_slave_config(),
tegra_qspi_start_tx_dma(), or tegra_qspi_start_rx_dma()), the function
returns the error directly:
ret = dmaengine_slave_config(tqspi->rx_dma_chan, &dma_sconfig);
if (ret < 0) {
dev_err(tqspi->dev, "failed DMA slave config: %d\n", ret);
return ret;
}
The caller tegra_qspi_non_combined_seq_xfer() then propagates the error
and exits, never calling tegra_qspi_dma_unmap_xfer().
Does this permanently leak the DMA mapped memory and IOMMU space?
[Severity: High]
This is a pre-existing issue, but does the DMA mapping calculation risk
crossing an unmapped physical page boundary?
In tegra_qspi_dma_map_xfer(), when tqspi->is_packed is true, the mapped
length is artificially inflated to a multiple of 4:
len = DIV_ROUND_UP(tqspi->curr_dma_words * tqspi->bytes_per_word, 4) * 4;
if (t->tx_buf) {
t->tx_dma = dma_map_single(tqspi->tx_dma_dev, (void *)tx_buf,
len, DMA_TO_DEVICE);
Because the original client-provided buffer might be exactly t->len bytes
long, mapping beyond its exact length could cross into restricted or
unallocated physical memory.
Could this trigger an IOMMU fault or a kernel panic?
[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
tegra_qspi_dma_map_xfer() unconditionally unmap the TX buffer even when
it was never mapped?
If t->tx_buf is NULL during a read-only transfer, the TX buffer mapping
is correctly skipped.
However, if the subsequent dma_map_single() for t->rx_buf fails, the
error handling block unmaps tx_dma without checking:
if (dma_mapping_error(tqspi->rx_dma_dev, t->rx_dma)) {
dma_unmap_single(tqspi->tx_dma_dev, t->tx_dma, len, DMA_TO_DEVICE);
return -ENOMEM;
}
Since t->tx_buf was NULL, t->tx_dma was never mapped by this function.
Could unmapping it here violate the DMA API contract, potentially
unmapping page 0 from the IOMMU or corrupting DMA tracking state?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=10
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 13/14] ASoC: dapm: Open-code widget invalidation walk
2026-06-09 6:13 [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Kaitao Cheng
` (3 preceding siblings ...)
2026-06-09 6:38 ` [PATCH v2 10/14] spi: tegra210-quad: " Kaitao Cheng
@ 2026-06-09 6:41 ` Kaitao Cheng
2026-06-09 6:41 ` [PATCH v2 14/14] list: Cache cursors in entry iterators Kaitao Cheng
2026-06-09 6:55 ` [PATCH v2 13/14] ASoC: dapm: Open-code widget invalidation walk sashiko-bot
2026-06-09 6:47 ` [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Andy Shevchenko
2026-06-09 10:33 ` Christian König
6 siblings, 2 replies; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:41 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
A later change will make list_for_each_entry() cache the next element
before entering the loop body. dapm_widget_invalidate_paths() appends
newly reached widgets to the temporary work list while walking it.
Keep the work-list walk open-coded so the next widget is looked up after
new widgets have been appended. This preserves the existing invalidation
traversal semantics and prepares the code for the list iterator update.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
sound/soc/soc-dapm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index d6192204e613..5bd921fca132 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -255,7 +255,9 @@ static __always_inline void dapm_widget_invalidate_paths(
list_add_tail(&w->work_list, &list);
w->endpoints[dir] = -1;
- list_for_each_entry(w, &list, work_list) {
+ for (w = list_first_entry(&list, typeof(*w), work_list);
+ !list_entry_is_head(w, &list, work_list);
+ w = list_next_entry(w, work_list)) {
snd_soc_dapm_widget_for_each_path(w, dir, p) {
if (p->is_supply || !p->connect)
continue;
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v2 14/14] list: Cache cursors in entry iterators
2026-06-09 6:41 ` [PATCH v2 13/14] ASoC: dapm: Open-code widget invalidation walk Kaitao Cheng
@ 2026-06-09 6:41 ` Kaitao Cheng
2026-06-09 6:59 ` sashiko-bot
2026-06-09 6:55 ` [PATCH v2 13/14] ASoC: dapm: Open-code widget invalidation walk sashiko-bot
1 sibling, 1 reply; 29+ messages in thread
From: Kaitao Cheng @ 2026-06-09 6:41 UTC (permalink / raw)
To: Andy Shevchenko, Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaitao Cheng, Kaitao Cheng
From: Kaitao Cheng <chengkaitao@kylinos.cn>
The non-safe list_for_each_entry() family advances by deriving the next
element from the current cursor in the loop step. If the loop body
unlinks the current entry, the step can no longer rely on the current
entry's list pointers.
Callers can use the _safe variants today, but those interfaces require a
temporary cursor to be declared outside the macro. That is necessary when
the caller actually needs the temporary cursor, but it looks redundant
and awkward when the cursor is only there to satisfy the macro and is
never otherwise used.
Add private next and previous cursors for the common entry iterators and
use unique internal names so callers keep the same interface. This lets
the loop step use a cursor captured before the body runs, while callers
that need to alter traversal state can still open-code the walk.
The safe variants remain useful when the caller needs access to the
temporary cursor or has stronger mutation requirements. Update their
comments to steer users toward the simpler iterators when that temporary
cursor is not needed.
Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
include/linux/list.h | 46 +++++++++++++++++++++++++++++++++-----------
1 file changed, 35 insertions(+), 11 deletions(-)
diff --git a/include/linux/list.h b/include/linux/list.h
index 09d979976b3b..9df84a56a789 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -809,6 +809,29 @@ static inline size_t list_count_nodes(struct list_head *head)
#define list_entry_is_head(pos, head, member) \
list_is_head(&pos->member, (head))
+#define __list_for_each_entry(pos, next, head, member) \
+ for (typeof(pos) next = list_next_entry(pos = \
+ list_first_entry(head, typeof(*pos), member), member); \
+ !list_entry_is_head(pos, head, member); \
+ pos = next, next = list_next_entry(next, member))
+
+#define __list_for_each_entry_reverse(pos, prev, head, member) \
+ for (typeof(pos) prev = list_prev_entry(pos = \
+ list_last_entry(head, typeof(*pos), member), member); \
+ !list_entry_is_head(pos, head, member); \
+ pos = prev, prev = list_prev_entry(prev, member))
+
+#define __list_for_each_entry_continue(pos, next, head, member) \
+ for (typeof(pos) next = list_next_entry(pos = \
+ list_next_entry(pos, member), member); \
+ !list_entry_is_head(pos, head, member); \
+ pos = next, next = list_next_entry(next, member))
+
+#define __list_for_each_entry_from(pos, next, head, member) \
+ for (typeof(pos) next = list_next_entry(pos, member); \
+ !list_entry_is_head(pos, head, member); \
+ pos = next, next = list_next_entry(next, member))
+
/**
* list_for_each_entry - iterate over list of given type
* @pos: the type * to use as a loop cursor.
@@ -816,9 +839,7 @@ static inline size_t list_count_nodes(struct list_head *head)
* @member: the name of the list_head within the struct.
*/
#define list_for_each_entry(pos, head, member) \
- for (pos = list_first_entry(head, typeof(*pos), member); \
- !list_entry_is_head(pos, head, member); \
- pos = list_next_entry(pos, member))
+ __list_for_each_entry(pos, __UNIQUE_ID(next), head, member)
/**
* list_for_each_entry_reverse - iterate backwards over list of given type.
@@ -827,9 +848,7 @@ static inline size_t list_count_nodes(struct list_head *head)
* @member: the name of the list_head within the struct.
*/
#define list_for_each_entry_reverse(pos, head, member) \
- for (pos = list_last_entry(head, typeof(*pos), member); \
- !list_entry_is_head(pos, head, member); \
- pos = list_prev_entry(pos, member))
+ __list_for_each_entry_reverse(pos, __UNIQUE_ID(prev), head, member)
/**
* list_prepare_entry - prepare a pos entry for use in list_for_each_entry_continue()
@@ -852,9 +871,7 @@ static inline size_t list_count_nodes(struct list_head *head)
* the current position.
*/
#define list_for_each_entry_continue(pos, head, member) \
- for (pos = list_next_entry(pos, member); \
- !list_entry_is_head(pos, head, member); \
- pos = list_next_entry(pos, member))
+ __list_for_each_entry_continue(pos, __UNIQUE_ID(next), head, member)
/**
* list_for_each_entry_continue_reverse - iterate backwards from the given point
@@ -879,8 +896,7 @@ static inline size_t list_count_nodes(struct list_head *head)
* Iterate over list of given type, continuing from current position.
*/
#define list_for_each_entry_from(pos, head, member) \
- for (; !list_entry_is_head(pos, head, member); \
- pos = list_next_entry(pos, member))
+ __list_for_each_entry_from(pos, __UNIQUE_ID(next), head, member)
/**
* list_for_each_entry_from_reverse - iterate backwards over list of given type
@@ -901,6 +917,8 @@ static inline size_t list_count_nodes(struct list_head *head)
* @n: another type * to use as temporary storage
* @head: the head for your list.
* @member: the name of the list_head within the struct.
+ *
+ * Prefer list_for_each_entry() unless the temporary cursor is needed.
*/
#define list_for_each_entry_safe(pos, n, head, member) \
for (pos = list_first_entry(head, typeof(*pos), member), \
@@ -917,6 +935,8 @@ static inline size_t list_count_nodes(struct list_head *head)
*
* Iterate over list of given type, continuing after current point,
* safe against removal of list entry.
+ *
+ * Prefer list_for_each_entry_continue() unless the temporary cursor is needed.
*/
#define list_for_each_entry_safe_continue(pos, n, head, member) \
for (pos = list_next_entry(pos, member), \
@@ -933,6 +953,8 @@ static inline size_t list_count_nodes(struct list_head *head)
*
* Iterate over list of given type from current point, safe against
* removal of list entry.
+ *
+ * Prefer list_for_each_entry_from() unless the temporary cursor is needed.
*/
#define list_for_each_entry_safe_from(pos, n, head, member) \
for (n = list_next_entry(pos, member); \
@@ -948,6 +970,8 @@ static inline size_t list_count_nodes(struct list_head *head)
*
* Iterate backwards over list of given type, safe against removal
* of list entry.
+ *
+ * Prefer list_for_each_entry_reverse() unless the temporary cursor is needed.
*/
#define list_for_each_entry_safe_reverse(pos, n, head, member) \
for (pos = list_last_entry(head, typeof(*pos), member), \
--
2.43.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 14/14] list: Cache cursors in entry iterators
2026-06-09 6:41 ` [PATCH v2 14/14] list: Cache cursors in entry iterators Kaitao Cheng
@ 2026-06-09 6:59 ` sashiko-bot
0 siblings, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:59 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Modifying the standard list iterators to pre-cache the `next` pointer fundamentally changes their traversal semantics from dynamic to static evaluation, causing use-after-free (UAF) vulnerabilities and silently breaking loops that modify the list ahead of the cursor or skip elements by advancing `pos`.
- [High] Eliminating the distinction between safe and non-safe list iterators obscures code intent, removes a vital visual guardrail for destructive operations, and introduces severe backporting risks.
--
commit e1c09f66ccd28cca66865681febeb6f8233f0a36
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
list: Cache cursors in entry iterators
This commit introduces private next and previous cursor variables into the
standard list iteration macros. It changes the semantics of the
list_for_each_entry() family to cache the next pointer before executing the
loop body. It also updates comments to recommend the non-safe variants over
the safe ones when a temporary cursor is not needed.
> diff --git a/include/linux/list.h b/include/linux/list.h
> index 09d979976b3b8..9df84a56a789b 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -809,6 +809,29 @@ static inline size_t list_count_nodes(struct list_head *head)
> #define list_entry_is_head(pos, head, member) \
> list_is_head(&pos->member, (head))
>
> +#define __list_for_each_entry(pos, next, head, member) \
> + for (typeof(pos) next = list_next_entry(pos = \
> + list_first_entry(head, typeof(*pos), member), member); \
> + !list_entry_is_head(pos, head, member); \
> + pos = next, next = list_next_entry(next, member))
[ ... ]
> @@ -816,9 +839,7 @@ static inline size_t list_count_nodes(struct list_head *head)
> * @member: the name of the list_head within the struct.
> */
> #define list_for_each_entry(pos, head, member) \
> - for (pos = list_first_entry(head, typeof(*pos), member); \
> - !list_entry_is_head(pos, head, member); \
> - pos = list_next_entry(pos, member))
> + __list_for_each_entry(pos, __UNIQUE_ID(next), head, member)
[Severity: Critical]
Does caching the next pointer fundamentally change the dynamic traversal
semantics of the standard list iterators kernel-wide?
A common kernel pattern is pinning the current entry (pos), dropping the
lock to perform a blocking operation, and reacquiring the lock. With
dynamic evaluation, pos->next is safely fetched under the lock.
With the new cached next pointer, if the next element was removed and freed
by a concurrent thread while the lock was dropped, next becomes a dangling
pointer, which could lead to a use-after-free when the loop steps.
Will this also silently skip dynamically inserted elements? If a loop body
inserts a new element immediately after pos, the old macro would naturally
visit it next. The new macro appears to silently skip it because it advances
using the pre-cached next.
Could this also overwrite manual cursor updates? If the loop body explicitly
alters pos to skip elements, the loop increment (pos = next) might overwrite
the updated cursor with the pre-cached next, negating the skip and potentially
causing incorrect traversal behavior.
[ ... ]
> @@ -901,6 +917,8 @@ static inline size_t list_count_nodes(struct list_head *head)
> * @n: another type * to use as temporary storage
> * @head: the head for your list.
> * @member: the name of the list_head within the struct.
> + *
> + * Prefer list_for_each_entry() unless the temporary cursor is needed.
> */
> #define list_for_each_entry_safe(pos, n, head, member) \
[Severity: High]
Does advising developers to prefer list_for_each_entry() over the _safe
variant for removals obscure the intent of the code?
The _safe variant explicitly signals to reviewers and static analysis tools
that the loop body may mutate the list. Conflating variable-scoping
convenience with traversal safety constraints removes a visual guardrail for
destructive operations.
Additionally, does this introduce a backporting hazard? If developers write
new code that unlinks elements using list_for_each_entry(), backporting those
patches to older stable kernels (which lack this caching patch) will compile
cleanly but might reliably introduce use-after-free vulnerabilities, as older
kernels evaluate pos->next after pos has been unlinked.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=14
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 13/14] ASoC: dapm: Open-code widget invalidation walk
2026-06-09 6:41 ` [PATCH v2 13/14] ASoC: dapm: Open-code widget invalidation walk Kaitao Cheng
2026-06-09 6:41 ` [PATCH v2 14/14] list: Cache cursors in entry iterators Kaitao Cheng
@ 2026-06-09 6:55 ` sashiko-bot
1 sibling, 0 replies; 29+ messages in thread
From: sashiko-bot @ 2026-06-09 6:55 UTC (permalink / raw)
To: Kaitao Cheng; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete preparation for list iterator semantics change causes DAPM power loop to terminate prematurely.
--
commit 43f2e05998002333ccbe3a3f202cf3f76c35d1b0
Author: Kaitao Cheng <chengkaitao@kylinos.cn>
ASoC: dapm: Open-code widget invalidation walk
This commit prepares dapm_widget_invalidate_paths() for an upcoming change
to list_for_each_entry() that will cache the next element. It open-codes the
loop to preserve the existing semantics where new widgets can be appended
during traversal.
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index d6192204e613d..5bd921fca1323 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -255,7 +255,9 @@ static __always_inline void dapm_widget_invalidate_paths(
> list_add_tail(&w->work_list, &list);
> w->endpoints[dir] = -1;
>
> - list_for_each_entry(w, &list, work_list) {
> + for (w = list_first_entry(&list, typeof(*w), work_list);
> + !list_entry_is_head(w, &list, work_list);
> + w = list_next_entry(w, work_list)) {
[Severity: High]
Does this open-coding also need to be applied to the loop in
dapm_power_widgets()?
dapm_power_widgets() loops over card->dapm_dirty using list_for_each_entry():
sound/soc/soc-dapm.c:dapm_power_widgets() {
...
list_for_each_entry(w, &card->dapm_dirty, dirty) {
dapm_power_one_widget(w, &up_list, &down_list);
}
...
}
Inside that loop, dapm_power_one_widget() can invoke
dapm_widget_set_peer_power(), which calls dapm_mark_dirty() to append peer
widgets to the tail of card->dapm_dirty.
If the upcoming list_for_each_entry() changes cache the next element, won't
the loop in dapm_power_widgets() terminate early and fail to process newly
dirtied widgets if they are appended while processing the current tail
element?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609064122.95825-1-kaitao.cheng@linux.dev?part=13
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state
2026-06-09 6:13 [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Kaitao Cheng
` (4 preceding siblings ...)
2026-06-09 6:41 ` [PATCH v2 13/14] ASoC: dapm: Open-code widget invalidation walk Kaitao Cheng
@ 2026-06-09 6:47 ` Andy Shevchenko
2026-06-09 7:05 ` Andy Shevchenko
2026-06-09 10:33 ` Christian König
6 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2026-06-09 6:47 UTC (permalink / raw)
To: Kaitao Cheng
Cc: Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Matthew Auld, Matthew Brost, Waiman Long,
drbd-dev, linux-block, linux1394-devel, dri-devel, intel-gfx,
linux-spi, linux-stm32, linux-arm-kernel, linux-tegra,
linux-sound, linux-kernel, Andrew Morton, Randy Dunlap,
Christian Brauner, David Howells, Luca Ceresoli, Kaito Cheng
On Tue, Jun 09, 2026 at 02:13:33PM +0800, Kaitao Cheng wrote:
>
> This series prepares for, and then updates, the list_for_each_entry()
> family so the common entry iterators cache their next or previous cursor
> before the loop body runs.
>
> The first 13 patches open-code loops that intentionally depend on the
> old "derive the next entry from the current cursor at the end of the
> iteration" behaviour. These loops append work to the list being walked,
> restart traversal after dropping a lock, skip an entry consumed by the
> current iteration, or otherwise adjust the cursor in the loop body.
>
> The final patch changes include/linux/list.h to keep a private cursor in
> the common entry iterators while preserving the public macro interface.
> The safe variants remain available when callers need the temporary
> cursor explicitly or have stronger mutation requirements.
Something is really wrong with the patch series email chaining.
Patches 3, 10, and 13 start the subthreads. Please, check your
tools and fix them accordingly.
Note, `git format-patch ...` should not have this "side-effect"
when used correctly.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state
2026-06-09 6:47 ` [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Andy Shevchenko
@ 2026-06-09 7:05 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2026-06-09 7:05 UTC (permalink / raw)
To: Kaitao Cheng
Cc: Muchun Song, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Takashi Sakamoto,
Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin,
Christian Koenig, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Matthew Auld, Matthew Brost, Waiman Long,
drbd-dev, linux-block, linux1394-devel, dri-devel, intel-gfx,
linux-spi, linux-stm32, linux-arm-kernel, linux-tegra,
linux-sound, linux-kernel, Andrew Morton, Randy Dunlap,
Christian Brauner, David Howells, Luca Ceresoli, Kaito Cheng
On Tue, Jun 09, 2026 at 09:47:34AM +0300, Andy Shevchenko wrote:
> On Tue, Jun 09, 2026 at 02:13:33PM +0800, Kaitao Cheng wrote:
> >
> > This series prepares for, and then updates, the list_for_each_entry()
> > family so the common entry iterators cache their next or previous cursor
> > before the loop body runs.
While code looks okay, this doesn't explain "why?" aspects.
> > The first 13 patches open-code loops that intentionally depend on the
> > old "derive the next entry from the current cursor at the end of the
> > iteration" behaviour. These loops append work to the list being walked,
> > restart traversal after dropping a lock, skip an entry consumed by the
> > current iteration, or otherwise adjust the cursor in the loop body.
> >
> > The final patch changes include/linux/list.h to keep a private cursor in
> > the common entry iterators while preserving the public macro interface.
> > The safe variants remain available when callers need the temporary
> > cursor explicitly or have stronger mutation requirements.
>
> Something is really wrong with the patch series email chaining.
> Patches 3, 10, and 13 start the subthreads. Please, check your
> tools and fix them accordingly.
>
> Note, `git format-patch ...` should not have this "side-effect"
> when used correctly.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state
2026-06-09 6:13 [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Kaitao Cheng
` (5 preceding siblings ...)
2026-06-09 6:47 ` [PATCH v2 00/14] list: Prepare entry iterators to cache cursor state Andy Shevchenko
@ 2026-06-09 10:33 ` Christian König
6 siblings, 0 replies; 29+ messages in thread
From: Christian König @ 2026-06-09 10:33 UTC (permalink / raw)
To: Kaitao Cheng, Andy Shevchenko, Muchun Song, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder, Jens Axboe,
Takashi Sakamoto, Andrzej Hajda, Neil Armstrong, Robert Foss,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Huang Rui, Eddie James, Mark Brown,
Maxime Coquelin, Alexandre Torgue, Laxman Dewangan,
Thierry Reding, Jonathan Hunter, Sowjanya Komatineni,
Davidlohr Bueso, Paul E . McKenney, Josh Triplett, Peter Zijlstra,
Ingo Molnar, Will Deacon, Boqun Feng, Liam Girdwood,
Jaroslav Kysela, Takashi Iwai
Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Matthew Auld,
Matthew Brost, Waiman Long, drbd-dev, linux-block,
linux1394-devel, dri-devel, intel-gfx, linux-spi, linux-stm32,
linux-arm-kernel, linux-tegra, linux-sound, linux-kernel,
Andrew Morton, Randy Dunlap, Christian Brauner, David Howells,
Luca Ceresoli, Kaito Cheng
On 6/9/26 08:13, Kaitao Cheng wrote:
> From: Kaito Cheng <chengkaitao@kylinos.cn>
>
> This series prepares for, and then updates, the list_for_each_entry()
> family so the common entry iterators cache their next or previous cursor
> before the loop body runs.
Why in the world would we want to do that?
The safe and non-safe variants have very distinct use cases and that is completely intentional.
What we could improve maybe is the documentation, from my experience an astonishing large amount of people have misconceptions about the safe variants.
> The first 13 patches open-code loops that intentionally depend on the
> old "derive the next entry from the current cursor at the end of the
> iteration" behaviour. These loops append work to the list being walked,
> restart traversal after dropping a lock, skip an entry consumed by the
> current iteration, or otherwise adjust the cursor in the loop body.
Well I have to clearly reject the changes for subsystems/components I'm maintaining, that just looks horrible to me and I clearly don't see a good reason for that.
Regards,
Christian.
>
> The final patch changes include/linux/list.h to keep a private cursor in
> the common entry iterators while preserving the public macro interface.
> The safe variants remain available when callers need the temporary
> cursor explicitly or have stronger mutation requirements.
>
> Changes in v2 (Muchun Song, Andy Shevchenko):
> - Drop the list_for_each_entry_mutable*() helpers from v1 and make the
> cursor change directly in the existing list_for_each_entry*() helpers.
> - Open-code special list walks that rely on updating the loop cursor in
> the body, preserving their existing traversal semantics.
>
> Link to v1:
> https://lore.kernel.org/all/20260529082149.76764-1-kaitao.cheng@linux.dev/
>
> Kaitao Cheng (14):
> drbd: Open-code transfer log list walk
> firewire: core: Open-code topology list walk
> drm/bridge: Open-code bridge chain list walks
> drm/i915/gt: Open-code active timeline walk
> drm/i915: Open-code DFS dependency list walk
> drm/ttm: Open-code reservation list walk
> spi: fsi: Open-code message transfer walk
> spi: stm32-ospi: Open-code message transfer walk
> spi: stm32-qspi: Open-code message transfer walk
> spi: tegra210-quad: Open-code message transfer walk
> locking/locktorture: Open-code ww mutex list walk
> locking/ww_mutex: Open-code stress reorder list walk
> ASoC: dapm: Open-code widget invalidation walk
> list: Cache cursors in entry iterators
>
> drivers/block/drbd/drbd_debugfs.c | 4 ++-
> drivers/firewire/core-topology.c | 4 ++-
> drivers/gpu/drm/drm_bridge.c | 7 ++--
> drivers/gpu/drm/i915/gt/intel_reset.c | 4 ++-
> drivers/gpu/drm/i915/i915_scheduler.c | 4 ++-
> drivers/gpu/drm/ttm/ttm_execbuf_util.c | 4 ++-
> drivers/spi/spi-fsi.c | 5 ++-
> drivers/spi/spi-stm32-ospi.c | 4 ++-
> drivers/spi/spi-stm32-qspi.c | 5 ++-
> drivers/spi/spi-tegra210-quad.c | 4 ++-
> include/linux/list.h | 46 ++++++++++++++++++++------
> kernel/locking/locktorture.c | 4 ++-
> kernel/locking/test-ww_mutex.c | 4 ++-
> sound/soc/soc-dapm.c | 4 ++-
> 14 files changed, 78 insertions(+), 25 deletions(-)
>
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread