From: Andy Grover <agrover@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: "Nicholas A. Bellinger" <nab@linux-iscsi.org>,
target-devel <target-devel@vger.kernel.org>,
linux-scsi <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH 1/2] target: Add transport_handle_cdb_direct optimization
Date: Thu, 16 Jun 2011 15:29:30 -0700 [thread overview]
Message-ID: <4DFA83CA.60600@redhat.com> (raw)
In-Reply-To: <20110616192256.GA14124@lst.de>
On 06/16/2011 12:22 PM, Christoph Hellwig wrote:
> On Thu, Jun 16, 2011 at 12:13:26PM -0700, Andy Grover wrote:
>> I agree. Also, I don't think there should be a handle_cdb_direct because
>> I think handle_cdb should call transport_generic_new_cmd, we don't need
>> a "direct" version. transport_generic_new_cmd should be safe (or made
>> safe) for calling from interrupt context. There's nothing in it that
>> demands it be executed in the backstore thread's context, and doing so
>> just incurs a two context switch latency penalty.
>
> I think the whole target context/threading model needs a makeover. As
> we need to switch to a usermode context for just about any operation
> we should just declare that any fabrict module must call all entry points
> from user context. The fabric modules can trivially do that using the
> concurrency managed workqueues if nessecary. At that point the target core
> can just go ahead by itself without bothering with context switches, and
> just using another workqueue where it might need additional concurreny
> (e.g. executing multiple task is parallel for the file backend)
I don't think we should be making it easy on the core, I think we should
be making it easy on the *fabrics*, if for no other reason that there
are >1 of them, but only one core. Less code duplicated.
Furthermore, many commands can be handled and generate a response
without submitting a read/write request to the backstores, which is the
only part that may need to sleep. If we decide all fabric calls to the
core must be from a thread, then all fabrics that could've gotten a
response to a command without context switching would then be the ones
taking unneeded context switches.
It's not clear to me yet what the barriers to fabrics calling core APIs
from interrupt are at this point. Do we just need to avoid GFP_KERNEL
allocs? The architecture as-is may be pretty close to doing this, and
then we'd be close to a reasonable framework.
Regards -- Andy
next prev parent reply other threads:[~2011-06-16 22:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-04 6:01 [PATCH 0/2] target/iscsi: Convert to RX context task mapping+queueing Nicholas A. Bellinger
2011-06-04 6:01 ` [PATCH 1/2] target: Add transport_handle_cdb_direct optimization Nicholas A. Bellinger
2011-06-04 14:03 ` Christoph Hellwig
2011-06-16 19:13 ` Andy Grover
2011-06-16 19:22 ` Christoph Hellwig
2011-06-16 22:29 ` Andy Grover [this message]
2011-06-17 12:01 ` Christoph Hellwig
2011-06-17 14:41 ` Christoph Hellwig
2011-06-04 6:01 ` [PATCH 2/2] iscsi-target: Convert to cmdsn_mutex and transport_handle_cdb_direct usage 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=4DFA83CA.60600@redhat.com \
--to=agrover@redhat.com \
--cc=hch@lst.de \
--cc=linux-scsi@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox