Discussion:
[PATCH 2.6.25.1] Add scsi_execute_async_fifo()
Bart Van Assche
2008-05-02 14:38:42 UTC
Permalink
The patch below implements the following:
* Defines a new internal function __scsi_execute_async(). The only difference
with the existing function scsi_execute_async() is that this new function
has an extra parameter, at_head, which allows to specify whether to insert
the new request at the head or at the tail of the queue.
* Replaces the implementation of scsi_execute_async() by a call to the new
function __scsi_execute_async(). The behavior of this function is not
changed.
* Defines a new function scsi_execute_async_fifo() which inserts a request at
the tail, in FIFO order.
* Adds #define SCSI_EXEC_REQ_FIFO_DEFINED such that out-of-tree kernel
modules can easily find out whether or not the new function
scsi_execute_async_fifo() is present.

Signed-off-by: ***@gmail.com


diff -uprN -X linux-2.6.25.1/Documentation/dontdiff
orig/linux-2.6.25.1/drivers/scsi/scsi_lib.c
linux-2.6.25.1/drivers/scsi/scsi_lib.c
--- orig/linux-2.6.25.1/drivers/scsi/scsi_lib.c 2008-05-01 23:45:25.000000000
+0200
+++ linux-2.6.25.1/drivers/scsi/scsi_lib.c 2008-05-02 15:26:46.000000000 +0200
@@ -363,7 +363,7 @@ free_bios:
}

