[PATCH v2 0/5] firewire: core: make execution context assumptions explicit

[PATCH v2 0/5] firewire: core: make execution context assumptions explicit

[Takashi Sakamoto]

Hi,

This series is take 2 of the previous series[1].

Recent code changes have guaranteed the execution contexts of several
functions in the transaction layer. Based on this, this series adds
might_sleep() calls where appropriate. It also adds sparse annotations
and lockdep checks for several locks to document the expected locking
context and help detect incorrect locking behavior.

Chanes from v1:
* drop a patch to arrange spinlock usages for reconsideration

[1] https://lore.kernel.org/lkml/20261008234432.251412-1-o-takashi@sakamocchi.jp/


Takashi Sakamoto (5):
  firewire: core: add might_sleep() checks
  firewire: core: use sparse annotations and lockdep checks for
    transaction lock
  firewire: core: use sparse annotations and lockdep checks for topology
    map lock
  firewire: core: use sparse annotations and lockdep checks for split
    timeout lock
  firewire: core: narrow the card lock scope when accessing node_id

 drivers/firewire/core-topology.c    |  3 ++
 drivers/firewire/core-transaction.c | 70 ++++++++++++++++++++++++++---
 2 files changed, 66 insertions(+), 7 deletions(-)


base-commit: a8cf19ce243bf2cd0d338823fa79cd927e34b830
-- 
2.53.0

[PATCH v2 1/5] firewire: core: add might_sleep() checks

[Takashi Sakamoto]

Some functions are now guaranteed to run in process context.

