2010-08-21 18:42:36

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

Nicholas A. Bellinger, on 08/20/2010 02:29 AM wrote:
> "We will be extending the scsi_tgt_[lib,if].c mapped ring interface to
> allow TCM to access userspace backstores transparently with existing
> kernel level TCM fabric modules, and using the generic configfs fabric
> module infrastructure in target_core_fabric_configfs.c for the port and
> I_T nexus control plane just as you would with any TCM backstore
> subsystem today.
>
> Again, in open source you have to build upon what already exists and
> move forward. The original STGT kernel<-> userspace ring abstraction
> and logic in drivers/scsi/scsi_tgt_lib.c:scsi_tgt_queue_command() ->
> scsi_tgt_uspace_send_cmd() is already going to do the vast majority of
> what is required for handling fabric I/O processing and I_T Nexus and
> Port management in kernel space with a userspace backstore. It is
> really just a matter of allowing the STGT ring request to optionally be
> sent out to userspace as a standalone LUN instead of as a target port."

You forgot to mention one small thing. You are going to (re)use current
STGT interface for user space backend, which in 5 years of being
mainline has not gained any noticeable interest, because it is
fundamentally slow. It needs a complete redesign to become fast. In
contrast, interface provided by scst_user has just 3% overhead comparing
with fully in-kernel backend and allows to build storage capable of
handling 1,500,000 IOPSes (Kaminario).

Vlad


2010-08-21 20:29:01

by Nicholas A. Bellinger

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Sat, 2010-08-21 at 22:42 +0400, Vladislav Bolkhovitin wrote:
> Nicholas A. Bellinger, on 08/20/2010 02:29 AM wrote:
> > "We will be extending the scsi_tgt_[lib,if].c mapped ring interface to
> > allow TCM to access userspace backstores transparently with existing
> > kernel level TCM fabric modules, and using the generic configfs fabric
> > module infrastructure in target_core_fabric_configfs.c for the port and
> > I_T nexus control plane just as you would with any TCM backstore
> > subsystem today.
> >
> > Again, in open source you have to build upon what already exists and
> > move forward. The original STGT kernel<-> userspace ring abstraction
> > and logic in drivers/scsi/scsi_tgt_lib.c:scsi_tgt_queue_command() ->
> > scsi_tgt_uspace_send_cmd() is already going to do the vast majority of
> > what is required for handling fabric I/O processing and I_T Nexus and
> > Port management in kernel space with a userspace backstore. It is
> > really just a matter of allowing the STGT ring request to optionally be
> > sent out to userspace as a standalone LUN instead of as a target port."
>
> You forgot to mention one small thing. You are going to (re)use current
> STGT interface for user space backend, which in 5 years of being
> mainline has not gained any noticeable interest, because it is
> fundamentally slow.

First, 'build upon' and blind '(re)use' are different approaches.
Building on is an important requirement for working with any existing
mainline code regardless of how much much attention the code itself may
require. Initially re-using pieces of the mainline code is acceptable
to get a prototype up and running, and since we don't have many users
for this piece of STGT, it will easier to make larger changes (eg: move
beyond simple re-use) without breaking a large legacy base.

This is what I actually prefer moving forward, as it gives us more
flexibility for the future.

> It needs a complete redesign to become fast.

Secondly, not being the original author of drivers/scsi/scsi_tgt_*, I am
happy to do the work and submit the necessary patches to achieve the
desired goal. Also, considering now we have the TCM v4 HBA/DEV
abstraction with a fabric independent control path in
target_core_fabric_configfs.c, there suddenly new interest to build upon
tomo's and mnc'c original STGT work in drivers/scsi/scsi_tgt_*.c

Using struct scsi_cmnd allocation from a specially allocated struct
request_queue still my preferred method for doing this, unless tomo and
mnc state otherwise. Also if we need to change the request_queue from
being per struct Scsi_Host to struct scsi_device that is one thing that
will be fine. If we need to make more drastic changes to
drivers/scsi/scsi_tgt_if.c that is also fine.

However saying that it needs a 'complete redesign', especially without
ever consulting or offering to creat patches with STGT guys is not how
mainline development functions.

> In
> contrast, interface provided by scst_user has just 3% overhead comparing
> with fully in-kernel backend and allows to build storage capable of
> handling 1,500,000 IOPSes (Kaminario).
>

Great! Please understand that you are more than welcome to create your
own TCM subsystem plugin for your SCST kernel <-> user passthrough
interface so it can take advantage of all the drivers/target/ code has
to offer. Also, now that target_core_iblock.c, target_core_pscsi.c,
target_core_file.c, and target_core_stgt.c are seperate standalone LKMs
loaded on-demand by TCM/ConfigFS, creating a new TCM struct
se_subsystem_api module will be even easier for you now.

I would even be happy to send a functioning struct se_subsystem_api
skeleton for your efforts once the tcm_mod_builder.py script I am
currently working on for producing unlimited ready-to-run configfs
skeletons for new TCM fabric modules is ready to be pushed.

Best,

--nab

2010-08-21 20:43:25

by James Bottomley

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Sat, 2010-08-21 at 22:42 +0400, Vladislav Bolkhovitin wrote:
> Nicholas A. Bellinger, on 08/20/2010 02:29 AM wrote:
> > "We will be extending the scsi_tgt_[lib,if].c mapped ring interface to
> > allow TCM to access userspace backstores transparently with existing
> > kernel level TCM fabric modules, and using the generic configfs fabric
> > module infrastructure in target_core_fabric_configfs.c for the port and
> > I_T nexus control plane just as you would with any TCM backstore
> > subsystem today.
> >
> > Again, in open source you have to build upon what already exists and
> > move forward. The original STGT kernel<-> userspace ring abstraction
> > and logic in drivers/scsi/scsi_tgt_lib.c:scsi_tgt_queue_command() ->
> > scsi_tgt_uspace_send_cmd() is already going to do the vast majority of
> > what is required for handling fabric I/O processing and I_T Nexus and
> > Port management in kernel space with a userspace backstore. It is
> > really just a matter of allowing the STGT ring request to optionally be
> > sent out to userspace as a standalone LUN instead of as a target port."
>
> You forgot to mention one small thing. You are going to (re)use current
> STGT interface for user space backend, which in 5 years of being
> mainline has not gained any noticeable interest, because it is
> fundamentally slow.

That's not exactly what the results of a speed comparison one of your
people did said, now is it? The results were actually not much
difference on line speeds up to GigE.

Interface re-use (or at least ABI compatibility) is the whole point,
it's what makes the solution a drop in replacement.

> It needs a complete redesign to become fast. In
> contrast, interface provided by scst_user has just 3% overhead comparing
> with fully in-kernel backend and allows to build storage capable of
> handling 1,500,000 IOPSes (Kaminario).

For a replacement, first we get the current userspace code working as
is, then you can propose modifications to make it go faster.

James

2010-08-22 07:39:07

by Bart Van Assche

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Sat, Aug 21, 2010 at 10:43 PM, James Bottomley
<[email protected]> wrote:
> On Sat, 2010-08-21 at 22:42 +0400, Vladislav Bolkhovitin wrote:
>> [ ... ]
>>
>> You forgot to mention one small thing. You are going to (re)use current
>> STGT interface for user space backend, which in 5 years of being
>> mainline has not gained any noticeable interest, because it is
>> fundamentally slow.
>
> That's not exactly what the results of a speed comparison one of your
> people did said, now is it? ?The results were actually not much
> difference on line speeds up to GigE.
>
> [ ... ]

I assume that you are referring to this message:
http://lkml.org/lkml/2008/1/29/387 ? You might have missed the reply
that was posted by Roland Dreier, a highly regarded kernel maintainer
and InfiniBand expert (http://lkml.org/lkml/2008/1/29/402):

"Maybe I'm all wet, but I think iSER vs. SRP should be roughly
comparable. The exact formatting of various messages etc. is
different but the data path using RDMA is pretty much identical. So
the big difference between STGT iSER and SCST SRP hints at some big
difference in the efficiency of the two implementations."

Furthermore, I would like to clarify that Vlad hasn't asked me to
start working on the SCST project but that I selected the SCST project
myself after an extensive stability and performance comparison of four
existing open source storage target projects.

Bart.

2010-08-22 20:29:22

by James Bottomley

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Sun, 2010-08-22 at 09:39 +0200, Bart Van Assche wrote:
> On Sat, Aug 21, 2010 at 10:43 PM, James Bottomley
> <[email protected]> wrote:
> > On Sat, 2010-08-21 at 22:42 +0400, Vladislav Bolkhovitin wrote:
> >> [ ... ]
> >>
> >> You forgot to mention one small thing. You are going to (re)use current
> >> STGT interface for user space backend, which in 5 years of being
> >> mainline has not gained any noticeable interest, because it is
> >> fundamentally slow.
> >
> > That's not exactly what the results of a speed comparison one of your
> > people did said, now is it? The results were actually not much
> > difference on line speeds up to GigE.
> >
> > [ ... ]
>
> I assume that you are referring to this message:
> http://lkml.org/lkml/2008/1/29/387 ? You might have missed the reply
> that was posted by Roland Dreier, a highly regarded kernel maintainer
> and InfiniBand expert (http://lkml.org/lkml/2008/1/29/402):
>
> "Maybe I'm all wet, but I think iSER vs. SRP should be roughly
> comparable. The exact formatting of various messages etc. is
> different but the data path using RDMA is pretty much identical. So
> the big difference between STGT iSER and SCST SRP hints at some big
> difference in the efficiency of the two implementations."

So the phrase "up to GigE" was deliberately in the above to exclude the
disputed infiniband results. I'm not really interested in re-opening
the arguments over how to interpret those results. The fact that SCST
and STGT were on par up to 1GbE is enough to refute the contention that
STGT is "fundamentally slow".

James

> Furthermore, I would like to clarify that Vlad hasn't asked me to
> start working on the SCST project but that I selected the SCST project
> myself after an extensive stability and performance comparison of four
> existing open source storage target projects.
>
> Bart.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-08-23 13:48:12

by Joe Landman

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

James Bottomley wrote:

> So the phrase "up to GigE" was deliberately in the above to exclude the
> disputed infiniband results. I'm not really interested in re-opening
> the arguments over how to interpret those results. The fact that SCST
> and STGT were on par up to 1GbE is enough to refute the contention that
> STGT is "fundamentally slow".

We haven't tested this in at least 6 months, but we did test iSCSI over
10GbE using SCST and STGT. This was one of the reasons we wound up
going with SCST. For the past several years, our performance on SCST
was much higher than on STGT.

If it helps, we can redo these tests with a modern kernel (2.6.35.x) and
same backend/frontend. We've been switching most of our IO performance
testing to Jens Axboe's excellent fio tool. I'd be happy to share our
testing decks with anyone (and collaborate on generating a good useful
set for general use)

This said, I have to add in my support to SCST and its developers.
Vlad, Bart, and the rest of the community have been very helpful to us.
We have been a consumer rather than a developer to date, so we have
less of a dog in this fight than others. We have nothing against LIO or
STGT. We will try them and see if they meet our needs. SRP is a hard
requirement, and it exists and works great in SCST. So for us, a
solution which gives us the best of all worlds would be one where we
don't have to give up what works well for us, and gets us new
capabilities/features. This is more of a wish-list ... we have to keep
using what works well for us.

Our interest in STGT is that we would like a working iSER. Vlad has
suggested a method that we will explore a little later on (next
month). The LIO folks do look like they have some interesting
capabilities beyond this, so we will look. But without SRP, and a fast
iSCSI, their really isn't much choice for us in the near term.

Regards,

Joe

--
Joseph Landman, Ph.D
Founder and CEO
Scalable Informatics Inc.
email: [email protected]
web : http://scalableinformatics.com
http://scalableinformatics.com/jackrabbit
phone: +1 734 786 8423 x121
fax : +1 866 888 3112
cell : +1 734 612 4615

2010-08-23 15:12:30

by Bart Van Assche

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Mon, Aug 23, 2010 at 3:47 PM, Joe Landman
<[email protected]> wrote:
>
> James Bottomley wrote:
>
>> So the phrase "up to GigE" was deliberately in the above to exclude the
>> disputed infiniband results. ?I'm not really interested in re-opening
>> the arguments over how to interpret those results. ?The fact that SCST
>> and STGT were on par up to 1GbE is enough to refute the contention that
>> STGT is "fundamentally slow".
>
> We haven't tested this in at least 6 months, but we did test iSCSI over 10GbE using SCST and STGT. ?This was one of the reasons we wound up going with SCST. ?For the past several years, our performance on SCST was much higher than on STGT.
>
> If it helps, we can redo these tests with a modern kernel (2.6.35.x) and same backend/frontend. ?We've been switching most of our IO performance testing to Jens Axboe's excellent fio tool. ?I'd be happy to share our testing decks with anyone (and collaborate on generating a good useful set for general use)
>
> This said, I have to add in my support to SCST and its developers. Vlad, Bart, and the rest of the community have been very helpful to us. ?We have been a consumer rather than a developer to date, so we have less of a dog in this fight than others. ?We have nothing against LIO or STGT. ?We will try them and see if they meet our needs. ?SRP is a hard requirement, and it exists and works great in SCST. ?So for us, a solution which gives us the best of all worlds would be one where we don't have to give up what works well for us, and gets us new capabilities/features. ?This is more of a wish-list ... we have to keep using what works well for us.
>
> Our interest in STGT is that we would like a working iSER. ?Vlad has suggested a method that we will explore a little later on (next
> month). ?The LIO folks do look like they have some interesting capabilities beyond this, so we will look. ?But without SRP, and a fast iSCSI, their really isn't much choice for us in the near term.

(resending as plain text)

There is an important design difference between SCST and LIO: SCST by
defaults creates multiple threads to process the I/O operations for a
storage target, while LIO only creates a single thread per storage
target. This makes SCST perform measurably faster.

Bart.

2010-08-23 19:41:33

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

James Bottomley, on 08/23/2010 12:29 AM wrote:
> So the phrase "up to GigE" was deliberately in the above to exclude the
> disputed infiniband results. I'm not really interested in re-opening
> the arguments over how to interpret those results. The fact that SCST
> and STGT were on par up to 1GbE is enough to refute the contention that
> STGT is "fundamentally slow".

Well, James, why not 100MbE? If you want a comparison of target
implementations you need a fast hardware with minimal latency.
Otherwise, the difference between the implementations can drown in the
overhead of the accompanying processing. 1GbE is a nearly 10 years ago
interface. Or are we going to stay ten years behind progress?

Thanks,
Vlad

2010-08-24 14:41:36

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

James Bottomley, on 08/22/2010 12:43 AM wrote:
> Interface re-use (or at least ABI compatibility) is the whole point,
> it's what makes the solution a drop in replacement.

I see now. You want ABI compatibility to keep the "contract" that no
kernel changes can break applications binary compatibility for unlimited
time.

OK, we will write the compatibility module. It shouldn't take much time.

But before we start, I'd like to clear 2 related questions:

1. How far the ABI compatibility "contract" goes? Are there cases, where
it isn't so strong? I'm asking, because I can recall that open-iscsi at
least once has broken ABI compatibility with user space tools. Was it an
accidental (but not corrected) mistake or was it deliberate? If the
latter, then, I guess, there must be some exceptions defining when ABI
compatibility can be not followed.

2. Currently STGT in the kernel is just 2 files, scsi_tgt_if.c and
scsi_tgt_lib.c (with headers), + ibmvstgt driver. The C files define the
STGT interface in question. So, if we keep ABI compatibility with the
new target engine, we would have to keep those 2 files included in the
kernel, which would effectively mean that STGT would stay in the kernel.
This would lead to the situation you are trying to avoid: 2 SCSI target
infrastructures in the kernel. Would it be OK?

Thanks for (eventually!) defining clear rules and removing confusions!

Vlad

2010-08-24 14:51:41

by Chris Weiss

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Tue, Aug 24, 2010 at 9:41 AM, Vladislav Bolkhovitin <[email protected]> wrote:
> James Bottomley, on 08/22/2010 12:43 AM wrote:
>> Interface re-use (or at least ABI compatibility) is the whole point,
>> it's what makes the solution a drop in replacement.
>
> I see now. You want ABI compatibility to keep the "contract" that no
> kernel changes can break applications binary compatibility for unlimited
> time.

ok now I'm confused, or maybe I'm not understanding ABI correctly, or
maybe you guys are using it in a way that is inconsistent with popular
convention. As a VMware user, I have experienced fully that the
kernel ABI changes in various places with every release. VMwares
drivers have to be constantly updated to match changes in kernel
function parameters and even what functions are available.

I've also experienced it with scsi cards, dsl modems, and other 3rd
party drivers. It's the one big downside to developing for the Linux
kernel, the ABI is /always/ changing.

2010-08-24 14:56:46

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Tue, Aug 24, 2010 at 09:51:04AM -0500, Chris Weiss wrote:
> ok now I'm confused, or maybe I'm not understanding ABI correctly, or
> maybe you guys are using it in a way that is inconsistent with popular
> convention. As a VMware user, I have experienced fully that the
> kernel ABI changes in various places with every release. VMwares
> drivers have to be constantly updated to match changes in kernel
> function parameters and even what functions are available.

You're talking about in-kernel ABI, which is constantly in flux. James
is talking about user <-> kernel ABI, which is not supposed to change in
an incompatible way.

--
Matthew Wilcox Intel Open Source Technology Centre
"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."

2010-08-24 14:58:08

by James Bottomley

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Tue, 2010-08-24 at 18:41 +0400, Vladislav Bolkhovitin wrote:
> James Bottomley, on 08/22/2010 12:43 AM wrote:
> > Interface re-use (or at least ABI compatibility) is the whole point,
> > it's what makes the solution a drop in replacement.
>
> I see now. You want ABI compatibility to keep the "contract" that no
> kernel changes can break applications binary compatibility for unlimited
> time.
>
> OK, we will write the compatibility module. It shouldn't take much time.
>
> But before we start, I'd like to clear 2 related questions:
>
> 1. How far the ABI compatibility "contract" goes? Are there cases, where
> it isn't so strong? I'm asking, because I can recall that open-iscsi at
> least once has broken ABI compatibility with user space tools. Was it an
> accidental (but not corrected) mistake or was it deliberate? If the
> latter, then, I guess, there must be some exceptions defining when ABI
> compatibility can be not followed.

I don't think it has to be complete. As long as the STGT people think
it's good enough, that's fine by me.

> 2. Currently STGT in the kernel is just 2 files, scsi_tgt_if.c and
> scsi_tgt_lib.c (with headers), + ibmvstgt driver. The C files define the
> STGT interface in question. So, if we keep ABI compatibility with the
> new target engine, we would have to keep those 2 files included in the
> kernel,

This isn't really correct. The ABI is defined by the headers not the
implementation.

> which would effectively mean that STGT would stay in the kernel.
> This would lead to the situation you are trying to avoid: 2 SCSI target
> infrastructures in the kernel. Would it be OK?

If you mean is the marketing solution of wedging two products into the
kernel and calling it a single one going to fly, the answer is no.

James

2010-08-24 18:08:44

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

Nicholas A. Bellinger, on 08/22/2010 12:25 AM wrote:
>> It needs a complete redesign to become fast.
>
> Secondly, not being the original author of drivers/scsi/scsi_tgt_*, I am
> happy to do the work and submit the necessary patches to achieve the
> desired goal. Also, considering now we have the TCM v4 HBA/DEV
> abstraction with a fabric independent control path in
> target_core_fabric_configfs.c, there suddenly new interest to build upon
> tomo's and mnc'c original STGT work in drivers/scsi/scsi_tgt_*.c
>
> Using struct scsi_cmnd allocation from a specially allocated struct
> request_queue still my preferred method for doing this, unless tomo and
> mnc state otherwise. Also if we need to change the request_queue from
> being per struct Scsi_Host to struct scsi_device that is one thing that
> will be fine. If we need to make more drastic changes to
> drivers/scsi/scsi_tgt_if.c that is also fine.

That would be a bad design, because it would couple together
fundamentally separate things: target and initiator subsystems (server
and client, eg apache and firefox, sendmail and mutt, etc.). It would
make the code harder to maintain and more fragile for modifications. On
the user level it would lead to the need to have target mode core module
always loaded. It is already so with STGT if you use FC or SRP. With
them the only way to not have scsi_tgt loaded is to remove STGT on the
compilation time.

> However saying that it needs a 'complete redesign', especially without
> ever consulting or offering to creat patches with STGT guys is not how
> mainline development functions.

Well, it isn't my habit to bother people asking something about what I
can find an answer myself. I have spent sufficient amount of time
hacking Linux kernel (>10 years, from which 8 in the target mode), so I
can read others C code as easy as a book.

I've already described which flaws I see in the STGT design and nobody
objected me (one of the objections I repeated above). You can find my
messages in the list archive. Isn't answering on exact critics a way how
a cooperative development is supposed to be done? If somebody comes with
a better solution for an existing problem is he supposed be ignored? Is
it the way how the mainline development functions?

>> In
>> contrast, interface provided by scst_user has just 3% overhead comparing
>> with fully in-kernel backend and allows to build storage capable of
>> handling 1,500,000 IOPSes (Kaminario).
>
> Please understand that you are more than welcome to create your
> own TCM subsystem plugin for your SCST kernel<-> user passthrough
> interface so it can take advantage of all the drivers/target/ code has
> to offer.

Well, please understand that you are more than welcome to stop
reinventing a wheel and instead just have the functionality and the
advantages you referring already long ago implemented in SCST and being
used to build (using your expression) records breaking storage products.
You are also more than welcome to add the only extra functionality LIO
has over SCST, ALUA, into SCST. Your patches are always welcome.

Vlad

2010-08-24 19:48:11

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

James Bottomley, on 08/24/2010 06:57 PM wrote:
> On Tue, 2010-08-24 at 18:41 +0400, Vladislav Bolkhovitin wrote:
>> James Bottomley, on 08/22/2010 12:43 AM wrote:
>>> Interface re-use (or at least ABI compatibility) is the whole point,
>>> it's what makes the solution a drop in replacement.
>>
>> I see now. You want ABI compatibility to keep the "contract" that no
>> kernel changes can break applications binary compatibility for unlimited
>> time.
>>
>> OK, we will write the compatibility module. It shouldn't take much time.
>>
>> But before we start, I'd like to clear 2 related questions:
>>
>> 1. How far the ABI compatibility "contract" goes? Are there cases, where
>> it isn't so strong? I'm asking, because I can recall that open-iscsi at
>> least once has broken ABI compatibility with user space tools. Was it an
>> accidental (but not corrected) mistake or was it deliberate? If the
>> latter, then, I guess, there must be some exceptions defining when ABI
>> compatibility can be not followed.
>
> I don't think it has to be complete. As long as the STGT people think
> it's good enough, that's fine by me.

Tomonori, Mike, could you comment on that, please?

>> 2. Currently STGT in the kernel is just 2 files, scsi_tgt_if.c and
>> scsi_tgt_lib.c (with headers), + ibmvstgt driver. The C files define the
>> STGT interface in question. So, if we keep ABI compatibility with the
>> new target engine, we would have to keep those 2 files included in the
>> kernel,
>
> This isn't really correct. The ABI is defined by the headers not the
> implementation.

Yes, but we on the target side would not be able to implement the ABI compatible interface without using library functions provided by those C files. Or, at least, it would be much harder.

So, would it be OK for you to keep those files?

>> which would effectively mean that STGT would stay in the kernel.
>> This would lead to the situation you are trying to avoid: 2 SCSI target
>> infrastructures in the kernel. Would it be OK?
>
> If you mean is the marketing solution of wedging two products into the
> kernel and calling it a single one going to fly, the answer is no.

I mean that if we keep those 2 files to ease our ABI compatibility effort, it would effectively mean that we would leave STGT merged. It isn't something we would create, it just would be so itself as a matter of fact. Ultimately, STGT is an user space engine. What it has in the kernel is the interface helper functions to interact with the in-kernel drivers. The simplest way to achieve the ABI compatibility is to make a backend module acting as an STGT in-target driver.

(Actually, I may not ask it, because this is the way how LIO seems[1] implemented that, which was approved on the LSF summit. I only want to make all pros and cons clear from the very beginning.)

Thanks,
Vlad

1. I wrote "seems", because currently LIO has the following code for STGT commands execution:

int stgt_do_task(se_task_t *task)
{
stgt_plugin_task_t *st = (stgt_plugin_task_t *) task->transport_req;
struct Scsi_Host *sh = task->se_dev->se_hba->hba_ptr;
struct scsi_cmnd *sc;
int tag = MSG_SIMPLE_TAG;

sc = scsi_host_get_command(sh, st->stgt_direction, GFP_KERNEL);
if (!sc) {
printk(KERN_ERR "Unable to allocate memory for struct"
" scsi_cmnd\n");
return PYX_TRANSPORT_LU_COMM_FAILURE;
}

memcpy(sc->cmnd, st->stgt_cdb, MAX_COMMAND_SIZE);
sc->sdb.length = task->task_size;
sc->sdb.table.sgl = task->task_sg;
sc->tag = tag;

BUG();
#warning FIXME: Get struct scsi_lun for scsi_tgt_queue_command()
#if 0
err = scsi_tgt_queue_command(sc, itn_id, (struct scsi_lun *)&cmd->lun,
cmd->tag);
if (err) {
printk(KERN_INFO "scsi_tgt_queue_command() failed for sc:"
" %p\n", sc);
scsi_host_put_command(sh, sc);
}
#endif
return PYX_TRANSPORT_SENT_TO_TRANSPORT;
}

which means that this pluging completely not functioning.

2010-08-24 21:27:25

by Nicholas A. Bellinger

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Tue, 2010-08-24 at 23:48 +0400, Vladislav Bolkhovitin wrote:
> James Bottomley, on 08/24/2010 06:57 PM wrote:
> > On Tue, 2010-08-24 at 18:41 +0400, Vladislav Bolkhovitin wrote:
> >> James Bottomley, on 08/22/2010 12:43 AM wrote:
> >>> Interface re-use (or at least ABI compatibility) is the whole point,
> >>> it's what makes the solution a drop in replacement.
> >>
> >> I see now. You want ABI compatibility to keep the "contract" that no
> >> kernel changes can break applications binary compatibility for unlimited
> >> time.
> >>
> >> OK, we will write the compatibility module. It shouldn't take much time.
> >>
> >> But before we start, I'd like to clear 2 related questions:
> >>
> >> 1. How far the ABI compatibility "contract" goes? Are there cases, where
> >> it isn't so strong? I'm asking, because I can recall that open-iscsi at
> >> least once has broken ABI compatibility with user space tools. Was it an
> >> accidental (but not corrected) mistake or was it deliberate? If the
> >> latter, then, I guess, there must be some exceptions defining when ABI
> >> compatibility can be not followed.
> >
> > I don't think it has to be complete. As long as the STGT people think
> > it's good enough, that's fine by me.
>
> Tomonori, Mike, could you comment on that, please?
>
> >> 2. Currently STGT in the kernel is just 2 files, scsi_tgt_if.c and
> >> scsi_tgt_lib.c (with headers), + ibmvstgt driver. The C files define the
> >> STGT interface in question. So, if we keep ABI compatibility with the
> >> new target engine, we would have to keep those 2 files included in the
> >> kernel,
> >
> > This isn't really correct. The ABI is defined by the headers not the
> > implementation.
>
> Yes, but we on the target side would not be able to implement the ABI compatible interface without using library functions provided by those C files. Or, at least, it would be much harder.
>
> So, would it be OK for you to keep those files?
>
> >> which would effectively mean that STGT would stay in the kernel.
> >> This would lead to the situation you are trying to avoid: 2 SCSI target
> >> infrastructures in the kernel. Would it be OK?
> >
> > If you mean is the marketing solution of wedging two products into the
> > kernel and calling it a single one going to fly, the answer is no.
>
> I mean that if we keep those 2 files to ease our ABI compatibility effort, it would effectively mean that we would leave STGT merged. It isn't something we would create, it just would be so itself as a matter of fact. Ultimately, STGT is an user space engine. What it has in the kernel is the interface helper functions to interact with the in-kernel drivers. The simplest way to achieve the ABI compatibility is to make a backend module acting as an STGT in-target driver.
>
> (Actually, I may not ask it, because this is the way how LIO seems[1] implemented that, which was approved on the LSF summit. I only want to make all pros and cons clear from the very beginning.)
>
> Thanks,
> Vlad
>
> 1. I wrote "seems", because currently LIO has the following code for STGT commands execution:
>
> int stgt_do_task(se_task_t *task)
> {
> stgt_plugin_task_t *st = (stgt_plugin_task_t *) task->transport_req;
> struct Scsi_Host *sh = task->se_dev->se_hba->hba_ptr;
> struct scsi_cmnd *sc;
> int tag = MSG_SIMPLE_TAG;
>
> sc = scsi_host_get_command(sh, st->stgt_direction, GFP_KERNEL);
> if (!sc) {
> printk(KERN_ERR "Unable to allocate memory for struct"
> " scsi_cmnd\n");
> return PYX_TRANSPORT_LU_COMM_FAILURE;
> }
>
> memcpy(sc->cmnd, st->stgt_cdb, MAX_COMMAND_SIZE);
> sc->sdb.length = task->task_size;
> sc->sdb.table.sgl = task->task_sg;
> sc->tag = tag;
>
> BUG();
> #warning FIXME: Get struct scsi_lun for scsi_tgt_queue_command()
> #if 0
> err = scsi_tgt_queue_command(sc, itn_id, (struct scsi_lun *)&cmd->lun,
> cmd->tag);
> if (err) {
> printk(KERN_INFO "scsi_tgt_queue_command() failed for sc:"
> " %p\n", sc);
> scsi_host_put_command(sh, sc);
> }
> #endif
> return PYX_TRANSPORT_SENT_TO_TRANSPORT;
> }

Vlad,

As mentioned explictly earlier in this thread, my WIP code for the
kernel level subsystem backstore using STGT kernel <-> user CDB
passthrough logic in drivers/target/target_core_stgt.c is a item on my
TODO list, but I will repeat, is NOT being tagged as a mainline .37
item.

Tomo-san, mnc, and other storage folks have been briefed on the
remaining issues that would be involved to get a prototype functioning
with drivers/target/target_core_stgt.c, and rough idea of what it would
take in existing mainline drivers/scsi/scsi_tgt_*.c code. With the
current WIP code this will allow the userspace CDB -> LUN passthrough to
function transparently with all TCM SPC-4 compliant logic as a
standalone struct se_subsystem_api tcm_core_stgt.ko backstore.

Best,

--nab

2010-08-25 22:21:36

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Tuesday 24 August 2010 10:51:04 Chris Weiss wrote:
> On Tue, Aug 24, 2010 at 9:41 AM, Vladislav Bolkhovitin <[email protected]> wrote:
> > James Bottomley, on 08/22/2010 12:43 AM wrote:
> >> Interface re-use (or at least ABI compatibility) is the whole point,
> >> it's what makes the solution a drop in replacement.
> >
> > I see now. You want ABI compatibility to keep the "contract" that no
> > kernel changes can break applications binary compatibility for unlimited
> > time.
>
> ok now I'm confused, or maybe I'm not understanding ABI correctly, or
> maybe you guys are using it in a way that is inconsistent with popular

You are thinking of the KABI. That changes per each release except if you buy
a vendor product. Red Hat for example keeps an KABI symbol list where they
guarantee that those parameters, structures ,etc will never change. John
Masters wrote a nice paper about how they solved this:
http://dup.et.redhat.com/presentations/DriverUpdateProgramTechnical.pdf

I don't have experience with other vendors.

In terms of ABI, think ioctl calls and its a parameters. They are suppose to
stay the same for long long durations.

> convention. As a VMware user, I have experienced fully that the
> kernel ABI changes in various places with every release. VMwares
> drivers have to be constantly updated to match changes in kernel
> function parameters and even what functions are available.
>
> I've also experienced it with scsi cards, dsl modems, and other 3rd
> party drivers. It's the one big downside to developing for the Linux
> kernel, the ABI is /always/ changing.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2010-08-25 22:45:16

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Wed, Aug 25, 2010 at 06:20:26PM -0400, Konrad Rzeszutek Wilk wrote:
> On Tuesday 24 August 2010 10:51:04 Chris Weiss wrote:
> > On Tue, Aug 24, 2010 at 9:41 AM, Vladislav Bolkhovitin <[email protected]> wrote:
> > > James Bottomley, on 08/22/2010 12:43 AM wrote:
> > >> Interface re-use (or at least ABI compatibility) is the whole point,
> > >> it's what makes the solution a drop in replacement.
> > >
> > > I see now. You want ABI compatibility to keep the "contract" that no
> > > kernel changes can break applications binary compatibility for unlimited
> > > time.
> >
> > ok now I'm confused, or maybe I'm not understanding ABI correctly, or
> > maybe you guys are using it in a way that is inconsistent with popular
>
> You are thinking of the KABI. That changes per each release except
> if you buy a vendor product. Red Hat for example keeps an KABI
> symbol list where they guarantee that those parameters, structures
> ,etc will never change. John Masters wrote a nice paper about how
> they solved this:
> http://dup.et.redhat.com/presentations/DriverUpdateProgramTechnical.pdf

Just to make sure people aren't getting more confused. What Red Hat
calls the KABI (and SLES and Ubuntu do something similar) is the
Kernel ABI which is presented to kernel modules. This is important
for companies shipping out-of-tree and proprietary kernel modules.
(And we'll ignore the questions about whether such proprietary kernel
modules violate the GPL or not; contact your favorite lawyer for an
opinion on that subject. It depends on the facts of the case and your
legal jourisdiction, almost certainly.)

> In terms of ABI, think ioctl calls and its a parameters. They are suppose to
> stay the same for long long durations.

When we talk about the ABI must be constant, this is the kernel
interface presented to userspace programs, including statically linked
userspace progams. So system calls, ioctl's, etc.

This is what allows you to download or purchase a userspace program
(including silly things like DB2, or Oracle Database, or Adobe Flash),
and it will work even if you upgrade your kernel.

- Ted

2010-08-26 20:11:11

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

Nicholas A. Bellinger, on 08/25/2010 01:23 AM wrote:
> As mentioned explictly earlier in this thread, my WIP code for the
> kernel level subsystem backstore using STGT kernel<-> user CDB
> passthrough logic in drivers/target/target_core_stgt.c is a item on
> my TODO list, but I will repeat, is NOT being tagged as a mainline
> .37 item.

Hmm, I can't understand, if target_core_stgt.c is "NOT being tagged as a
mainline .37 item", then the STGT ABI compatibility for being a drop in
replacement for STGT isn't a requirement? Or am I missing something?

> Tomo-san, mnc, and other storage folks have been briefed on the
> remaining issues that would be involved to get a prototype
> functioning with drivers/target/target_core_stgt.c, and rough idea of
> what it would take in existing mainline drivers/scsi/scsi_tgt_*.c
> code. With the current WIP code this will allow the userspace CDB ->
> LUN passthrough to function transparently with all TCM SPC-4
> compliant logic as a standalone struct se_subsystem_api
> tcm_core_stgt.ko backstore.

This is exactly what we are discussing.

Thanks,
Vlad

2010-08-26 21:27:18

by Nicholas A. Bellinger

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Fri, 2010-08-27 at 00:11 +0400, Vladislav Bolkhovitin wrote:
> Nicholas A. Bellinger, on 08/25/2010 01:23 AM wrote:
> > As mentioned explictly earlier in this thread, my WIP code for the
> > kernel level subsystem backstore using STGT kernel<-> user CDB
> > passthrough logic in drivers/target/target_core_stgt.c is a item on
> > my TODO list, but I will repeat, is NOT being tagged as a mainline
> > .37 item.
>
> Hmm, I can't understand, if target_core_stgt.c is "NOT being tagged as a
> mainline .37 item", then the STGT ABI compatibility for being a drop in
> replacement for STGT isn't a requirement? Or am I missing something?
>

Sorry, but I have no idea what you are trying to conjour up here.

To spell out (again) the TCM/LIO<->STGT compatibility stages that have
been persued pubically over the last months:

I) Create proper userspace tgt.git SG_IO and BSG passthrough into
TCM_Loop v4 using high-level multi-fabric WWPN emulation so that TCM
Core SPC-4 kernel emulation is exposed to STGT user fabrics, eg:
userspace fabric module -> kernel backstore passthrough. (DONE
for .37, and STGT passthrough + BSG backstore support merged into
tgt.git by Tomo-san)

II) Complete target_core_stgt.c TCM subsystem plugin for kernel -> user
CDB -> LUN passthrough building upon existing
drivers/scsi/scsi_tgt_*.c code. (WIP for .38, made available
initially as a seperate standalone .ko module in lio-core-2.6.git)

