From nobody Thu Oct 2 15:36:59 2025 Received: from fout-b5-smtp.messagingengine.com (fout-b5-smtp.messagingengine.com [202.12.124.148]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AE0B12DF149 for ; Mon, 15 Sep 2025 23:47:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.148 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757980080; cv=none; b=mSbcHMwWFQHBzG7sAS0QyagfpKel7gumUrdIHl0mdoXv1Uvmc6USA00JoLCrjf/L2Puupk5kh0hohjd35GKl18jBcJyVmIRg+swfZkgRjrUqzph/VyLVwFZ5qRgu+qFK362VB0rJe9gAAH2bxpBRL5/n/RvBtyXfQDVAfT10PAg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757980080; c=relaxed/simple; bh=0j3gWXdeyB+uXOybDwzFHQT6HxRLv5uMoPrCHdBqsoQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=lajglRsCOYyuYWqGK7D1gg1kgbSNJK/FdaG//pAXJCUzkCAsMAARes+TSVVYNA3ycMmGeaRugDkUgyk+MoQrPouNvDbNW/Wjbz0Wd5gNMnjS3qf1dQE+c6MtRSMQS02Iz6VbZj6NR2D97gWgBA9G2Xfh6pSx1IfBpVGvbDEInLA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sakamocchi.jp; spf=pass smtp.mailfrom=sakamocchi.jp; dkim=pass (2048-bit key) header.d=sakamocchi.jp header.i=@sakamocchi.jp header.b=pywV4gCN; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=bmS/Bh7b; arc=none smtp.client-ip=202.12.124.148 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=sakamocchi.jp Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=sakamocchi.jp Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=sakamocchi.jp header.i=@sakamocchi.jp header.b="pywV4gCN"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="bmS/Bh7b" Received: from phl-compute-01.internal (phl-compute-01.internal [10.202.2.41]) by mailfout.stl.internal (Postfix) with ESMTP id A37921D001B7; Mon, 15 Sep 2025 19:47:57 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Mon, 15 Sep 2025 19:47:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sakamocchi.jp; h=cc:cc:content-transfer-encoding:content-type:date:date:from :from:in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm1; t=1757980077; x= 1758066477; bh=ftRdwZYdPo5igfENPCqfeghdCxDccZ9UOFd6fov0EIE=; b=p ywV4gCNyrk68zkwyUIwCL9BMtcUTqujSgeDdoA490/iHRipJirD0CmsCEXJd81l3 q/vxmmfgZW8pS51F7tmTNSosSqa1oseA1WSVrk4ajA5vv1QBD5Up8FCJzBvrdhvX 9Ar56LWOUbg8h/LD79k93Mbnai2y8P9DMW8ffIZJHiybLGwfPQWOX4AKNUQfDkwg 7UCw64Rxg82BbJ8/X4A+YW40GRNi1zvaySWhJFuVeYR3caZeYAWhTofQ0O7sxb80 VxiydZ5gSmEtRD9gi2Vqc1N87HEX61JQwtWvxaRj+ZE9IJHy8a2kYxkjgT4dhkuv MJ0dj4rFbB31WgbouazGA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to:x-me-proxy:x-me-sender :x-me-sender:x-sasl-enc; s=fm1; t=1757980077; x=1758066477; bh=f tRdwZYdPo5igfENPCqfeghdCxDccZ9UOFd6fov0EIE=; b=bmS/Bh7bGeAdCpfuC ej14kHpghapfjS96YTApc6IDLCLkHOqEDvpXvm3+L2L5VInZ1pECYLcDlTm7vS7H CeaCRbK4ZbASgCSioO17eOUEWLYBRSXJDhndMdnktAtKLdeWGnPZfnzIELbj9jWD /BkGKtOcikKfwZ6/6VGU3RAz9hfl7z+nnVOMI7JODtC4oo5hL6TNZiI0TRusQ+ov wxA9VN5Xm6e85LOEemWBaFCpYp8gxd/OEpYEXgWQ3Nc3QRvgIHZM2xrNLsp3NQA0 PLfoN/XkHb1lUjm69DZZHknxgDmQCQ0+c5m/kR7UaQO6uaNPF3+mQYq/UPh/OQOf YeZnw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggdefledthecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpuffrtefokffrpgfnqfghnecuuegr ihhlohhuthemuceftddtnecunecujfgurhephffvvefufffkofgjfhgggfestdekredtre dttdenucfhrhhomhepvfgrkhgrshhhihcuufgrkhgrmhhothhouceoohdqthgrkhgrshhh ihesshgrkhgrmhhotggthhhirdhjpheqnecuggftrfgrthhtvghrnhepvdejgfejuedvgf duudekleevtefgtdevhfdtffefiefgveeuteffiedvffekvddtnecuvehluhhsthgvrhfu ihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepohdqthgrkhgrshhhihesshgrkh grmhhotggthhhirdhjphdpnhgspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhht pdhrtghpthhtoheplhhinhhugidufeelgedquggvvhgvlheslhhishhtshdrshhouhhrtg gvfhhorhhgvgdrnhgvthdprhgtphhtthhopehlihhnuhigqdhkvghrnhgvlhesvhhgvghr rdhkvghrnhgvlhdrohhrgh X-ME-Proxy: Feedback-ID: ie8e14432:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 15 Sep 2025 19:47:56 -0400 (EDT) From: Takashi Sakamoto To: linux1394-devel@lists.sourceforge.net Cc: linux-kernel@vger.kernel.org Subject: [PATCH 4/6] firewire: core: use spin lock specific to transaction Date: Tue, 16 Sep 2025 08:47:45 +0900 Message-ID: <20250915234747.915922-5-o-takashi@sakamocchi.jp> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250915234747.915922-1-o-takashi@sakamocchi.jp> References: <20250915234747.915922-1-o-takashi@sakamocchi.jp> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The list of instance for asynchronous transaction to wait for response subaction is maintained as a member of fw_card structure. The card-wide spinlock is used at present for any operation over the list, however it is not necessarily suited for the purpose. This commit adds and uses the spin lock specific to maintain the list. Signed-off-by: Takashi Sakamoto --- drivers/firewire/core-card.c | 12 ++++-- drivers/firewire/core-transaction.c | 59 ++++++++++++++++++----------- include/linux/firewire.h | 10 +++-- 3 files changed, 51 insertions(+), 30 deletions(-) diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index 616abb836ef3..39f95007305f 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -544,8 +544,12 @@ void fw_card_initialize(struct fw_card *card, card->index =3D atomic_inc_return(&index); card->driver =3D driver; card->device =3D device; - card->current_tlabel =3D 0; - card->tlabel_mask =3D 0; + + card->transactions.current_tlabel =3D 0; + card->transactions.tlabel_mask =3D 0; + INIT_LIST_HEAD(&card->transactions.list); + spin_lock_init(&card->transactions.lock); + card->split_timeout_hi =3D DEFAULT_SPLIT_TIMEOUT / 8000; card->split_timeout_lo =3D (DEFAULT_SPLIT_TIMEOUT % 8000) << 19; card->split_timeout_cycles =3D DEFAULT_SPLIT_TIMEOUT; @@ -555,7 +559,7 @@ void fw_card_initialize(struct fw_card *card, =20 kref_init(&card->kref); init_completion(&card->done); - INIT_LIST_HEAD(&card->transaction_list); + spin_lock_init(&card->lock); =20 card->local_node =3D NULL; @@ -772,7 +776,7 @@ void fw_core_remove_card(struct fw_card *card) destroy_workqueue(card->isoc_wq); destroy_workqueue(card->async_wq); =20 - WARN_ON(!list_empty(&card->transaction_list)); + WARN_ON(!list_empty(&card->transactions.list)); } EXPORT_SYMBOL(fw_core_remove_card); =20 diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-tr= ansaction.c index 8edffafd21c1..5366d8a781ac 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -49,12 +49,14 @@ static int close_transaction(struct fw_transaction *tra= nsaction, struct fw_card { struct fw_transaction *t =3D NULL, *iter; =20 - scoped_guard(spinlock_irqsave, &card->lock) { - list_for_each_entry(iter, &card->transaction_list, link) { + // 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) { + list_for_each_entry(iter, &card->transactions.list, link) { if (iter =3D=3D transaction) { if (try_cancel_split_timeout(iter)) { list_del_init(&iter->link); - card->tlabel_mask &=3D ~(1ULL << iter->tlabel); + card->transactions.tlabel_mask &=3D ~(1ULL << iter->tlabel); t =3D iter; } break; @@ -117,11 +119,11 @@ static void split_transaction_timeout_callback(struct= timer_list *timer) struct fw_transaction *t =3D timer_container_of(t, timer, split_timeout_t= imer); struct fw_card *card =3D t->card; =20 - scoped_guard(spinlock_irqsave, &card->lock) { + scoped_guard(spinlock_irqsave, &card->transactions.lock) { if (list_empty(&t->link)) return; list_del(&t->link); - card->tlabel_mask &=3D ~(1ULL << t->tlabel); + card->transactions.tlabel_mask &=3D ~(1ULL << t->tlabel); } =20 if (!t->with_tstamp) { @@ -259,18 +261,21 @@ static void fw_fill_request(struct fw_packet *packet,= int tcode, int tlabel, } =20 static int allocate_tlabel(struct fw_card *card) +__must_hold(&card->transactions_lock) { int tlabel; =20 - tlabel =3D card->current_tlabel; - while (card->tlabel_mask & (1ULL << tlabel)) { + lockdep_assert_held(&card->transactions.lock); + + tlabel =3D card->transactions.current_tlabel; + while (card->transactions.tlabel_mask & (1ULL << tlabel)) { tlabel =3D (tlabel + 1) & 0x3f; - if (tlabel =3D=3D card->current_tlabel) + if (tlabel =3D=3D card->transactions.current_tlabel) return -EBUSY; } =20 - card->current_tlabel =3D (tlabel + 1) & 0x3f; - card->tlabel_mask |=3D 1ULL << tlabel; + card->transactions.current_tlabel =3D (tlabel + 1) & 0x3f; + card->transactions.tlabel_mask |=3D 1ULL << tlabel; =20 return tlabel; } @@ -331,7 +336,6 @@ void __fw_send_request(struct fw_card *card, struct fw_= transaction *t, int tcode void *payload, size_t length, union fw_transaction_callback callback, bool with_tstamp, void *callback_data) { - unsigned long flags; int tlabel; =20 /* @@ -339,11 +343,11 @@ void __fw_send_request(struct fw_card *card, struct f= w_transaction *t, int tcode * the list while holding the card spinlock. */ =20 - spin_lock_irqsave(&card->lock, flags); - - tlabel =3D allocate_tlabel(card); + // 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) + tlabel =3D allocate_tlabel(card); if (tlabel < 0) { - spin_unlock_irqrestore(&card->lock, flags); if (!with_tstamp) { callback.without_tstamp(card, RCODE_SEND_ERROR, NULL, 0, callback_data); } else { @@ -368,15 +372,22 @@ void __fw_send_request(struct fw_card *card, struct f= w_transaction *t, int tcode t->callback =3D callback; t->with_tstamp =3D with_tstamp; t->callback_data =3D callback_data; - - fw_fill_request(&t->packet, tcode, t->tlabel, destination_id, card->node_= id, generation, - speed, offset, payload, length); t->packet.callback =3D transmit_complete_callback; =20 - list_add_tail(&t->link, &card->transaction_list); + // 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 SelfIDCompl= ete. + fw_fill_request(&t->packet, tcode, t->tlabel, destination_id, card->node= _id, + generation, speed, offset, payload, length); + } =20 - spin_unlock_irqrestore(&card->lock, flags); + // 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) + list_add_tail(&t->link, &card->transactions.list); =20 + // Safe with no lock, since the index field of fw_card is immutable once = assigned. trace_async_request_outbound_initiate((uintptr_t)t, card->index, generati= on, speed, t->packet.header, payload, tcode_is_read_request(tcode) ? 0 : length / 4); @@ -1111,12 +1122,14 @@ void fw_core_handle_response(struct fw_card *card, = struct fw_packet *p) break; } =20 - scoped_guard(spinlock_irqsave, &card->lock) { - list_for_each_entry(iter, &card->transaction_list, link) { + // 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) { + list_for_each_entry(iter, &card->transactions.list, link) { if (iter->node_id =3D=3D source && iter->tlabel =3D=3D tlabel) { if (try_cancel_split_timeout(iter)) { list_del_init(&iter->link); - card->tlabel_mask &=3D ~(1ULL << iter->tlabel); + card->transactions.tlabel_mask &=3D ~(1ULL << iter->tlabel); t =3D iter; } break; diff --git a/include/linux/firewire.h b/include/linux/firewire.h index aeb71c39e57e..8d6801cf2fca 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -88,11 +88,15 @@ struct fw_card { =20 int node_id; int generation; - int current_tlabel; - u64 tlabel_mask; - struct list_head transaction_list; u64 reset_jiffies; =20 + struct { + int current_tlabel; + u64 tlabel_mask; + struct list_head list; + spinlock_t lock; + } transactions; + u32 split_timeout_hi; u32 split_timeout_lo; unsigned int split_timeout_cycles; --=20 2.48.1