Add might_sleep() calls to these functions to document and check this
requirement.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index a0379fc3e60e..be8b2b7b887f 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -114,6 +114,9 @@ void fw_cancel_pending_transactions(struct fw_card *card)
  */
 int fw_cancel_transaction(struct fw_card *card, struct fw_transaction *transaction)
 {
+	// Allow the call of struct fw_card_driver.cancel_packet() to wait for any event.
+	might_sleep();
+
 	// Cancel the packet transmission if it's still queued. That will schedule the packet
 	// transmission callback which cancels the transaction.
 	if (card->driver->cancel_packet(card, &transaction->packet) == 0)
@@ -488,6 +491,9 @@ int fw_run_transaction(struct fw_card *card, int tcode, int destination_id,
 	struct transaction_callback_data d;
 	struct fw_transaction t;
 
+	// Due to the call of wait_for_completion().
+	might_sleep();
+
 	timer_setup_on_stack(&t.split_timeout_timer, NULL, 0);
 	init_completion(&d.done);
 	d.payload = payload;
@@ -524,6 +530,9 @@ void fw_send_phy_config(struct fw_card *card,
 	long timeout = msecs_to_jiffies(100);
 	u32 data = 0;
 
+	// Due to the call of wait_for_completion_timeout().
+	might_sleep();
+
 	phy_packet_set_packet_identifier(&data, PHY_PACKET_PACKET_IDENTIFIER_PHY_CONFIG);
 
 	if (node_id != FW_PHY_CONFIG_NO_NODE_ID) {
@@ -697,6 +706,9 @@ EXPORT_SYMBOL(fw_core_add_address_handler);
  */
 void fw_core_remove_address_handler(struct fw_address_handler *handler)
 {
+	// Due to synchronize_rcu().
+	might_sleep();
+
 	scoped_guard(spinlock, &address_handler_list_lock)
 		list_del_rcu(&handler->link);
 
@@ -1104,6 +1116,10 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
 	unsigned long long offset;
 	unsigned int tcode;
 
+	// Allow the call of allocate_request() to perform object allocation with GFP_KERNEL,
+	// as well as the address handlers to work with the sleepable lock primitives.
+	might_sleep();
+
 	if (p->ack != ACK_PENDING && p->ack != ACK_COMPLETE)
 		return;
 
@@ -1142,6 +1158,9 @@ void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
 	size_t data_length;
 	int tcode, tlabel, source, rcode;
 
+	// Allow the call of struct fw_card_driver.cancel_packet() to wait for any event.
+	might_sleep();
+
 	tcode = async_header_get_tcode(p->header);
 	tlabel = async_header_get_tlabel(p->header);
 	source = async_header_get_source(p->header);
-- 
2.53.0

[PATCH v2 2/5] firewire: core: use sparse annotations and lockdep checks for transaction lock

[Takashi Sakamoto]

The spinlock protects the list of pending transactions and the transaction
label.

Add sparse annotations and lockdep checks for this lock.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index be8b2b7b887f..2692e875b00a 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -38,16 +38,21 @@
 
 /* returns 0 if the split timeout handler is already running */
 static int try_cancel_split_timeout(struct fw_transaction *t)
+__must_hold(&t->card->transactions.lock)
 {
+	lockdep_assert_held(&t->card->transactions.lock);
+
 	if (t->is_split_transaction)
 		return timer_delete(&t->split_timeout_timer) || disable_work(&t->error_work);
 	else
 		return 1;
 }
 
-// card->transactions.lock must be acquired in advance.
 static void remove_transaction_entry(struct fw_card *card, struct fw_transaction *entry)
+__must_hold(&card->transactions.lock)
 {
+	lockdep_assert_held(&card->transactions.lock);
+
 	list_del_init(&entry->link);
 	card->transactions.tlabel_mask &= ~(1ULL << entry->tlabel);
 }
@@ -65,10 +70,13 @@ static void invoke_callback(struct fw_transaction *t, int rcode, u32 response_ts
 
 // Must be called without holding card->transactions.lock.
 void fw_cancel_pending_transactions(struct fw_card *card)
+__must_not_hold(&card->transactions.lock)
 {
 	struct fw_transaction *t, *tmp;
 	LIST_HEAD(pending_list);
 
+	lockdep_assert_not_held(&card->transactions.lock);
+
 	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
 	// local destination never runs in any type of IRQ context.
 	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
@@ -113,7 +121,10 @@ void fw_cancel_pending_transactions(struct fw_card *card)
  * pending transaction.
  */
 int fw_cancel_transaction(struct fw_card *card, struct fw_transaction *transaction)
+__must_not_hold(&card->transactions.lock)
 {
+	lockdep_assert_not_held(&card->transactions.lock);
+
 	// Allow the call of struct fw_card_driver.cancel_packet() to wait for any event.
 	might_sleep();
 
@@ -161,10 +172,13 @@ static void schedule_error_callback(struct fw_transaction *t, int rcode, u32 res
 }
 
 static void split_transaction_timeout_callback(struct timer_list *timer)
+__must_not_hold(&card->transactions.lock)
 {
 	struct fw_transaction *t = timer_container_of(t, timer, split_timeout_timer);
 	struct fw_card *card = t->card;
 
+	lockdep_assert_not_held(&card->transactions.lock);
+
 	scoped_guard(spinlock_irqsave, &card->transactions.lock) {
 		if (list_empty(&t->link))
 			return;
@@ -174,9 +188,11 @@ static void split_transaction_timeout_callback(struct timer_list *timer)
 	schedule_error_callback(t, RCODE_CANCELLED, t->split_timeout_cycle);
 }
 
-// card->transactions.lock should be acquired in advance for the linked list.
 static void start_split_transaction_timeout(struct fw_transaction *t, unsigned int delta)
+__must_hold(&t->card->transactions.lock)
 {
+	lockdep_assert_held(&t->card->transactions.lock);
+
 	if (list_empty(&t->link) || WARN_ON(t->is_split_transaction))
 		return;
 
@@ -189,10 +205,13 @@ static u32 compute_split_timeout_timestamp(struct fw_card *card, u32 request_tim
 
 static void transmit_complete_callback(struct fw_packet *packet,
 				       struct fw_card *card, int status)
+__must_not_hold(&card->transactions.lock)
 {
 	struct fw_transaction *t =
 	    container_of(packet, struct fw_transaction, packet);
 
+	lockdep_assert_not_held(&card->transactions.lock);
+
 	trace_async_request_outbound_complete((uintptr_t)t, card->index, packet->generation,
 					      packet->speed, status, packet->timestamp);
 
@@ -388,9 +407,12 @@ void __fw_send_request(struct fw_card *card, struct fw_transaction *t, int tcode
 		int destination_id, int generation, int speed, unsigned long long offset,
 		void *payload, size_t length, union fw_transaction_callback callback,
 		bool with_tstamp, void *callback_data)
+__must_not_hold(&card->transactions.lock)
 {
 	int tlabel;
 
+	lockdep_assert_not_held(&card->transactions.lock);
+
 	t->card = card;
 	t->callback = callback;
 	t->with_tstamp = with_tstamp;
@@ -1152,12 +1174,15 @@ void fw_core_handle_request(struct fw_card *card, struct fw_packet *p)
 EXPORT_SYMBOL(fw_core_handle_request);
 
 void fw_core_handle_response(struct fw_card *card, struct fw_packet *p)
+__must_not_hold(&card->transactions.lock)
 {
 	struct fw_transaction *t = NULL;
 	u32 *data;
 	size_t data_length;
 	int tcode, tlabel, source, rcode;
 
+	lockdep_assert_not_held(&card->transactions.lock);
+
 	// Allow the call of struct fw_card_driver.cancel_packet() to wait for any event.
 	might_sleep();
 
-- 
2.53.0

[PATCH v2 3/5] firewire: core: use sparse annotations and lockdep checks for topology map lock

[Takashi Sakamoto]

The spinlock protects the topology map from concurrent access.

Add sparse annotations and lockdep checks for this lock.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-topology.c    | 3 +++
 drivers/firewire/core-transaction.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index ee6b54f89859..512b7e4473af 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -474,9 +474,12 @@ static void update_topology_map(__be32 *buffer, size_t buffer_size, int root_nod
 
 void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation,
 			      int self_id_count, u32 *self_ids, bool bm_abdicate)
+__must_not_hold(&card->topology_map.lock)
 {
 	struct fw_node *local_node;
 
+	lockdep_assert_not_held(&card->topology_map.lock);
+
 	trace_bus_reset_handle(card->index, generation, node_id, bm_abdicate, self_ids, self_id_count);
 
 	scoped_guard(spinlock, &card->lock) {
diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 2692e875b00a..e7cff597855b 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -1280,9 +1280,12 @@ static void handle_topology_map(struct fw_card *card, struct fw_request *request
 		int tcode, int destination, int source, int generation,
 		unsigned long long offset, void *payload, size_t length,
 		void *callback_data)
+__must_not_hold(&card->topology_map.lock)
 {
 	int start;
 
+	lockdep_assert_not_held(&card->topology_map.lock);
+
 	if (!tcode_is_read_request(tcode)) {
 		fw_send_response(card, request, RCODE_TYPE_ERROR);
 		return;
-- 
2.53.0

[PATCH v2 4/5] firewire: core: use sparse annotations and lockdep checks for split timeout lock

[Takashi Sakamoto]

The spinlock protects parameters related to split transaction timeouts.

Add sparse annotations and lockdep checks for this lock.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index e7cff597855b..f290ce25facc 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -205,11 +205,13 @@ static u32 compute_split_timeout_timestamp(struct fw_card *card, u32 request_tim
 
 static void transmit_complete_callback(struct fw_packet *packet,
 				       struct fw_card *card, int status)
+__must_not_hold(&card->split_timeout.lock)
 __must_not_hold(&card->transactions.lock)
 {
 	struct fw_transaction *t =
 	    container_of(packet, struct fw_transaction, packet);
 
+	lockdep_assert_not_held(&card->split_timeout.lock);
 	lockdep_assert_not_held(&card->transactions.lock);
 
 	trace_async_request_outbound_complete((uintptr_t)t, card->index, packet->generation,
@@ -893,11 +895,14 @@ __must_hold(&card->split_timeout.lock)
 
 static struct fw_request *allocate_request(struct fw_card *card,
 					   struct fw_packet *p)
+__must_not_hold(&card->split_timeout.lock)
 {
 	struct fw_request *request;
 	u32 *data, length;
 	int request_tcode;
 
+	lockdep_assert_not_held(&card->split_timeout.lock);
+
 	request_tcode = async_header_get_tcode(p->header);
 	switch (request_tcode) {
 	case TCODE_WRITE_QUADLET_REQUEST:
@@ -1318,6 +1323,8 @@ __must_hold(&card->split_timeout.lock)
 {
 	unsigned int cycles;
 
+	lockdep_assert_held(&card->split_timeout.lock);
+
 	cycles = card->split_timeout.hi * 8000 + (card->split_timeout.lo >> 19);
 
 	/* minimum per IEEE 1394, maximum which doesn't overflow OHCI */
-- 
2.53.0

[PATCH v2 5/5] firewire: core: narrow the card lock scope when accessing node_id

[Takashi Sakamoto]

When preparing the request subaction for an asynchronous transaction,
the node_id field in the fw_card structure is accessed under the
card-level spinlock. However, the lock is held across a call to a helper
function, unnecessarily extending the critical section.

Narrow the critical section to cover only the access to node_id.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-transaction.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index f290ce25facc..1c07fd0d2dd8 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -450,13 +450,15 @@ __must_not_hold(&card->transactions.lock)
 	timer_setup(&t->split_timeout_timer, split_transaction_timeout_callback, 0);
 	t->packet.callback = transmit_complete_callback;
 
+	// The node_id field of fw_card can be updated when handling SelfIDComplete.
+	int node_id;
+
 	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
 	// local destination never runs in any type of IRQ context.
-	scoped_guard(spinlock_irqsave, &card->lock) {
-		// The node_id field of fw_card can be updated when handling SelfIDComplete.
-		fw_fill_request(&t->packet, tcode, t->tlabel, destination_id, card->node_id,
-				generation, speed, offset, payload, length);
-	}
+	scoped_guard(spinlock_irqsave, &card->lock)
+		node_id = card->node_id;
+	fw_fill_request(&t->packet, tcode, t->tlabel, destination_id, node_id, generation, speed,
+			offset, payload, length);
 
 	// NOTE: This can be without irqsave when we can guarantee that __fw_send_request() for
 	// local destination never runs in any type of IRQ context.
-- 
2.53.0

Re: [PATCH v2 0/5] firewire: core: make execution context assumptions explicit

[Takashi Sakamoto]

Hi,

On Fri, Oct 09, 2026 at 09:10:43PM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> This series is take 2 of the previous series[1].
> 
> Recent code changes have guaranteed the execution contexts of several
> functions in the transaction layer. Based on this, this series adds
> might_sleep() calls where appropriate. It also adds sparse annotations
> and lockdep checks for several locks to document the expected locking
> context and help detect incorrect locking behavior.
> 
> Chanes from v1:
> * drop a patch to arrange spinlock usages for reconsideration
> 
> [1] https://lore.kernel.org/lkml/20261008234432.251412-1-o-takashi@sakamocchi.jp/
> 
> 
> Takashi Sakamoto (5):
>   firewire: core: add might_sleep() checks
>   firewire: core: use sparse annotations and lockdep checks for
>     transaction lock
>   firewire: core: use sparse annotations and lockdep checks for topology
>     map lock
>   firewire: core: use sparse annotations and lockdep checks for split
>     timeout lock
>   firewire: core: narrow the card lock scope when accessing node_id
> 
>  drivers/firewire/core-topology.c    |  3 ++
>  drivers/firewire/core-transaction.c | 70 ++++++++++++++++++++++++++---
>  2 files changed, 66 insertions(+), 7 deletions(-)

I realized that Sparse nowadays seems to be excluded from in-kernel
context analysis[1]. Clang took over this function with more sophisticated
features.

Let me work for refining this series with Compiler-based Context
Analysis[2].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b63d0ae94cc
[2] https://docs.kernel.org/dev-tools/context-analysis.html


Regards

Takashi Sakamoto