> > Tomo-san, mnc, and other storage folks have been briefed on the
> > remaining issues that would be involved to get a prototype
> > functioning with drivers/target/target_core_stgt.c, and rough idea of
> > what it would take in existing mainline drivers/scsi/scsi_tgt_*.c
> > code. With the current WIP code this will allow the userspace CDB ->
> > LUN passthrough to function transparently with all TCM SPC-4
> > compliant logic as a standalone struct se_subsystem_api
> > tcm_core_stgt.ko backstore.
>
> This is exactly what we are discussing.

Then I suggest you start working on a patch for drivers/scsi/scsi_tgt_*
in order to address what you believe are the preceived shortcomings of
the original design.

Until you can do that, and actually take an impartial look at the
existing drivers/scsi/scsi_tgt_*.c, it would be a bit difficult to
beleive you genuinely want to steer our current level of interaction
away from continued hand-waving noise into a real rational technical
discourse between yourself and the LIO/STGT development community.

Best,

--nab

2010-08-28 17:33:43

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

Nicholas A. Bellinger, on 08/27/2010 01:23 AM wrote:
>> Nicholas A. Bellinger, on 08/25/2010 01:23 AM wrote:
>>> As mentioned explictly earlier in this thread, my WIP code for the
>>> kernel level subsystem backstore using STGT kernel<-> user CDB
>>> passthrough logic in drivers/target/target_core_stgt.c is a item on
>>> my TODO list, but I will repeat, is NOT being tagged as a mainline
>>> .37 item.
>>
>> Hmm, I can't understand, if target_core_stgt.c is "NOT being tagged as a
>> mainline .37 item", then the STGT ABI compatibility for being a drop in
>> replacement for STGT isn't a requirement? Or am I missing something?
>
> Sorry, but I have no idea what you are trying to conjour up here.
>
> To spell out (again) the TCM/LIO<->STGT compatibility stages that have
> been persued pubically over the last months:
>
> I) Create proper userspace tgt.git SG_IO and BSG passthrough into
> TCM_Loop v4 using high-level multi-fabric WWPN emulation so that TCM
> Core SPC-4 kernel emulation is exposed to STGT user fabrics, eg:
> userspace fabric module -> kernel backstore passthrough. (DONE
> for .37, and STGT passthrough + BSG backstore support merged into
> tgt.git by Tomo-san)
>
> II) Complete target_core_stgt.c TCM subsystem plugin for kernel -> user
> CDB -> LUN passthrough building upon existing
> drivers/scsi/scsi_tgt_*.c code. (WIP for .38, made available
> initially as a seperate standalone .ko module in lio-core-2.6.git)