/**
- * scsi_execute_async - insert request
+ * __scsi_execute_async - insert request
* @sdev: scsi device
* @cmd: scsi command
* @cmd_len: length of scsi cdb
@@ -376,11 +376,14 @@ free_bios:
* @privdata: data passed to done()
* @done: callback function when done
* @gfp: memory allocation flags
+ * @at_head: insert request at head or tail of queue
*/
-int scsi_execute_async(struct scsi_device *sdev, const unsigned char *cmd,
+static inline int __scsi_execute_async(struct scsi_device *sdev,
+ const unsigned char *cmd,
int cmd_len, int data_direction, void *buffer, unsigned bufflen,
int use_sg, int timeout, int retries, void *privdata,
- void (*done)(void *, char *, int, int), gfp_t gfp)
+ void (*done)(void *, char *, int, int), gfp_t gfp,
+ int at_head)
{
struct request *req;
struct scsi_io_context *sioc;
@@ -417,7 +420,7 @@ int scsi_execute_async(struct scsi_devic
sioc->data = privdata;
sioc->done = done;

- blk_execute_rq_nowait(req->q, NULL, req, 1, scsi_end_async);
+ blk_execute_rq_nowait(req->q, NULL, req, at_head, scsi_end_async);
return 0;

free_req:
@@ -426,8 +429,57 @@ free_sense:
kmem_cache_free(scsi_io_context_cache, sioc);
return DRIVER_ERROR << 24;
}
+
+/**
+ * scsi_execute_async - insert request
+ * @sdev: scsi device
+ * @cmd: scsi command
+ * @cmd_len: length of scsi cdb
+ * @data_direction: data direction
+ * @buffer: data buffer (this can be a kernel buffer or scatterlist)
+ * @bufflen: len of buffer
+ * @use_sg: if buffer is a scatterlist this is the number of elements
+ * @timeout: request timeout in seconds
+ * @retries: number of times to retry request
+ * @flags: or into request flags
+ **/
+int scsi_execute_async(struct scsi_device *sdev, const unsigned char *cmd,
+ int cmd_len, int data_direction, void *buffer,
+ unsigned bufflen, int use_sg, int timeout, int retries,
+ void *privdata, void (*done)(void *, char *, int, int),
+ gfp_t gfp)
+{
+ return __scsi_execute_async(sdev, cmd, cmd_len, data_direction, buffer,
+ bufflen, use_sg, timeout, retries, privdata,
+ done, gfp, 1);
+}
EXPORT_SYMBOL_GPL(scsi_execute_async);

+/**
+ * scsi_execute_async_fifo - insert request at tail, in FIFO order
+ * @sdev: scsi device
+ * @cmd: scsi command
+ * @cmd_len: length of scsi cdb
+ * @data_direction: data direction
+ * @buffer: data buffer (this can be a kernel buffer or scatterlist)
+ * @bufflen: len of buffer
+ * @use_sg: if buffer is a scatterlist this is the number of elements
+ * @timeout: request timeout in seconds
+ * @retries: number of times to retry request
+ * @flags: or into request flags
+ **/
+int scsi_execute_async_fifo(struct scsi_device *sdev, const unsigned char
*cmd,
+ int cmd_len, int data_direction, void *buffer,
+ unsigned bufflen, int use_sg, int timeout,
+ int retries, void *privdata,
+ void (*done)(void *, char *, int, int), gfp_t gfp)
+{
+ return __scsi_execute_async(sdev, cmd, cmd_len, data_direction, buffer,
+ bufflen, use_sg, timeout, retries, privdata,
+ done, gfp, 0);
+}
+EXPORT_SYMBOL_GPL(scsi_execute_async_fifo);
+
/*
* Function: scsi_init_cmd_errh()
*
diff -uprN -X linux-2.6.25.1/Documentation/dontdiff
orig/linux-2.6.25.1/include/scsi/scsi_device.h
linux-2.6.25.1/include/scsi/scsi_device.h
--- orig/linux-2.6.25.1/include/scsi/scsi_device.h 2008-05-01
23:45:25.000000000 +0200
+++ linux-2.6.25.1/include/scsi/scsi_device.h 2008-05-02 15:28:29.000000000
+0200
@@ -332,6 +332,14 @@ extern int scsi_execute_async(struct scs
int timeout, int retries, void *privdata,
void (*done)(void *, char *, int, int),
gfp_t gfp);
+#define SCSI_EXEC_REQ_FIFO_DEFINED
+extern int scsi_execute_async_fifo(struct scsi_device *sdev,
+ const unsigned char *cmd, int cmd_len,
+ int data_direction, void *buffer,
+ unsigned bufflen, int use_sg,
+ int timeout, int retries, void *privdata,
+ void (*done)(void *, char *, int, int),
+ gfp_t gfp);

static inline int __must_check scsi_device_reprobe(struct scsi_device *sdev)
{
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2008-05-02 15:33:06 UTC
Permalink
NACK.

First scsi_execute_async has a horrible API and thus it's on it's way
out, no way we'll add another variant of it.

Second we're not going to add things to the core just for out of tree
modules.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-02 15:53:22 UTC
Permalink
Post by Christoph Hellwig
NACK.
First scsi_execute_async has a horrible API and thus it's on it's way
out, no way we'll add another variant of it.
Second we're not going to add things to the core just for out of tree
modules.
Regarding out-of-tree modules: this is just a preparatory step before
submitting SCST for inclusion in the mainstream kernel.

Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2008-05-02 15:55:25 UTC
Permalink
Post by Bart Van Assche
Regarding out-of-tree modules: this is just a preparatory step before
submitting SCST for inclusion in the mainstream kernel.
And what crackpipe did you smoke to thing we'd put duplicated target
framework in?
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
Post by Bart Van Assche
Bart.
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-02 16:06:59 UTC
Permalink
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding out-of-tree modules: this is just a preparatory step before
submitting SCST for inclusion in the mainstream kernel.
And what crackpipe did you smoke to thing we'd put duplicated target
framework in?
Why are you so aggressive ? I didn't insult you in any way.

Regarding inclusion of SCSI target code in the mainline, this subject
has already been discussed extensively in the past
(http://lkml.org/lkml/2008/1/23/134). The conclusion was clear: SCST
is faster than any other existing iSCSI target for Linux (IET, STGT,
LIO), stable, well maintained and the most standards compliant target.
Why do you want to reopen this discussion ?

James Bottomley clearly expressed in that thread that he doesn't want
to maintain two SCSI target frameworks. So what I propose is that SCST
is included in the mainline and afterwards that it is evaluated
whether or not it is desirable to keep other target code in the
mainline kernel.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Matthew Wilcox
2008-05-02 16:16:29 UTC
Permalink
Post by Bart Van Assche
James Bottomley clearly expressed in that thread that he doesn't want
to maintain two SCSI target frameworks. So what I propose is that SCST
is included in the mainline and afterwards that it is evaluated
whether or not it is desirable to keep other target code in the
mainline kernel.
That's not the way it works, sorry.

The way to get SCST in is to work with the people who care about target
frameworks (which doesn't include me, fwiw). You come to a consensus
about the way to proceed. Normally this will be a gradual migration of
the good bits from SCST into the kernel. In *exceptional* circumstances,
we've replaced one piece of infrastructure with another (eg wireless
midlayers), but those are by no means the preferred ways to go.

Let me just re-emphasise this bit. You HAVE to work with the existing
people. If you can't come to a common understanding, your code won't
get in. Even if it's better.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-02 16:23:08 UTC
Permalink
Post by Matthew Wilcox
Post by Bart Van Assche
James Bottomley clearly expressed in that thread that he doesn't want
to maintain two SCSI target frameworks. So what I propose is that SCST
is included in the mainline and afterwards that it is evaluated
whether or not it is desirable to keep other target code in the
mainline kernel.
That's not the way it works, sorry.
The way to get SCST in is to work with the people who care about target
frameworks (which doesn't include me, fwiw). You come to a consensus
about the way to proceed. Normally this will be a gradual migration of
the good bits from SCST into the kernel. In *exceptional* circumstances,
we've replaced one piece of infrastructure with another (eg wireless
midlayers), but those are by no means the preferred ways to go.
Let me just re-emphasise this bit. You HAVE to work with the existing
people. If you can't come to a common understanding, your code won't
get in. Even if it's better.
Which target code is already in the mainline kernel, and who are the
maintainers for this code ? I checked the MAINTAINERS file in the
2.6.25 tarball, but in this file I could not find the information I
was looking for.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
James Bottomley
2008-05-02 16:30:09 UTC
Permalink
Post by Bart Van Assche
Post by Matthew Wilcox
Post by Bart Van Assche
James Bottomley clearly expressed in that thread that he doesn't want
to maintain two SCSI target frameworks. So what I propose is that SCST
is included in the mainline and afterwards that it is evaluated
whether or not it is desirable to keep other target code in the
mainline kernel.
That's not the way it works, sorry.
The way to get SCST in is to work with the people who care about target
frameworks (which doesn't include me, fwiw). You come to a consensus
about the way to proceed. Normally this will be a gradual migration of
the good bits from SCST into the kernel. In *exceptional* circumstances,
we've replaced one piece of infrastructure with another (eg wireless
midlayers), but those are by no means the preferred ways to go.
Let me just re-emphasise this bit. You HAVE to work with the existing
people. If you can't come to a common understanding, your code won't
get in. Even if it's better.
Which target code is already in the mainline kernel, and who are the
maintainers for this code ? I checked the MAINTAINERS file in the
2.6.25 tarball, but in this file I could not find the information I
was looking for.
Um, you claimed to be running benchmarks against it in the thread you
just quoted to push for SCST inclusion. If you genuinely don't know
which code is actually in the kernel, what exactly were the benchmarks
against?

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-02 16:43:22 UTC
Permalink
On Fri, May 2, 2008 at 6:30 PM, James Bottomley
Post by James Bottomley
Um, you claimed to be running benchmarks against it in the thread you
just quoted to push for SCST inclusion. If you genuinely don't know
which code is actually in the kernel, what exactly were the benchmarks
against?
This is getting off-topic. To answer your question: for downloading,
installing and benchmarking an iSCSI target implementation, you don't
have to know the names of any kernel source files. Knowledge of the
name of the relevant kernel module is enough.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Matthew Wilcox
2008-05-02 16:49:06 UTC
Permalink
Post by Bart Van Assche
Which target code is already in the mainline kernel, and who are the
maintainers for this code ? I checked the MAINTAINERS file in the
2.6.25 tarball, but in this file I could not find the information I
was looking for.
Can you really not locate drivers/scsi/scsi_tgt_lib.c in the tree? I
appreciate that there's a lot of files in the drivers/scsi/ directory,
but c'mon. I had no idea what it was called and it took about 30
seconds to find them once I realised you wanted all the work to be done
for you.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-02 16:57:10 UTC
Permalink
Post by Matthew Wilcox
Can you really not locate drivers/scsi/scsi_tgt_lib.c in the tree? I
appreciate that there's a lot of files in the drivers/scsi/ directory,
but c'mon. I had no idea what it was called and it took about 30
seconds to find them once I realised you wanted all the work to be done
for you.
Thanks for looking this up. Would it be an acceptable approach if the
SCST code is modified such that it calls the code inside
drivers/scsi/scsi_tgt_lib.c whenever this is appropriate ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Matthew Wilcox
2008-05-02 17:02:44 UTC
Permalink
Post by Bart Van Assche
Thanks for looking this up. Would it be an acceptable approach if the
SCST code is modified such that it calls the code inside
drivers/scsi/scsi_tgt_lib.c whenever this is appropriate ?
You need to talk to Mike and Tomo.
Post by Bart Van Assche
From my point of view, the most important bit is the target layer <->
driver API. ie the transfer_response() method in the host template.
I'm sure other people have their own preferences though.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-02 18:21:08 UTC
Permalink
Post by Matthew Wilcox
Post by Bart Van Assche
Thanks for looking this up. Would it be an acceptable approach if the
SCST code is modified such that it calls the code inside
drivers/scsi/scsi_tgt_lib.c whenever this is appropriate ?
You need to talk to Mike and Tomo.
Post by Bart Van Assche
From my point of view, the most important bit is the target layer <->
driver API. ie the transfer_response() method in the host template.
I'm sure other people have their own preferences though.
SCST can't use anything from STGT or other SCSI targets support
functionality of the kernel. But it can integrate inside something from
transport attributes. See my e-mail with subject "SCSI target subsystem"
why.

Vlad


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
James Bottomley
2008-05-02 16:18:04 UTC
Permalink
Post by Bart Van Assche
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding out-of-tree modules: this is just a preparatory step before
submitting SCST for inclusion in the mainstream kernel.
And what crackpipe did you smoke to thing we'd put duplicated target
framework in?
Why are you so aggressive ? I didn't insult you in any way.
Regarding inclusion of SCSI target code in the mainline, this subject
has already been discussed extensively in the past
(http://lkml.org/lkml/2008/1/23/134). The conclusion was clear: SCST
is faster than any other existing iSCSI target for Linux (IET, STGT,
LIO), stable, well maintained and the most standards compliant target.
Why do you want to reopen this discussion ?
That's an interesting rewrite of history. The evidence you presented
showed fairly identical results apart from on one contrived IB benchmark
that couldn't directly compare the two.

I'm also on record in the thread saying that was insufficient proof for
me to justify throwing STGT out and replacing it with SCST.
Post by Bart Van Assche
James Bottomley clearly expressed in that thread that he doesn't want
to maintain two SCSI target frameworks. So what I propose is that SCST
is included in the mainline and afterwards that it is evaluated
whether or not it is desirable to keep other target code in the
mainline kernel.
That's hardly sufficient. STGT is already in use. Their either has to
be a migration path or, the preferred option, take the pieces of SCST
that are actual improvements and embed them in STGT.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-02 16:39:05 UTC
Permalink
On Fri, May 2, 2008 at 6:18 PM, James Bottomley
Post by James Bottomley
Post by Bart Van Assche
Regarding inclusion of SCSI target code in the mainline, this subject
has already been discussed extensively in the past
(http://lkml.org/lkml/2008/1/23/134). The conclusion was clear: SCST
is faster than any other existing iSCSI target for Linux (IET, STGT,
LIO), stable, well maintained and the most standards compliant target.
Why do you want to reopen this discussion ?
That's an interesting rewrite of history. The evidence you presented
showed fairly identical results apart from on one contrived IB benchmark
that couldn't directly compare the two.
I'm also on record in the thread saying that was insufficient proof for
me to justify throwing STGT out and replacing it with SCST.
The trend of the measurements I did was very clear: SCST has a higher
bandwidth and a lower latency than the other three iSCSI
implementations I benchmarked. While this difference is neglectible
for 100 Mbit/s media, the difference is significant for 10 Gbit/s
media. One of the tests I did was with iSCSI via IPoIB. These results
allow direct comparison between STGT and SCST.
Post by James Bottomley
Post by Bart Van Assche
James Bottomley clearly expressed in that thread that he doesn't want
to maintain two SCSI target frameworks. So what I propose is that SCST
is included in the mainline and afterwards that it is evaluated
whether or not it is desirable to keep other target code in the
mainline kernel.
That's hardly sufficient. STGT is already in use. Their either has to
be a migration path or, the preferred option, take the pieces of SCST
that are actual improvements and embed them in STGT.
In the thread I referred to it has been explained that for optimal
speed and minimum latency a kernelspace implementation is required.
Since most of STGT is implemented in userspace, embedding pieces of
SCST in STGT is not an option.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-02 18:17:28 UTC
Permalink
Post by James Bottomley
Post by Bart Van Assche
James Bottomley clearly expressed in that thread that he doesn't want
to maintain two SCSI target frameworks. So what I propose is that SCST
is included in the mainline and afterwards that it is evaluated
whether or not it is desirable to keep other target code in the
mainline kernel.
That's hardly sufficient. STGT is already in use. Their either has to
be a migration path or, the preferred option, take the pieces of SCST
that are actual improvements and embed them in STGT.
Actually, between SCSI initiator and target subsystems there is almost
*nothing* in common. This claim, at first glance, looks pretty wrong,
because both serve SCSI, so they must have a lot common. But look deeper
and you quickly find out that the majority of functionality as well as
data they use are dedicated for each subsystem, not shared.

Just look at SCST/qla2x00t/(changes done in the initiator qla2xxx driver
to support target mode, patch attached): 90% of changes is adding
callbacks for external target add-on, the rest is support for older,
than 2.6.17, kernels and sysfs magic. Note, no data are common between
initiator and target parts in the meaning that they both use them.

Then look at SCST (http://scst.sf.net). It implements complete
pass-through SCSI support and look how it interacts with initiator SCSI
subsystem. It calls only 2 functions: FIFO version of
scsi_execute_async() (original scsi_execute_async() provides
unacceptable LIFO commands order) and scsi_reset_provider() for task
management. And there is only one common variable: struct scsi_device.
That's all! In other storage modes (FILEIO/BLOCKIO) there is nothing
common with SCSI initiator subsystem at all.

Finally, try to find out in SCST any duplicated functionality.

Now, let's look how SCSI target/initiator integration currently done in
the kernel. For me it looks pretty artificial. For example, if I make a
general purpose kernel, for which 1% of users would run target mode, I
would have to enable as module "SCSI target support" as well as SCSI
target support for transport attributes. Now 99% of users of my kernel,
who don't need SCSI target, but need SCSI initiator drivers, would have
to have scsi_tgt loaded, because transport attribute drivers would
depend on it:

# lsmod
Module Size Used by
qla2xxx 130844 0
firmware_class 8064 1 qla2xxx
scsi_transport_fc 40900 1 qla2xxx
scsi_tgt 12196 1 scsi_transport_fc
brd 6924 0
xfs 511280 1
dm_mirror 24368 0
dm_mod 51148 1 dm_mirror
uhci_hcd 21400 0
sg 31784 0
e1000 114536 0
pcspkr 3328 0

No target functionality needed, but target mode subsystem is needed. Is
it a good design?

I wrote all above to support my at first glance shocking conclusion that
SCSI target subsystem is completely new subsystem of the kernel and it
should live on its own with its own maintainer! This is the same as with
the current interaction between SCSI and block subsystems in the kernel:
SCSI uses block's functionality, but that doesn't mean that block and
SCSI are the same subsystem.

Thus, how IMHO initiator and target drivers should be written:

- All initiator drivers should live in the SCSI initiator subsystem
(aka current SCSI subsystem) only, the same as today.

- All target drivers should live in the SCSI target subsystem only and
be either add-ons to initiator drivers, like, e.g., qla2x00t, or be a
complete driver, like, e.g., iSCSI-SCST.

Vlad
Bart Van Assche
2008-05-03 09:41:41 UTC
Permalink
Post by Vladislav Bolkhovitin
I wrote all above to support my at first glance shocking conclusion that
SCSI target subsystem is completely new subsystem of the kernel and it
should live on its own with its own maintainer! This is the same as with the
current interaction between SCSI and block subsystems in the kernel: SCSI
uses block's functionality, but that doesn't mean that block and SCSI are
the same subsystem.
Hello Tomo,

Due to the IET and STGT projects you have a lot of experience with
implementing SCSI target frameworks. What is your opinion about how a
kernel space SCSI target framework should fit in the Linux kernel ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Matthew Wilcox
2008-05-03 09:53:14 UTC
Permalink
Post by Bart Van Assche
Post by Vladislav Bolkhovitin
I wrote all above to support my at first glance shocking conclusion that
SCSI target subsystem is completely new subsystem of the kernel and it
should live on its own with its own maintainer! This is the same as with the
current interaction between SCSI and block subsystems in the kernel: SCSI
uses block's functionality, but that doesn't mean that block and SCSI are
the same subsystem.
Hello Tomo,
Due to the IET and STGT projects you have a lot of experience with
implementing SCSI target frameworks. What is your opinion about how a
kernel space SCSI target framework should fit in the Linux kernel ?
Bart, what is your role in the SCST project? You don't seem to have
contributed any code to it (going by the SVN logs on sourceforge), and
your questions and suggestions seem to be those of someone not familiar
with the code. If you're not a developer, it might be more helpful for
you to step back and let Vladislav handle this.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-03 10:39:19 UTC
Permalink
Post by Matthew Wilcox
Bart, what is your role in the SCST project? You don't seem to have
contributed any code to it (going by the SVN logs on sourceforge), and
your questions and suggestions seem to be those of someone not familiar
with the code. If you're not a developer, it might be more helpful for
you to step back and let Vladislav handle this.
I'd like to see this discussion focus on the technical issues. Why are
people constantly throwing up political arguments in this discussion ?

To answer your question: as you noticed, I did not yet contribute any
code to the SCST project. I'm an iSCSI/iSER/SRP user. I'd like to have
a SCSI target framework available on Linux that is as fast as
possible, as reliable as possible, as standards compliant as possible,
well maintained and for which all kernel code is integrated in the
mainstream Linux kernel. I'm probably sharing this desire with all
STGT, IET, SCST and LIO users. Do you consider the opinion of SCSI
target framework users irrelevant in this discussion ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Matthew Wilcox
2008-05-03 13:28:46 UTC
Permalink
Post by Bart Van Assche
To answer your question: as you noticed, I did not yet contribute any
code to the SCST project. I'm an iSCSI/iSER/SRP user. I'd like to have
a SCSI target framework available on Linux that is as fast as
possible, as reliable as possible, as standards compliant as possible,
well maintained and for which all kernel code is integrated in the
mainstream Linux kernel. I'm probably sharing this desire with all
STGT, IET, SCST and LIO users. Do you consider the opinion of SCSI
target framework users irrelevant in this discussion ?
Yes.

Once Vladislav is ready for his project to be merged, let him approach
us and ask. I don't know if he considers his project ready for merging.
Did he ask you for his help? It's generally considered rather rude to
merge someone else's code without their consent. The way you tried to
get it merged was unhelpful and, if Vladislav were less skillful a
communicator, could have actively hindered getting it merged.

The correct way to have done this would have been to start a thread on
the scst mailing list asking whether the developers felt it ready to
merge. Then *leave it to them*.
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-03 14:48:08 UTC
Permalink
Post by Matthew Wilcox
Once Vladislav is ready for his project to be merged, let him approach
us and ask. I don't know if he considers his project ready for merging.
Did he ask you for his help? It's generally considered rather rude to
merge someone else's code without their consent. The way you tried to
get it merged was unhelpful and, if Vladislav were less skillful a
communicator, could have actively hindered getting it merged.
You'd better verify your assumptions before you take them for truth. I
asked Vladislav for permission by private e-mail before I posted the
scsi_execute_async_fifo() patch. Did try I to merge this code without
Vladislav's consent ? No. Anyone can see that I kept Vladislav and the
scst-devel mailing list in CC.

Let us return to the actual topic of this thread. Does the above mean
that you agree with inclusion of SCST in the mainline kernel ? It
still has to be decided who will maintain the mainline kernel SCSI
target code. In the past James wrote that he does not really have the
time for this. Until now the only one who volunteered to maintain such
a subsystem is Vladislav.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-04 15:53:32 UTC
Permalink
On Sat, May 3, 2008 at 12:39 PM, Bart Van Assche
Post by Bart Van Assche
To answer your question: as you noticed, I did not yet contribute any
code to the SCST project. I'm an iSCSI/iSER/SRP user. I'd like to have
a SCSI target framework available on Linux that is as fast as
possible, as reliable as possible, as standards compliant as possible,
well maintained and for which all kernel code is integrated in the
mainstream Linux kernel. I'm probably sharing this desire with all
STGT, IET, SCST and LIO users.
Hello Matthew,

To be complete I should have mentioned that there already exists an
FCoE target implementation for SCST. I'm still learning about FCoE.
What I read about it is that FCoE has more to offer with regard to
storage management than iSCSI/iSER/SRP ?

See also: http://www.open-fcoe.org/openfc/wiki/index.php/Quickstart

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-04 11:35:51 UTC
Permalink
Post by Matthew Wilcox
Post by Bart Van Assche
Post by Vladislav Bolkhovitin
I wrote all above to support my at first glance shocking conclusion that
SCSI target subsystem is completely new subsystem of the kernel and it
should live on its own with its own maintainer! This is the same as with the
current interaction between SCSI and block subsystems in the kernel: SCSI
uses block's functionality, but that doesn't mean that block and SCSI are
the same subsystem.
Hello Tomo,
Due to the IET and STGT projects you have a lot of experience with
implementing SCSI target frameworks. What is your opinion about how a
kernel space SCSI target framework should fit in the Linux kernel ?
Bart, what is your role in the SCST project? You don't seem to have
contributed any code to it (going by the SVN logs on sourceforge), and
your questions and suggestions seem to be those of someone not familiar
with the code. If you're not a developer, it might be more helpful for
you to step back and let Vladislav handle this.
Consider Bart as a member of SCST team at the moment responsible for
Linux kernel inclusion.

Vlad

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-04 15:23:06 UTC
Permalink
Post by Vladislav Bolkhovitin
Post by James Bottomley
Post by Bart Van Assche
James Bottomley clearly expressed in that thread that he doesn't want
to maintain two SCSI target frameworks. So what I propose is that SCST
is included in the mainline and afterwards that it is evaluated
whether or not it is desirable to keep other target code in the
mainline kernel.
That's hardly sufficient. STGT is already in use. Their either has to
be a migration path or, the preferred option, take the pieces of SCST
that are actual improvements and embed them in STGT.
Actually, between SCSI initiator and target subsystems there is almost
*nothing* in common. This claim, at first glance, looks pretty wrong,
because both serve SCSI, so they must have a lot common. But look deeper
and you quickly find out that the majority of functionality as well as
data they use are dedicated for each subsystem, not shared.
Just look at SCST/qla2x00t/(changes done in the initiator qla2xxx driver
to support target mode, patch attached): 90% of changes is adding
callbacks for external target add-on, the rest is support for older,
than 2.6.17, kernels and sysfs magic. Note, no data are common between
initiator and target parts in the meaning that they both use them.
Perhaps, I should elaborate more on this to eliminate possible
misunderstanding. Of course, both main initiator driver and target
add-on driver directly use the same hardware, so they share all internal
hardware-related data, e.g. hardware_lock, but this doesn't matter for
our topic, because this sharing is on the different level. All such data
are hardware specific, hence different hardware have different sets of
such data, hence it is impractical to find something common in them to
expose as a common interface, which all initiator drivers should expose
to its target add-ons: the interface would be more complicated than
direct implementation in each particular case.
Post by Vladislav Bolkhovitin
Then look at SCST (http://scst.sf.net). It implements complete
pass-through SCSI support and look how it interacts with initiator SCSI
subsystem. It calls only 2 functions: FIFO version of
scsi_execute_async() (original scsi_execute_async() provides
unacceptable LIFO commands order) and scsi_reset_provider() for task
management. And there is only one common variable: struct scsi_device.
That's all! In other storage modes (FILEIO/BLOCKIO) there is nothing
common with SCSI initiator subsystem at all.
Finally, try to find out in SCST any duplicated functionality.
Now, let's look how SCSI target/initiator integration currently done in
the kernel. For me it looks pretty artificial. For example, if I make a
general purpose kernel, for which 1% of users would run target mode, I
would have to enable as module "SCSI target support" as well as SCSI
target support for transport attributes. Now 99% of users of my kernel,
who don't need SCSI target, but need SCSI initiator drivers, would have
to have scsi_tgt loaded, because transport attribute drivers would
# lsmod
Module Size Used by
qla2xxx 130844 0
firmware_class 8064 1 qla2xxx
scsi_transport_fc 40900 1 qla2xxx
scsi_tgt 12196 1 scsi_transport_fc
brd 6924 0
xfs 511280 1
dm_mirror 24368 0
dm_mod 51148 1 dm_mirror
uhci_hcd 21400 0
sg 31784 0
e1000 114536 0
pcspkr 3328 0
No target functionality needed, but target mode subsystem is needed. Is
it a good design?
I wrote all above to support my at first glance shocking conclusion that
SCSI target subsystem is completely new subsystem of the kernel and it
should live on its own with its own maintainer! This is the same as with
SCSI uses block's functionality, but that doesn't mean that block and
SCSI are the same subsystem.
- All initiator drivers should live in the SCSI initiator subsystem
(aka current SCSI subsystem) only, the same as today.
- All target drivers should live in the SCSI target subsystem only and
be either add-ons to initiator drivers, like, e.g., qla2x00t, or be a
complete driver, like, e.g., iSCSI-SCST.
Vlad
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-02 18:09:51 UTC
Permalink
Post by James Bottomley
Post by Bart Van Assche
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding out-of-tree modules: this is just a preparatory step before
submitting SCST for inclusion in the mainstream kernel.
And what crackpipe did you smoke to thing we'd put duplicated target
framework in?
Why are you so aggressive ? I didn't insult you in any way.
Regarding inclusion of SCSI target code in the mainline, this subject
has already been discussed extensively in the past
(http://lkml.org/lkml/2008/1/23/134). The conclusion was clear: SCST
is faster than any other existing iSCSI target for Linux (IET, STGT,
LIO), stable, well maintained and the most standards compliant target.
Why do you want to reopen this discussion ?
That's an interesting rewrite of history. The evidence you presented
showed fairly identical results apart from on one contrived IB benchmark
that couldn't directly compare the two.
I'm also on record in the thread saying that was insufficient proof for
me to justify throwing STGT out and replacing it with SCST.
James, why do you keep ignoring important points, written by me in that
e-mail: http://lkml.org/lkml/2008/1/30/178?

Namely:

1. Solid architecture of SCST is inherently more simple than distributed
user/kernel space processing, when kernel behaves under control of user
space, used in STGT, and allows to get better results with less effort.
Better in all aspects: simplicity (hence, maintainability), reliability
and performance. Linux once made step away from microkernel based design
and that was for really good reasons.

2. Zero-copy operations with page cache will halve processing latency on
high speed links, like InfiniBand, and it is impossible to implement
that in a sane way with STGT approach, while for SCST it can be
implemented simply and naturally.

Vlad

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-04 11:48:52 UTC
Permalink
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
scsi_execute_async() is already a nice and simple helper function on top
blk_execute_rq_nowait(). What's the point then to remove it? Do you
consider that exposing scsi_execute_async() internals to its users is
better?
Post by Christoph Hellwig
Post by Bart Van Assche
Bart.
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Boaz Harrosh
2008-05-04 17:53:38 UTC
Permalink
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
scsi_execute_async() is already a nice and simple helper function on top
blk_execute_rq_nowait(). What's the point then to remove it? Do you
consider that exposing scsi_execute_async() internals to its users is
better?
The problem with it is the use of sg list to *hack* in bio's. Which totally
ignores/duplicates block layer mechanisms. There is pending a large patchset
that removes the use of scsi_execute_async from sg/sr and friends to use blk_map_*
members and directly call blk_execute_*. The original patchset was written by
Mike Christie but is now brought up to date by Tomo. It should be submitted soon I think.
If you need good example of usage check out bsg.c it maps user-space data in all kind
of combinations. If you have kernel space memory it is even simpler.
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Bart.
---end quoted text---
If you need any help, please feel free to ask.

Boaz

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-13 16:48:19 UTC
Permalink
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
scsi_execute_async() is already a nice and simple helper function on top
blk_execute_rq_nowait(). What's the point then to remove it? Do you
consider that exposing scsi_execute_async() internals to its users is
better?
The problem with it is the use of sg list to *hack* in bio's. Which totally
ignores/duplicates block layer mechanisms. There is pending a large patchset
that removes the use of scsi_execute_async from sg/sr and friends to use blk_map_*
members and directly call blk_execute_*. The original patchset was written by
Mike Christie but is now brought up to date by Tomo. It should be submitted soon I think.
If you need good example of usage check out bsg.c it maps user-space data in all kind
of combinations. If you have kernel space memory it is even simpler.
Thanks for pointing on it. But it still remained unclear for me what's
the point in the scsi_execute_async() removal. Function
scsi_req_map_sg() looks pretty simple and straightforward, so I don't
see how the overall code can be simplified.
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Bart.
---end quoted text---
If you need any help, please feel free to ask.
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Boaz Harrosh
2008-05-13 17:35:10 UTC
Permalink
Post by Vladislav Bolkhovitin
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
scsi_execute_async() is already a nice and simple helper function on top
blk_execute_rq_nowait(). What's the point then to remove it? Do you
consider that exposing scsi_execute_async() internals to its users is
better?
The problem with it is the use of sg list to *hack* in bio's. Which totally
ignores/duplicates block layer mechanisms. There is pending a large patchset
that removes the use of scsi_execute_async from sg/sr and friends to use blk_map_*
members and directly call blk_execute_*. The original patchset was written by
Mike Christie but is now brought up to date by Tomo. It should be submitted soon I think.
If you need good example of usage check out bsg.c it maps user-space data in all kind
of combinations. If you have kernel space memory it is even simpler.
Thanks for pointing on it. But it still remained unclear for me what's
the point in the scsi_execute_async() removal. Function
scsi_req_map_sg() looks pretty simple and straightforward, so I don't
see how the overall code can be simplified.
Well No, scsi_req_map_sg() is a complete hack. If you have user memory
or kernel memory you better go through blk_map_* which will take care of
device masks, alignment and all, where here the ULD does that. So you have
2 places of waisted code both at ULD to build the SG right, and here to
translate SG to BIO. Where at block layer you have one function call.
Try it out you see that not using scsi_execute_async() is much more simple
at ULD then using it.
If you do mmap then Tomo has code for block layer to support that.
Post by Vladislav Bolkhovitin
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Bart.
---end quoted text---
If you need any help, please feel free to ask.
Boaz

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-14 15:58:42 UTC
Permalink
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
scsi_execute_async() is already a nice and simple helper function on top
blk_execute_rq_nowait(). What's the point then to remove it? Do you
consider that exposing scsi_execute_async() internals to its users is
better?
The problem with it is the use of sg list to *hack* in bio's. Which totally
ignores/duplicates block layer mechanisms. There is pending a large patchset
that removes the use of scsi_execute_async from sg/sr and friends to use blk_map_*
members and directly call blk_execute_*. The original patchset was written by
Mike Christie but is now brought up to date by Tomo. It should be submitted soon I think.
If you need good example of usage check out bsg.c it maps user-space data in all kind
of combinations. If you have kernel space memory it is even simpler.
Thanks for pointing on it. But it still remained unclear for me what's
the point in the scsi_execute_async() removal. Function
scsi_req_map_sg() looks pretty simple and straightforward, so I don't
see how the overall code can be simplified.
Well No, scsi_req_map_sg() is a complete hack. If you have user memory
or kernel memory you better go through blk_map_* which will take care of
device masks, alignment and all, where here the ULD does that. So you have
2 places of waisted code both at ULD to build the SG right, and here to
translate SG to BIO. Where at block layer you have one function call.
Try it out you see that not using scsi_execute_async() is much more simple
at ULD then using it.
If you do mmap then Tomo has code for block layer to support that.
Seems, I'm starting understanding you. You mean that all ULDs (User
Level Devices, i.e. sg, st, etc.) deal with user supplied buffers, i.e.
pointers to virtually continuous memory, which at the moment it has to
convert to SG vector, which then will be translated to BIOs for the
corresponding LDD by scsi_execute_async() (and then back to SG vector on
the queuecommand() time). So, it will be simpler to supply that buffer
pointer directly to block functions. Correct?

But the problem is that in SCST in each data transfer 2 LDDs
participate: one target and one backstorage (initiator). And the target
LDD deals with SG vectors only. So, SCST never deals with buffers, it
always deals with SG vectors and pass them between target LDDs and
backstorage as necessary.

Thus, looks like for SCST in the pass-through mode there is no
alternative to scsi_execute_async().

Thanks,
Vlad

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Boaz Harrosh
2008-05-14 16:38:26 UTC
Permalink
Post by Vladislav Bolkhovitin
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
scsi_execute_async() is already a nice and simple helper function on top
blk_execute_rq_nowait(). What's the point then to remove it? Do you
consider that exposing scsi_execute_async() internals to its users is
better?
The problem with it is the use of sg list to *hack* in bio's. Which totally
ignores/duplicates block layer mechanisms. There is pending a large patchset
that removes the use of scsi_execute_async from sg/sr and friends to use blk_map_*
members and directly call blk_execute_*. The original patchset was written by
Mike Christie but is now brought up to date by Tomo. It should be submitted soon I think.
If you need good example of usage check out bsg.c it maps user-space data in all kind
of combinations. If you have kernel space memory it is even simpler.
Thanks for pointing on it. But it still remained unclear for me what's
the point in the scsi_execute_async() removal. Function
scsi_req_map_sg() looks pretty simple and straightforward, so I don't
see how the overall code can be simplified.
Well No, scsi_req_map_sg() is a complete hack. If you have user memory
or kernel memory you better go through blk_map_* which will take care of
device masks, alignment and all, where here the ULD does that. So you have
2 places of waisted code both at ULD to build the SG right, and here to
translate SG to BIO. Where at block layer you have one function call.
Try it out you see that not using scsi_execute_async() is much more simple
at ULD then using it.
If you do mmap then Tomo has code for block layer to support that.
Seems, I'm starting understanding you. You mean that all ULDs (User
Level Devices, i.e. sg, st, etc.) deal with user supplied buffers, i.e.
pointers to virtually continuous memory, which at the moment it has to
convert to SG vector, which then will be translated to BIOs for the
corresponding LDD by scsi_execute_async() (and then back to SG vector on
the queuecommand() time). So, it will be simpler to supply that buffer
pointer directly to block functions. Correct?
But the problem is that in SCST in each data transfer 2 LDDs
participate: one target and one backstorage (initiator). And the target
LDD deals with SG vectors only. So, SCST never deals with buffers, it
always deals with SG vectors and pass them between target LDDs and
backstorage as necessary.
Where is that SG coming from? is it from Network stack? or is it an
SG prepared by scsi-ml with call to blk_rq_map_sg()?
Post by Vladislav Bolkhovitin
Thus, looks like for SCST in the pass-through mode there is no
alternative to scsi_execute_async().
Tomo what do you think is it logical to add a blk function that will
accept an SG list and map it to a request. Like, for example, an SG
from Network? opposite of what blk_rq_map_sg does.
Post by Vladislav Bolkhovitin
Thanks,
Vlad
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-14 16:49:41 UTC
Permalink
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
scsi_execute_async() is already a nice and simple helper function on top
blk_execute_rq_nowait(). What's the point then to remove it? Do you
consider that exposing scsi_execute_async() internals to its users is
better?
The problem with it is the use of sg list to *hack* in bio's. Which totally
ignores/duplicates block layer mechanisms. There is pending a large patchset
that removes the use of scsi_execute_async from sg/sr and friends to use blk_map_*
members and directly call blk_execute_*. The original patchset was written by
Mike Christie but is now brought up to date by Tomo. It should be submitted soon I think.
If you need good example of usage check out bsg.c it maps user-space data in all kind
of combinations. If you have kernel space memory it is even simpler.
Thanks for pointing on it. But it still remained unclear for me what's
the point in the scsi_execute_async() removal. Function
scsi_req_map_sg() looks pretty simple and straightforward, so I don't
see how the overall code can be simplified.
Well No, scsi_req_map_sg() is a complete hack. If you have user memory
or kernel memory you better go through blk_map_* which will take care of
device masks, alignment and all, where here the ULD does that. So you have
2 places of waisted code both at ULD to build the SG right, and here to
translate SG to BIO. Where at block layer you have one function call.
Try it out you see that not using scsi_execute_async() is much more simple
at ULD then using it.
If you do mmap then Tomo has code for block layer to support that.
Seems, I'm starting understanding you. You mean that all ULDs (User
Level Devices, i.e. sg, st, etc.) deal with user supplied buffers, i.e.
pointers to virtually continuous memory, which at the moment it has to
convert to SG vector, which then will be translated to BIOs for the
corresponding LDD by scsi_execute_async() (and then back to SG vector on
the queuecommand() time). So, it will be simpler to supply that buffer
pointer directly to block functions. Correct?
But the problem is that in SCST in each data transfer 2 LDDs
participate: one target and one backstorage (initiator). And the target
LDD deals with SG vectors only. So, SCST never deals with buffers, it
always deals with SG vectors and pass them between target LDDs and
backstorage as necessary.
Where is that SG coming from? is it from Network stack? or is it an
SG prepared by scsi-ml with call to blk_rq_map_sg()?
It's prepared by SCST core. A request comes from target LDD and reply
finally sent there. Everything goes in the kernel mode only. It is
simple pass-through, exactly as it sounds.
Post by Boaz Harrosh
Post by Vladislav Bolkhovitin
Thus, looks like for SCST in the pass-through mode there is no
alternative to scsi_execute_async().
Tomo what do you think is it logical to add a blk function that will
accept an SG list and map it to a request. Like, for example, an SG
from Network? opposite of what blk_rq_map_sg does.
Post by Vladislav Bolkhovitin
Thanks,
Vlad
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-04 15:30:29 UTC
Permalink
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
Source reading learned me that the function scsi_execute_async() calls
blk_execute_rq_nowait() with 1 as fourth argument, which means that
the request is inserted at the head of the queue. This means that
requests queued with scsi_execute_async() are executed before other
queued requests, and that this function has LIFO (last in first out)
semantics.

All non-SCSI calls to blk_execute_rq() / blk_execute_rq_nowait() add
requests at the end of the queue (except those calls for terminating
I/O).

What is the background of this special behavior of the SCSI subsystem ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bart Van Assche
2008-05-08 15:02:50 UTC
Permalink
On Sun, May 4, 2008 at 5:30 PM, Bart Van Assche
Post by Bart Van Assche
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
Source reading learned me that the function scsi_execute_async() calls
blk_execute_rq_nowait() with 1 as fourth argument, which means that
the request is inserted at the head of the queue. This means that
requests queued with scsi_execute_async() are executed before other
queued requests, and that this function has LIFO (last in first out)
semantics.
All non-SCSI calls to blk_execute_rq() / blk_execute_rq_nowait() add
requests at the end of the queue (except those calls for terminating
I/O).
What is the background of this special behavior of the SCSI subsystem ?
Is the linux-scsi mailing list the appropriate mailing list to post
questions like the above ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Arne Redlich
2008-05-08 15:54:03 UTC
Permalink
Post by Bart Van Assche
On Sun, May 4, 2008 at 5:30 PM, Bart Van Assche
Post by Bart Van Assche
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
Source reading learned me that the function scsi_execute_async() calls
blk_execute_rq_nowait() with 1 as fourth argument, which means that
the request is inserted at the head of the queue. This means that
requests queued with scsi_execute_async() are executed before other
queued requests, and that this function has LIFO (last in first out)
semantics.
All non-SCSI calls to blk_execute_rq() / blk_execute_rq_nowait() add
requests at the end of the queue (except those calls for terminating
I/O).
What is the background of this special behavior of the SCSI subsystem ?
Is the linux-scsi mailing list the appropriate mailing list to post
questions like the above ?
I guess this should
http://marc.info/?l=linux-scsi&m=116668527804801&w=2
answer your question.

HTH,
Arne

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Vladislav Bolkhovitin
2008-05-13 16:47:47 UTC
Permalink
Post by Arne Redlich
Post by Bart Van Assche
On Sun, May 4, 2008 at 5:30 PM, Bart Van Assche
Post by Bart Van Assche
Post by Christoph Hellwig
Post by Bart Van Assche
Regarding scsi_execute_async(): I didn't know that this API is on its
way out. What will it be replaced by, and when ?
blk_execute_rq/blk_execute_rq_nowait plus the block level helpers built
ontop to build requests.
Source reading learned me that the function scsi_execute_async() calls
blk_execute_rq_nowait() with 1 as fourth argument, which means that
the request is inserted at the head of the queue. This means that
requests queued with scsi_execute_async() are executed before other
queued requests, and that this function has LIFO (last in first out)
semantics.
All non-SCSI calls to blk_execute_rq() / blk_execute_rq_nowait() add
requests at the end of the queue (except those calls for terminating
I/O).
What is the background of this special behavior of the SCSI subsystem ?
Is the linux-scsi mailing list the appropriate mailing list to post
questions like the above ?
I guess this should
http://marc.info/?l=linux-scsi&m=116668527804801&w=2
answer your question.
Thanks for pointing on it. There were another threads about that, e.g.
http://thread.gmane.org/gmane.linux.scsi/16498/focus=16784. So, there
was an agreement that the current LIFO requests submission behavior is
inappropriate and a good solution will be to add to scsi_execute_async()
one more argument to specify where to insert the request, in the tail or
in the head of the queue. The patch Bart sent basically implements
exactly that, with the only difference that the new argument passed
explicitly via new function scsi_execute_async_fifo(). There only thing
left is to update via another patch other users of scsi_execute_async()
to use new function, where necessary. Correct?

I wouldn't mind if that will be done after the pending patchset removing
scsi_execute_async() will be applied for new scsi_execute_async()
substitute function. But it seems this patchset is pending for pretty
long time (a half a year? Or even more?). So, what does prevent it from
being applied? How long more are we going to wait for it?

Vlad
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...