From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: Chris Boot <bootc@bootc.net>
Cc: linux1394-devel@lists.sourceforge.net,
target-devel@vger.kernel.org, linux-kernel@vger.kernel.org,
agrover@redhat.com, clemens@ladisch.de, nab@linux-iscsi.org
Subject: Re: [PATCH 08/11] firewire-sbp-target: Add sbp_login.{c,h}
Date: Sat, 14 Apr 2012 12:17:43 +0200 [thread overview]
Message-ID: <20120414121743.30d46a77@stein> (raw)
In-Reply-To: <1334154043-69076-9-git-send-email-bootc@bootc.net>
On Apr 11 Chris Boot wrote:
> +static void session_check_for_reset(struct sbp_session *sess)
> +{
> + bool card_valid = false;
> +
> + spin_lock_bh(&sess->lock);
> +
> + if (sess->card) {
> + spin_lock_irq(&sess->card->lock);
> + card_valid = (sess->card->local_node != NULL);
> + spin_unlock_irq(&sess->card->lock);
> +
> + if (!card_valid) {
> + fw_card_put(sess->card);
> + sess->card = NULL;
> + }
> + }
> +
> + if (!card_valid || (sess->generation != sess->card->generation)) {
> + pr_info("Waiting for reconnect from node: %016llx\n",
> + sess->guid);
> +
> + sess->node_id = -1;
> + sess->reconnect_expires = get_jiffies_64() +
> + ((sess->reconnect_hold + 1) * HZ);
> + }
> +
> + spin_unlock_bh(&sess->lock);
> +}
The card->local_node != NULL test by itself is atomic, it does not benefit
from being wrapped by card->lock acquisition.
Well, OK, the lock effectively forces the compiler to determine the value
of card_valid only once. If the lock weren't there I guess the compiler
might feel entitled to reload card->local_node in the second !card_valid
test. But even if you lose ACCESS_ONCE behavior by removing the card->lock
acquisition, I can't see how that could be detrimental relative to the
current code.
I am wondering on the other hand if there isn't actually a dependency
between this local_node test and something else, e.g. the generation test.
I.e. might the locking be incomplete? Not sure about that. I think I
rather want to look at that again when I received the code through
mainline.
BTW "card_valid" sounds rather generic; maybe call it "topology_valid"?
Either way, it can turn valid or invalid any time when firewire-core gets
to handle a self-ID-complete event.
--
Stefan Richter
-=====-===-- -=-- -===-
http://arcgraph.de/sr/
next prev parent reply other threads:[~2012-04-14 10:18 UTC|newest]
Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-17 14:51 FireWire/SBP2 Target mode Chris Boot
2011-08-17 18:57 ` Stefan Richter
2011-08-18 16:19 ` Clemens Ladisch
2012-02-01 19:50 ` Andy Grover
2012-02-01 21:41 ` Stefan Richter
2012-02-02 9:22 ` Boaz Harrosh
2012-02-02 10:09 ` Clemens Ladisch
2012-02-06 13:13 ` Chris Boot
2012-02-06 14:43 ` Clemens Ladisch
2012-02-06 14:51 ` Chris Boot
2012-02-06 20:26 ` Stefan Richter
2012-02-06 22:28 ` Chris Boot
2012-02-06 23:00 ` Julian Calaby
2012-02-06 23:09 ` Chris Boot
2012-02-07 7:38 ` Chris Boot
2012-02-07 10:06 ` Julian Calaby
2012-02-07 19:17 ` Stefan Richter
2012-02-07 19:53 ` Chris Boot
2012-02-11 19:43 ` [RFC][PATCH 00/13] firewire-sbp-target: FireWire SBP-2 SCSI target Chris Boot
2012-02-11 19:44 ` [PATCH 01/13] firewire: Add function to get speed from opaque struct fw_request Chris Boot
2012-02-11 19:44 ` [PATCH 02/13] firewire: Add EXPORT_SYMBOL_GPL(fw_card_release) Chris Boot
2012-02-11 19:44 ` [PATCH 03/13] firewire-sbp-target: Add Kconfig, Makefile and TODO Chris Boot
2012-02-13 12:50 ` Nicholas A. Bellinger
2012-02-11 19:44 ` [PATCH 04/13] firewire-sbp-target: Add sbp_base.h header Chris Boot
2012-02-11 19:44 ` [PATCH 05/13] firewire-sbp-target: Add sbp_configfs.c Chris Boot
2012-02-11 19:44 ` [PATCH 06/13] firewire-sbp-target: Add sbp_fabric.{c,h} Chris Boot
2012-02-13 13:06 ` Nicholas A. Bellinger
[not found] ` <337FFBD7-6B4A-41CA-BB57-6038C935B5BF@bootc.net>
2012-02-13 19:53 ` Stefan Richter
2012-02-13 22:41 ` Nicholas A. Bellinger
2012-02-11 19:44 ` [PATCH 07/13] firewire-sbp-target: Add sbp_proto.{c,h} Chris Boot
2012-02-11 19:44 ` [PATCH 08/13] firewire-sbp-target: add sbp_management_agent.{c,h} Chris Boot
2012-02-11 19:44 ` [PATCH 09/13] firewire-sbp-target: Add sbp_login.{c,h} Chris Boot
2012-02-11 19:44 ` [PATCH 10/13] firewire-sbp-target: Add sbp_target_agent.{c,h} Chris Boot
2012-02-11 19:44 ` [PATCH 11/13] firewire-sbp-target: Add sbp_scsi_cmnd.{c,h} Chris Boot
2012-02-11 19:44 ` [PATCH 12/13] firewire-sbp-target: Add sbp_util.{c,h} Chris Boot
2012-02-11 19:44 ` [PATCH 13/13] firewire-sbp-target: Add to target Kconfig and Makefile Chris Boot
2012-02-12 14:12 ` [RFC][PATCH 00/13] firewire-sbp-target: FireWire SBP-2 SCSI target Stefan Richter
2012-02-12 15:13 ` Chris Boot
2012-02-12 16:16 ` Stefan Richter
2012-02-15 14:47 ` [PATCH v2 00/11] " Chris Boot
2012-02-15 14:47 ` [PATCH v2 01/11] firewire: Add function to get speed from opaque struct fw_request Chris Boot
2012-02-15 19:09 ` Stefan Richter
2012-02-15 19:10 ` Chris Boot
2012-02-15 22:01 ` Stefan Richter
2012-02-16 9:12 ` Chris Boot
2012-02-15 14:47 ` [PATCH v2 02/11] firewire: Move fw_card kref functions into linux/firewire.h Chris Boot
2012-02-15 19:10 ` Stefan Richter
2012-02-16 9:18 ` Chris Boot
2012-02-15 14:47 ` [PATCH v2 03/11] firewire-sbp-target: Add Kconfig, Makefile and TODO Chris Boot
2012-02-15 14:47 ` [PATCH v2 04/11] firewire-sbp-target: Add sbp_base.h header Chris Boot
2012-02-15 19:15 ` Stefan Richter
2012-02-16 9:55 ` Chris Boot
2012-02-15 14:47 ` [PATCH v2 05/11] firewire-sbp-target: Add sbp_configfs.c Chris Boot
2012-02-15 19:21 ` Stefan Richter
2012-02-16 9:57 ` Chris Boot
2012-02-16 13:48 ` Stefan Richter
2012-02-15 14:47 ` [PATCH v2 06/11] firewire-sbp-target: Add sbp_fabric.{c,h} Chris Boot
2012-02-15 14:47 ` [PATCH v2 07/11] firewire-sbp-target: add sbp_management_agent.{c,h} Chris Boot
2012-02-15 19:48 ` Stefan Richter
2012-02-16 10:28 ` Chris Boot
2012-02-16 14:12 ` Stefan Richter
2012-02-15 14:47 ` [PATCH v2 08/11] firewire-sbp-target: Add sbp_login.{c,h} Chris Boot
2012-02-15 21:00 ` Stefan Richter
2012-02-16 11:21 ` Chris Boot
2012-03-03 17:37 ` Stefan Richter
2012-03-15 17:48 ` Paul E. McKenney
2012-02-15 14:47 ` [PATCH v2 09/11] firewire-sbp-target: Add sbp_target_agent.{c,h} Chris Boot
2012-02-15 21:27 ` Stefan Richter
2012-02-16 11:25 ` Chris Boot
2012-02-18 14:59 ` Stefan Richter
2012-02-18 15:05 ` Chris Boot
2012-02-15 14:47 ` [PATCH v2 10/11] firewire-sbp-target: Add sbp_scsi_cmnd.{c,h} Chris Boot
2012-02-15 14:47 ` [PATCH v2 11/11] firewire-sbp-target: Add to target Kconfig and Makefile Chris Boot
2012-04-11 14:20 ` [PATCH v3 00/11] firewire-sbp-target: FireWire SBP-2 SCSI target Chris Boot
2012-04-11 14:20 ` [PATCH 01/11] firewire: Add function to get speed from opaque struct fw_request Chris Boot
2012-04-11 14:20 ` [PATCH 02/11] firewire: Move fw_card kref functions into linux/firewire.h Chris Boot
2012-04-11 14:20 ` [PATCH 03/11] firewire-sbp-target: Add Kconfig, Makefile and TODO Chris Boot
2012-04-11 14:20 ` [PATCH 04/11] firewire-sbp-target: Add sbp_base.h header Chris Boot
2012-04-11 14:20 ` [PATCH 05/11] firewire-sbp-target: Add sbp_configfs.c Chris Boot
2012-04-11 14:20 ` [PATCH 06/11] firewire-sbp-target: Add sbp_fabric.{c,h} Chris Boot
2012-04-11 14:20 ` [PATCH 07/11] firewire-sbp-target: Add sbp_management_agent.{c,h} Chris Boot
2012-04-11 14:20 ` [PATCH 08/11] firewire-sbp-target: Add sbp_login.{c,h} Chris Boot
2012-04-14 10:17 ` Stefan Richter [this message]
2012-04-11 14:20 ` [PATCH 09/11] firewire-sbp-target: Add sbp_target_agent.{c,h} Chris Boot
2012-04-14 10:49 ` Stefan Richter
2012-04-14 11:33 ` Stefan Richter
2012-04-11 14:20 ` [PATCH 10/11] firewire-sbp-target: Add sbp_scsi_cmnd.{c,h} Chris Boot
2012-04-11 14:20 ` [PATCH 11/11] firewire-sbp-target: Add to target Kconfig and Makefile Chris Boot
2012-04-12 21:02 ` [PATCH v3 00/11] firewire-sbp-target: FireWire SBP-2 SCSI target Andy Grover
2012-04-13 3:03 ` Nicholas A. Bellinger
2012-04-13 13:16 ` Chris Boot
2012-04-14 1:23 ` Nicholas A. Bellinger
2012-04-14 9:12 ` [PATCH 0/2] sbp-target: cleanup after merge into single file Chris Boot
2012-04-14 9:12 ` [PATCH 1/2] sbp-target: minor cleanups after merging " Chris Boot
2012-04-14 9:12 ` [PATCH 2/2] sbp-target: update TODO file Chris Boot
2012-04-14 21:44 ` [PATCH 0/2] sbp-target: cleanup after merge into single file Nicholas A. Bellinger
2012-04-14 23:11 ` Stefan Richter
2012-04-15 1:22 ` Nicholas A. Bellinger
2012-04-17 10:48 ` [PATCH v3 00/11] firewire-sbp-target: FireWire SBP-2 SCSI target Chris Boot
2012-04-18 7:17 ` Nicholas A. Bellinger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120414121743.30d46a77@stein \
--to=stefanr@s5r6.in-berlin.de \
--cc=agrover@redhat.com \
--cc=bootc@bootc.net \
--cc=clemens@ladisch.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux1394-devel@lists.sourceforge.net \
--cc=nab@linux-iscsi.org \
--cc=target-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).