That would mean that if LIO merged in .37:

1. It would be merged _without_ STGT ABI compatibility, i.e. one of the
requirements for a new SCSI target infrastructure merge is violated.

2. It would _not_ be a drop in replacement for STGT, because STGT's
drivers/scsi/scsi_tgt_*.c code would stay in the kernel. Those would
effectively mean that another requirement for a new SCSI target
infrastructure merge would be violated: there would be 2 SCSI target
infrastructures in the kernel and any STGT in-kernel driver (for
instance, built as an outside module) would continue working. My
understanding of "drop in replacement" is "one out, another in at once".

Please tell me where I'm wrong? I would appreciate if you be precise and
answer the above 2 my concerns. There is no point to again repeat what
you have already written without adding any new information.

>>> Tomo-san, mnc, and other storage folks have been briefed on the
>>> remaining issues that would be involved to get a prototype
>>> functioning with drivers/target/target_core_stgt.c, and rough idea of
>>> what it would take in existing mainline drivers/scsi/scsi_tgt_*.c
>>> code. With the current WIP code this will allow the userspace CDB ->
>>> LUN passthrough to function transparently with all TCM SPC-4
>>> compliant logic as a standalone struct se_subsystem_api
>>> tcm_core_stgt.ko backstore.
>>
>> This is exactly what we are discussing.
>
> Then I suggest you start working on a patch for drivers/scsi/scsi_tgt_*
> in order to address what you believe are the preceived shortcomings of
> the original design.
>
> Until you can do that, and actually take an impartial look at the
> existing drivers/scsi/scsi_tgt_*.c, it would be a bit difficult to
> beleive you genuinely want to steer our current level of interaction
> away from continued hand-waving noise into a real rational technical
> discourse between yourself and the LIO/STGT development community.

