From: "Jörn Engel" <joern@logfs.org>
To: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
Cc: linux-kernel@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
target-devel <target-devel@vger.kernel.org>
Subject: Re: [PATCH 3/3] target: simplify target_wait_for_sess_cmds()
Date: Tue, 14 May 2013 12:29:39 -0400 [thread overview]
Message-ID: <20130514162939.GA32463@logfs.org> (raw)
In-Reply-To: <1368500924.11576.16.camel@haakon3.risingtidesystems.com>
On Mon, 13 May 2013 20:08:44 -0700, Nicholas A. Bellinger wrote:
> On Mon, 2013-05-13 at 18:00 -0400, Jörn Engel wrote:
> >
> > I agree that the overhead doesn't matter. The msleep(100) spells this
> > out rather explicitly. What does matter is that a) the patch retains
> > old behaviour with much simpler code and b) it fixes a race that kills
> > the machine. I can live without a, but very much want to keep b. ;)
>
> Fucking around with ->sess_cmd_lock during each loop of ->sess_cmd_list
> in target_wait_for_sess_cmds is not simpler code..
I could argue that fucking around with ->sess_cmd_lock during each
loop is simpler than the communication through cmd_wait_set and
cmd_wait_comp. But simplicity is ultimately subjective and we can
argue all day.
drivers/infiniband/ulp/srpt/ib_srpt.c | 2 +-
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2 +-
drivers/target/target_core_transport.c | 64 +++++++++-----------------------
include/target/target_core_base.h | 2 -
include/target/target_core_fabric.h | 2 +-
5 files changed, 20 insertions(+), 52 deletions(-)
But diffstat is reasonably objective. Do you really want me to come
up with an alternative patch that adds code instead of removing it?
Jörn
--
Simplicity is prerequisite for reliability.
-- Edsger W. Dijkstra
next prev parent reply other threads:[~2013-05-14 17:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-13 20:30 [PATCH 0/3] target: Fix two races leading to use-after-free Joern Engel
2013-05-13 20:30 ` [PATCH 1/3] target: removed unused transport_state flag Joern Engel
2013-05-13 20:30 ` [PATCH 2/3] target: close target_put_sess_cmd() vs. core_tmr_abort_task() race v5 Joern Engel
2013-05-13 23:02 ` Nicholas A. Bellinger
2013-05-14 19:04 ` Greg Kroah-Hartman
2013-05-15 3:07 ` Nicholas A. Bellinger
2013-05-13 20:30 ` [PATCH 3/3] target: simplify target_wait_for_sess_cmds() Joern Engel
2013-05-13 23:00 ` Nicholas A. Bellinger
2013-05-13 22:00 ` Jörn Engel
2013-05-14 3:08 ` Nicholas A. Bellinger
2013-05-14 16:29 ` Jörn Engel [this message]
2013-05-15 3:05 ` Nicholas A. Bellinger
2013-05-15 2:19 ` Jörn Engel
2013-05-15 6:05 ` 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=20130514162939.GA32463@logfs.org \
--to=joern@logfs.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.