My look is completely impartial. With all my respect, I'm just quite
ahead of you and can see what you are unable (or don't want?) to see. I
did what you are currently doing almost 4 years ago. I have already
written all the necessary code and addressed all _rose on practice_
concerns. But all my attempts to explain my view are just blindly
ignored without any considerations, so I have no idea how more I can
explain it.

Thanks,
Vlad

2010-08-28 20:51:38

by Nicholas A. Bellinger

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Sat, 2010-08-28 at 21:32 +0400, Vladislav Bolkhovitin wrote:
> Nicholas A. Bellinger, on 08/27/2010 01:23 AM wrote:
> >> Nicholas A. Bellinger, on 08/25/2010 01:23 AM wrote:
> >>> As mentioned explictly earlier in this thread, my WIP code for the
> >>> kernel level subsystem backstore using STGT kernel<-> user CDB
> >>> passthrough logic in drivers/target/target_core_stgt.c is a item on
> >>> my TODO list, but I will repeat, is NOT being tagged as a mainline
> >>> .37 item.
> >>
> >> Hmm, I can't understand, if target_core_stgt.c is "NOT being tagged as a
> >> mainline .37 item", then the STGT ABI compatibility for being a drop in
> >> replacement for STGT isn't a requirement? Or am I missing something?
> >
> > Sorry, but I have no idea what you are trying to conjour up here.
> >
> > To spell out (again) the TCM/LIO<->STGT compatibility stages that have
> > been persued pubically over the last months:
> >
> > I) Create proper userspace tgt.git SG_IO and BSG passthrough into
> > TCM_Loop v4 using high-level multi-fabric WWPN emulation so that TCM
> > Core SPC-4 kernel emulation is exposed to STGT user fabrics, eg:
> > userspace fabric module -> kernel backstore passthrough. (DONE
> > for .37, and STGT passthrough + BSG backstore support merged into
> > tgt.git by Tomo-san)
> >
> > II) Complete target_core_stgt.c TCM subsystem plugin for kernel -> user
> > CDB -> LUN passthrough building upon existing
> > drivers/scsi/scsi_tgt_*.c code. (WIP for .38, made available
> > initially as a seperate standalone .ko module in lio-core-2.6.git)
>
> That would mean that if LIO merged in .37:
>
> 1. It would be merged _without_ STGT ABI compatibility, i.e. one of the
> requirements for a new SCSI target infrastructure merge is violated.
>

Sorry, but you are completely wrong here. This has nothing to do with a
question of STGT kernel 'ABI compatibility' (because there is only one
mainline user!), but has everything to do with being able to expose TCM
kernel level SPC-4 emulation, and make this logic available to existing
userspace fabrics in tgt.git. Again, we are talking about userspace
STGT fabric module <-> TCM kernel backstore support for .37, which has
already been merged by into tgt.git, and being used by other STGT users
for SG_IO and BSG passthrough.

> 2. It would _not_ be a drop in replacement for STGT, because STGT's
> drivers/scsi/scsi_tgt_*.c code would stay in the kernel. Those would
> effectively mean that another requirement for a new SCSI target
> infrastructure merge would be violated: there would be 2 SCSI target
> infrastructures in the kernel and any STGT in-kernel driver (for
> instance, built as an outside module) would continue working. My
> understanding of "drop in replacement" is "one out, another in at once".
>

Sorry, but this is just more generic handwaving on your part. What
constitutes a 'drop in replacement' for STGT is a decision that was to
be made by the STGT developers, not by you. target_core_stgt.c is an
extraordinarly transparent method of bringing drivers/scsi/scsi_tgt_*
logic into the TCM Core HBA/DEV design, and allows us to build upon and
improve the existing mainline kernel code.

> Please tell me where I'm wrong? I would appreciate if you be precise and
> answer the above 2 my concerns. There is no point to again repeat what
> you have already written without adding any new information.
>
> >>> Tomo-san, mnc, and other storage folks have been briefed on the
> >>> remaining issues that would be involved to get a prototype
> >>> functioning with drivers/target/target_core_stgt.c, and rough idea of
> >>> what it would take in existing mainline drivers/scsi/scsi_tgt_*.c
> >>> code. With the current WIP code this will allow the userspace CDB ->
> >>> LUN passthrough to function transparently with all TCM SPC-4
> >>> compliant logic as a standalone struct se_subsystem_api
> >>> tcm_core_stgt.ko backstore.
> >>
> >> This is exactly what we are discussing.
> >
> > Then I suggest you start working on a patch for drivers/scsi/scsi_tgt_*
> > in order to address what you believe are the preceived shortcomings of
> > the original design.
> >
> > Until you can do that, and actually take an impartial look at the
> > existing drivers/scsi/scsi_tgt_*.c, it would be a bit difficult to
> > beleive you genuinely want to steer our current level of interaction
> > away from continued hand-waving noise into a real rational technical
> > discourse between yourself and the LIO/STGT development community.
>
> My look is completely impartial. With all my respect, I'm just quite
> ahead of you and can see what you are unable (or don't want?) to see. I
> did what you are currently doing almost 4 years ago. I have already
> written all the necessary code and addressed all _rose on practice_
> concerns.

It is obvious to even an casual observer from watching the TCM/LIO patch
series that have been flying across the linux-scsi wire the last 24
months that the major features (including PR and ALUA, and new fabric
module drivers) have been developed individual feature bit by feature
bit using a distributed git workflow in a bisectable manner. Each
series was produced in such a manner that each patch could be reviewed
individually by those interested parties.

This is the big difference between our respective development processes.
This is the case not only for SCSI feature bits that TCM/LIO and SCST
share, but for features in TCM/LIO v4 that are unique and not available
in any other target implemention, anywhere. And yes, I am most
certainly talking about code beyond just the SCSI level fabric emulation
bits you are mentioning above.

> But all my attempts to explain my view are just blindly
> ignored without any considerations, so I have no idea how more I can
> explain it.
>

Then I suggest you learn a better way of communicating your ideas if you
really want to work with members of the LIO/STGT development
communities.

First, I suggest you start explaining your ideas with actual kernel code
that is 1) human readable and 2) presented in such a manner that makes
it accessable for others with skills possibly different (and greater)
than your own to review and give feedback.

Best,

--nab

2010-08-30 20:47:40

by Vladislav Bolkhovitin

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

Nicholas A. Bellinger, on 08/29/2010 12:47 AM wrote:
>>>>> As mentioned explictly earlier in this thread, my WIP code for the
>>>>> kernel level subsystem backstore using STGT kernel<-> user CDB
>>>>> passthrough logic in drivers/target/target_core_stgt.c is a item on
>>>>> my TODO list, but I will repeat, is NOT being tagged as a mainline
>>>>> .37 item.
>>>>
>>>> Hmm, I can't understand, if target_core_stgt.c is "NOT being tagged as a
>>>> mainline .37 item", then the STGT ABI compatibility for being a drop in
>>>> replacement for STGT isn't a requirement? Or am I missing something?
>>>
>>> Sorry, but I have no idea what you are trying to conjour up here.
>>>
>>> To spell out (again) the TCM/LIO<->STGT compatibility stages that have
>>> been persued pubically over the last months:
>>>
>>> I) Create proper userspace tgt.git SG_IO and BSG passthrough into
>>> TCM_Loop v4 using high-level multi-fabric WWPN emulation so that TCM
>>> Core SPC-4 kernel emulation is exposed to STGT user fabrics, eg:
>>> userspace fabric module -> kernel backstore passthrough. (DONE
>>> for .37, and STGT passthrough + BSG backstore support merged into
>>> tgt.git by Tomo-san)
>>>
>>> II) Complete target_core_stgt.c TCM subsystem plugin for kernel -> user
>>> CDB -> LUN passthrough building upon existing
>>> drivers/scsi/scsi_tgt_*.c code. (WIP for .38, made available
>>> initially as a seperate standalone .ko module in lio-core-2.6.git)
>>
>> That would mean that if LIO merged in .37:
>>
>> 1. It would be merged _without_ STGT ABI compatibility, i.e. one of the
>> requirements for a new SCSI target infrastructure merge is violated.
>>
>
> Sorry, but you are completely wrong here. This has nothing to do with a
> question of STGT kernel 'ABI compatibility' (because there is only one
> mainline user!), but has everything to do with being able to expose TCM
> kernel level SPC-4 emulation, and make this logic available to existing
> userspace fabrics in tgt.git. Again, we are talking about userspace
> STGT fabric module<-> TCM kernel backstore support for .37, which has
> already been merged by into tgt.git, and being used by other STGT users
> for SG_IO and BSG passthrough.

You are proving that I'm actually right ;). In the beginning of this
thread I offered a transition path from STGT to SCST and James rejected
it, because it didn't confirm a requirement that a new SCSI target
infrastructure must be STGT ABI compatible. But latter James left this
decision up to the STGT developers and now you are confirming that they
don't see any ABI compatibility issues, so my transition path fully
confirms all the requirements.

>> 2. It would _not_ be a drop in replacement for STGT, because STGT's
>> drivers/scsi/scsi_tgt_*.c code would stay in the kernel. Those would
>> effectively mean that another requirement for a new SCSI target
>> infrastructure merge would be violated: there would be 2 SCSI target
>> infrastructures in the kernel and any STGT in-kernel driver (for
>> instance, built as an outside module) would continue working. My
>> understanding of "drop in replacement" is "one out, another in at once".
>
> Sorry, but this is just more generic handwaving on your part. What
> constitutes a 'drop in replacement' for STGT is a decision that was to
> be made by the STGT developers, not by you. target_core_stgt.c is an
> extraordinarly transparent method of bringing drivers/scsi/scsi_tgt_*
> logic into the TCM Core HBA/DEV design, and allows us to build upon and
> improve the existing mainline kernel code.

I'm not deciding anything, I'm only analyzing and seeing a contradiction:

1. James wants only one SCSI target infrastructure in the kernel.

2. If drivers/scsi/scsi_tgt_*.c left after the merge of the new SCSI
target infrastructure, it would mean that STGT left as well => 2 SCSI
target infrastructure in the kernel.

For me it doesn't matter. I just want clear rules, hence asking for
clarification.

> It is obvious to even an casual observer from watching the TCM/LIO patch
> series that have been flying across the linux-scsi wire the last 24
> months that the major features (including PR and ALUA, and new fabric
> module drivers) have been developed individual feature bit by feature
> bit using a distributed git workflow in a bisectable manner. Each
> series was produced in such a manner that each patch could be reviewed
> individually by those interested parties.

That's a nice, but quite meaningless LIO advertisement. SCST is using
the same bisectable, distributed and reviewable workflow. If we had a
kernel.org git account, we would use it as well, but so far we are happy
with SF.net hosting. BTW, how was you able to get the git account
without a single patch merged in the kernel? You must have good
connections in the kernel community.

> This is the big difference between our respective development processes.
> This is the case not only for SCSI feature bits that TCM/LIO and SCST
> share, but for features in TCM/LIO v4 that are unique and not available
> in any other target implemention, anywhere. And yes, I am most
> certainly talking about code beyond just the SCSI level fabric emulation
> bits you are mentioning above.

Can you list us benefits for an average user of that work?

>> But all my attempts to explain my view are just blindly
>> ignored without any considerations, so I have no idea how more I can
>> explain it.
>>
>
> Then I suggest you learn a better way of communicating your ideas if you
> really want to work with members of the LIO/STGT development
> communities.
>
> First, I suggest you start explaining your ideas with actual kernel code
> that is 1) human readable and 2) presented in such a manner that makes
> it accessable for others with skills possibly different (and greater)
> than your own to review and give feedback.

I have sent patches twice, the second time few months ago. Weren't they
human readable and accessible for kernel developers who are experts in
dealing with sent by e-mail patches?

Thanks,
Vlad

2010-08-30 21:50:22

by Nicholas A. Bellinger

[permalink] [raw]
Subject: Re: [Scst-devel] Fwd: Re: linuxcon 2010...

On Tue, 2010-08-31 at 00:47 +0400, Vladislav Bolkhovitin wrote:
> Nicholas A. Bellinger, on 08/29/2010 12:47 AM wrote:
> >>>>> As mentioned explictly earlier in this thread, my WIP code for the
> >>>>> kernel level subsystem backstore using STGT kernel<-> user CDB
> >>>>> passthrough logic in drivers/target/target_core_stgt.c is a item on
> >>>>> my TODO list, but I will repeat, is NOT being tagged as a mainline
> >>>>> .37 item.
> >>>>
> >>>> Hmm, I can't understand, if target_core_stgt.c is "NOT being tagged as a
> >>>> mainline .37 item", then the STGT ABI compatibility for being a drop in
> >>>> replacement for STGT isn't a requirement? Or am I missing something?
> >>>
> >>> Sorry, but I have no idea what you are trying to conjour up here.
> >>>
> >>> To spell out (again) the TCM/LIO<->STGT compatibility stages that have
> >>> been persued pubically over the last months:
> >>>
> >>> I) Create proper userspace tgt.git SG_IO and BSG passthrough into
> >>> TCM_Loop v4 using high-level multi-fabric WWPN emulation so that TCM
> >>> Core SPC-4 kernel emulation is exposed to STGT user fabrics, eg:
> >>> userspace fabric module -> kernel backstore passthrough. (DONE
> >>> for .37, and STGT passthrough + BSG backstore support merged into
> >>> tgt.git by Tomo-san)
> >>>
> >>> II) Complete target_core_stgt.c TCM subsystem plugin for kernel -> user
> >>> CDB -> LUN passthrough building upon existing
> >>> drivers/scsi/scsi_tgt_*.c code. (WIP for .38, made available
> >>> initially as a seperate standalone .ko module in lio-core-2.6.git)
> >>
> >> That would mean that if LIO merged in .37:
> >>
> >> 1. It would be merged _without_ STGT ABI compatibility, i.e. one of the
> >> requirements for a new SCSI target infrastructure merge is violated.
> >>
> >
> > Sorry, but you are completely wrong here. This has nothing to do with a
> > question of STGT kernel 'ABI compatibility' (because there is only one
> > mainline user!), but has everything to do with being able to expose TCM
> > kernel level SPC-4 emulation, and make this logic available to existing
> > userspace fabrics in tgt.git. Again, we are talking about userspace
> > STGT fabric module<-> TCM kernel backstore support for .37, which has
> > already been merged by into tgt.git, and being used by other STGT users
> > for SG_IO and BSG passthrough.
>
> You are proving that I'm actually right ;). In the beginning of this
> thread I offered a transition path from STGT to SCST and James rejected
> it, because it didn't confirm a requirement that a new SCSI target
> infrastructure must be STGT ABI compatible. But latter James left this
> decision up to the STGT developers and now you are confirming that they
> don't see any ABI compatibility issues, so my transition path fully
> confirms all the requirements.
>

Again, I still have no idea what you are trying to conjour up. The
passthrough for STGT compatibility with TCM_Loop via SG_IO and BSG has
already been merged by Tomo-san into tgt.git. Futhermore, we are using
the same TCM_Loop high level multi-fabric WWPN emulation together with
new (and old) QEMU HBA emulation into KVM guest using SG_IO and BSG as
well.

> >> 2. It would _not_ be a drop in replacement for STGT, because STGT's
> >> drivers/scsi/scsi_tgt_*.c code would stay in the kernel. Those would
> >> effectively mean that another requirement for a new SCSI target
> >> infrastructure merge would be violated: there would be 2 SCSI target
> >> infrastructures in the kernel and any STGT in-kernel driver (for
> >> instance, built as an outside module) would continue working. My
> >> understanding of "drop in replacement" is "one out, another in at once".
> >
> > Sorry, but this is just more generic handwaving on your part. What
> > constitutes a 'drop in replacement' for STGT is a decision that was to
> > be made by the STGT developers, not by you. target_core_stgt.c is an
> > extraordinarly transparent method of bringing drivers/scsi/scsi_tgt_*
> > logic into the TCM Core HBA/DEV design, and allows us to build upon and
> > improve the existing mainline kernel code.
>
> I'm not deciding anything, I'm only analyzing and seeing a contradiction:
>
> 1. James wants only one SCSI target infrastructure in the kernel.
>
> 2. If drivers/scsi/scsi_tgt_*.c left after the merge of the new SCSI
> target infrastructure, it would mean that STGT left as well => 2 SCSI
> target infrastructure in the kernel.
>
> For me it doesn't matter. I just want clear rules, hence asking for
> clarification.

Seriously, arguing over the use of people's language from weeks ago once
the decision has already been made to move forward is really of little
value at this point.

>
> > It is obvious to even an casual observer from watching the TCM/LIO patch
> > series that have been flying across the linux-scsi wire the last 24
> > months that the major features (including PR and ALUA, and new fabric
> > module drivers) have been developed individual feature bit by feature
> > bit using a distributed git workflow in a bisectable manner. Each
> > series was produced in such a manner that each patch could be reviewed
> > individually by those interested parties.
>
> That's a nice, but quite meaningless LIO advertisement. SCST is using
> the same bisectable, distributed and reviewable workflow.

Actually, that is incorrect. Your project uses a centralized
development model, which has it's obvious limitiations in terms of
speed, flexability, and community scale. But really, don't take my word
for it, you can hear it for yourself directly from the horse's mouth
here:

http://www.youtube.com/watch?v=4XpnKHJAok8

I also very strongly suggest you find a transcript of this talk so you
can really understand what Linus means here wrt to a distributed
development workflow.

> If we had a
> kernel.org git account, we would use it as well, but so far we are happy
> with SF.net hosting. BTW, how was you able to get the git account
> without a single patch merged in the kernel? You must have good
> connections in the kernel community.

Please don't turn this into a kernel.org vs. SF.net thing.. There are
plenty of places that offer free git hosting (see github)

>
> > This is the big difference between our respective development processes.
> > This is the case not only for SCSI feature bits that TCM/LIO and SCST
> > share, but for features in TCM/LIO v4 that are unique and not available
> > in any other target implemention, anywhere. And yes, I am most
> > certainly talking about code beyond just the SCSI level fabric emulation
> > bits you are mentioning above.
>
> Can you list us benefits for an average user of that work?

Well, amougst the biggest advantages is actually having a sane ConfigFS
interface that can represent the parent / child relationship between
data structures across multiple LKMs. Not only does this give us a
proper representation of TCM and fabric data structures that can be
accesses directly by an advanced user in /sys/kernel/config/target/, it
also provides an ideal foundation to build 1) a userspace library in
intrepted languages and 2) create high level applications using said
userspace libraries that allow compatiblity to be contained in the lib
below.

But wrt to actual kernel code (because this is a kernel list), the
shortlog for the RFC posting below nicely sums up what TCM v4 is
bringing to the table:

http://lkml.org/lkml/2010/8/30/88


> >> But all my attempts to explain my view are just blindly
> >> ignored without any considerations, so I have no idea how more I can
> >> explain it.
> >>
> >
> > Then I suggest you learn a better way of communicating your ideas if you
> > really want to work with members of the LIO/STGT development
> > communities.
> >
> > First, I suggest you start explaining your ideas with actual kernel code
> > that is 1) human readable and 2) presented in such a manner that makes
> > it accessable for others with skills possibly different (and greater)
> > than your own to review and give feedback.
>
> I have sent patches twice, the second time few months ago. Weren't they
> human readable and accessible for kernel developers who are experts in
> dealing with sent by e-mail patches?

I think a proper kernel subsystem maintainer workflow has to do alot
more to do these days than just sending patches over email, than it did
say 10 years ago. Like it or not, git is the language of the mainline
kernel development process, and trying to imagine a *new* kernel
subsystem maintainer (besides say, someone like akpm) not using git is
quite a stretch of the imagination at this point.

Best,

--nab