Greetings all,
I recently patch-bombed the LIO-devel list with two different series of
patches against my work in the lio-core-2.6.git tree on kernel.org. Not
wanting to spam the kernel lists with these patches (not yet anyways :-)
I am posting the links to the patch series so that interested parties
could have a look at the current progress. The code is currently
running on v2.6.28-rc7 and will continue to be tracking linux-2.6.git.
Things are moving quickly for Target_Core_Mod/ConfigFS and LIO-Target
v3.0 development, and I invite folks to have a look at the code. To
dive into the rest of the v3.0 code via git-web, check out:
http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=tree;f=drivers/lio-core
They are made against against lio-core-2.6.git/master and tested on
v2.6.28-rc7.
For Target_Core_Mod/ConfigFS:
drivers/lio-core/target_core_base.h | 165 +++++-
drivers/lio-core/target_core_device.c | 122 ++++-
drivers/lio-core/target_core_device.h | 1 +
drivers/lio-core/target_core_fabric_ops.h | 17 +-
drivers/lio-core/target_core_file.c | 20 +-
drivers/lio-core/target_core_iblock.c | 16 +-
drivers/lio-core/target_core_pscsi.c | 15 +-
drivers/lio-core/target_core_rd.c | 26 +-
drivers/lio-core/target_core_rd.h | 4 +-
drivers/lio-core/target_core_reportluns.c | 340 +++++++++
drivers/lio-core/target_core_reportluns.h | 55 ++
drivers/lio-core/target_core_seobj.c | 20 +-
drivers/lio-core/target_core_seobj.h | 12 +-
drivers/lio-core/target_core_transport.c | 1067
++++++++++++++++------------
drivers/lio-core/target_core_transport.h | 124 ++--
15 files changed, 1411 insertions(+), 593 deletions(-)
create mode 100644 drivers/lio-core/target_core_reportluns.c
create mode 100644 drivers/lio-core/target_core_reportluns.h
[Target_Core_Mod 1/11]: Add se_* prefixed structures and definitons for generic target core
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/aee873be39d7f32f#
[Target_Core_Mod 2/11]: Add generic transport_get_lun_for_cmd()
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/60b00363b7cd6669#
[Target_Core_Mod 3/11]: Convert dev seobj code to se_cmd_t
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/bf0571ba8d98d6f5#
[Target_Core_Mod 4/11]: Make REPORT_LUNS generic to all $FABRIC_MODs
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/32f02a0ba13a5385#
[Target_Core_Mod 5/11]: Update struct target_core_fabric_ops
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/c0fadc6d4db25e2b#
[Target_Core_Mod 6/11]: Convert core engine to use generic CMD_TFO() callers
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/9ca8cc44af8cbba7#
[Target_Core_Mod 7/11]: Remove iSCSI dependent code from generic write pending codepath
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/f6b6931413a20976#
[Target_Core_Mod/PSCSI 8/11]: Remove iscsi_cmd code from PSCSI subsystem plugins
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/b4aea9852b2eab0a#
[Target_Core_Mod/IBLOCK 9/11]: Remove iscsi_cmd code from IBLOCK subsystem plugins
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/609aaad0b1a761e3#
[Target_Core_Mod/FILEIO 10/11]: Remove iscsi_cmd code from FILEIO subsystem plugins
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/de03d303f5a27d83#
[Target_Core_Mod/RAMDISK 11/11]: Remove iscsi_cmd code from RAMDISK_* subsystem plugins
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/1921518c262dbbac#
and for LIO-Target v3.0:
drivers/lio-core/Makefile | 2 +-
drivers/lio-core/iscsi_target.c | 333 ++++++++++++++++----------
drivers/lio-core/iscsi_target.h | 9 +
drivers/lio-core/iscsi_target_configfs.c | 17 +-
drivers/lio-core/iscsi_target_core.h | 100 ++-------
drivers/lio-core/iscsi_target_device.c | 115 ++--------
drivers/lio-core/iscsi_target_device.h | 2 +-
drivers/lio-core/iscsi_target_erl1.c | 29 ++-
drivers/lio-core/iscsi_target_erl2.c | 53 +++--
drivers/lio-core/iscsi_target_reportluns.c | 351 ----------------------------
drivers/lio-core/iscsi_target_reportluns.h | 52 ----
drivers/lio-core/iscsi_target_tmr.c | 19 ++-
drivers/lio-core/iscsi_target_util.c | 142 +++++++-----
drivers/lio-core/iscsi_target_util.h | 11 +-
14 files changed, 429 insertions(+), 806 deletions(-)
delete mode 100644 drivers/lio-core/iscsi_target_reportluns.c
delete mode 100644 drivers/lio-core/iscsi_target_reportluns.h
[LIO-Target 1/10]: Remove iSCSI REPORT_LUNS code
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/42132a2f99f3f1b2#
[LIO-Target 2/10]: Move generic code from iscsi_get_lun_for_cmd()
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/f6ae8deb036f93ed#
[LIO-Target 3/10]: Convert ErrorRecoveryLevel=1 code to use se_cmd_t
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/ebb8e185a2e67d06#
[LIO-Target 4/10]: Convert ErrorRecoveryLevel=2 code to use se_cmd_t
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/df0043bc3fd75323#
[LIO-Target 5/10]: Convert iSCSI Task Management code to use se_cmd_t
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/f1aa66b411d259c4#
[LIO-Target 6/10]: Add iscsi_allocate_se_cmd() and se_cmd_t usage
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/fa304b123df19b4f#
[LIO-Target 7/10]: Updates for iscsi_target_core.h
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/dbb209036ac78ef2#
[LIO-Target 8/10] Update iSCSI Target to use se_cmd_t structure and macros
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/fb39db1f12ede911#
[Target_Core_Mod/LIO-Target 9/10] Update Makefile for target_core_reportluns.o
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/be40736f783b53e7#
[LIO-Target/ConfigFS 10/10]: Update struct target_core_fabric_ops for iscsi_target_mod
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/8f9248905df3d217#
--nab
On Thu, Dec 11, 2008 at 4:14 AM, Nicholas A. Bellinger
<[email protected]> wrote:
> Things are moving quickly for Target_Core_Mod/ConfigFS and LIO-Target
> v3.0 development, and I invite folks to have a look at the code. To
> dive into the rest of the v3.0 code via git-web, check out:
>
> http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=tree;f=drivers/lio-core
I didn't have a look at the LIO code, but I ran it through checkpatch.
The result summary is as follows:
$ cat lio-checkpatch-output.txt | grep -E '^WARNING|^ERROR' | sort |
uniq -c | sort -rn
2419 ERROR: return is not a function, parentheses are not required
2303 WARNING: line over 80 characters
1950 ERROR: trailing whitespace
1801 WARNING: space prohibited between function name and open parenthesis '('
758 ERROR: code indent should use tabs where possible
698 ERROR: do not use assignment in if condition
407 WARNING: labels should not be indented
386 ERROR: spaces required around that ':' (ctx:VxW)
312 ERROR: do not use C99 // comments
309 WARNING: externs should be avoided in .c files
205 WARNING: printk() should include KERN_ facility level
103 WARNING: do not add new typedefs
62 ERROR: space required before that '*' (ctx:OxV)
58 WARNING: consider using a completion
55 WARNING: unnecessary cast may hide bugs, see
http://c-faq.com/malloc/mallocnocast.html
48 WARNING: consider using strict_strtoul in preference to
simple_strtoul
41 ERROR: spaces required around that ':' (ctx:VxV)
38 WARNING: EXPORT_SYMBOL(foo); should immediately follow its
function/variable
32 ERROR: Macros with multiple statements should be enclosed in a
do - while loop
28 ERROR: else should follow close brace '}'
27 WARNING: kfree(NULL) is safe this check is probably not
required
23 WARNING: braces {} are not necessary for any arm of this
statement
18 ERROR: open brace '{' following struct go on the same line
16 ERROR: spaces required around that '?' (ctx:VxW)
15 ERROR: space required after that ',' (ctx:VxV)
11 ERROR: switch and case should be at the same indent
8 WARNING: mutexes are preferred for single holder semaphores
8 WARNING: __func__ should be used instead of gcc specific
__FUNCTION__
8 WARNING: braces {} are not necessary for single statement
blocks
6 ERROR: spaces required around that '?' (ctx:VxE)
6 ERROR: space required before the open parenthesis '('
5 WARNING: suspect code indent for conditional statements (15,
23)
5 ERROR: Macros with complex values should be enclosed in
parenthesis
5 ERROR: Don't use kernel_thread(): see
Documentation/feature-removal-schedule.txt
5 ERROR: do not initialise externals to 0 or NULL
4 WARNING: suspect code indent for conditional statements (2, 4)
4 ERROR: spaces required around that ':' (ctx:VxE)
4 ERROR: space required after that ';' (ctx:VxV)
3 WARNING: suspect code indent for conditional statements (8,
17)
3 WARNING: suspect code indent for conditional statements (7,
15)
3 WARNING: suspect code indent for conditional statements (24,
33)
3 WARNING: suspect code indent for conditional statements (0, 0)
3 ERROR: trailing statements should be on next line
3 ERROR: that open brace { should be on the previous line
3 ERROR: spaces required around that '>=' (ctx:WxV)
3 ERROR: spaces required around that '&&' (ctx:VxV)
3 ERROR: spaces required around that '=' (ctx:VxV)
3 ERROR: do not initialise statics to 0 or NULL
2 WARNING: Use #include <linux/types.h> instead of <asm/types.h>
2 WARNING: suspect code indent for conditional statements (8, 0)
2 WARNING: suspect code indent for conditional statements (16,
25)
2 WARNING: consider using strict_strtol in preference to
simple_strtol
2 ERROR: need consistent spacing around '*' (ctx:WxV)
1 WARNING: Use #include <linux/byteorder.h> instead of
<asm/byteorder.h>
1 WARNING: trailing semicolon indicates no statements, indent
implies otherwise
1 WARNING: suspect code indent for conditional statements (9,
13)
1 WARNING: suspect code indent for conditional statements (8,
15)
1 WARNING: suspect code indent for conditional statements (6,
15)
1 WARNING: suspect code indent for conditional statements (4, 6)
1 WARNING: suspect code indent for conditional statements (33,
41)
1 WARNING: suspect code indent for conditional statements (32,
41)
1 WARNING: suspect code indent for conditional statements (16,
23)
1 WARNING: suspect code indent for conditional statements (16,
16)
1 WARNING: suspect code indent for conditional statements (0, 4)
1 WARNING: consider using strict_strtoull in preference to
simple_strtoull
1 ERROR: spaces required around that ':' (ctx:WxV)
1 ERROR: spaces required around that '=' (ctx:WxV)
1 ERROR: spaces required around that '<' (ctx:VxV)
1 ERROR: spaces required around that '==' (ctx:VxE)
1 ERROR: spaces prohibited around that '->' (ctx:VxW)
1 ERROR: space required before that '!' (ctx:VxE)
1 ERROR: space prohibited after that '!' (ctx:BxW)
1 ERROR: space prohibited after that '-' (ctx:BxW)
1 ERROR: open brace '{' following union go on the same line
1 ERROR: Missing Signed-off-by: line(s)
1 ERROR: "(foo*)" should be "(foo *)"
1 ERROR: "foo * bar" should be "foo *bar"
Bart.
On Thu, 2008-12-11 at 20:24 +0100, Bart Van Assche wrote:
> On Thu, Dec 11, 2008 at 4:14 AM, Nicholas A. Bellinger
> <[email protected]> wrote:
> > Things are moving quickly for Target_Core_Mod/ConfigFS and LIO-Target
> > v3.0 development, and I invite folks to have a look at the code. To
> > dive into the rest of the v3.0 code via git-web, check out:
> >
> > http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=tree;f=drivers/lio-core
>
> I didn't have a look at the LIO code, but I ran it through checkpatch.
> The result summary is as follows:
>
Hi Bart,
Thanks for the note, but checkpatch output on the entire tree (not just
the patchs I posted) does not really help the discussion. Obviously
this is on my list before submission for Target_Core_Mod/ConfigFS for
v2.6.29. However, what would be nice if you actually sent a kernel
patch for these issues.
--nab
> $ cat lio-checkpatch-output.txt | grep -E '^WARNING|^ERROR' | sort |
> uniq -c | sort -rn
> 2419 ERROR: return is not a function, parentheses are not required
> 2303 WARNING: line over 80 characters
> 1950 ERROR: trailing whitespace
> 1801 WARNING: space prohibited between function name and open parenthesis '('
> 758 ERROR: code indent should use tabs where possible
> 698 ERROR: do not use assignment in if condition
> 407 WARNING: labels should not be indented
> 386 ERROR: spaces required around that ':' (ctx:VxW)
> 312 ERROR: do not use C99 // comments
> 309 WARNING: externs should be avoided in .c files
> 205 WARNING: printk() should include KERN_ facility level
> 103 WARNING: do not add new typedefs
> 62 ERROR: space required before that '*' (ctx:OxV)
> 58 WARNING: consider using a completion
> 55 WARNING: unnecessary cast may hide bugs, see
> http://c-faq.com/malloc/mallocnocast.html
> 48 WARNING: consider using strict_strtoul in preference to
> simple_strtoul
> 41 ERROR: spaces required around that ':' (ctx:VxV)
> 38 WARNING: EXPORT_SYMBOL(foo); should immediately follow its
> function/variable
> 32 ERROR: Macros with multiple statements should be enclosed in a
> do - while loop
> 28 ERROR: else should follow close brace '}'
> 27 WARNING: kfree(NULL) is safe this check is probably not
> required
> 23 WARNING: braces {} are not necessary for any arm of this
> statement
> 18 ERROR: open brace '{' following struct go on the same line
> 16 ERROR: spaces required around that '?' (ctx:VxW)
> 15 ERROR: space required after that ',' (ctx:VxV)
> 11 ERROR: switch and case should be at the same indent
> 8 WARNING: mutexes are preferred for single holder semaphores
> 8 WARNING: __func__ should be used instead of gcc specific
> __FUNCTION__
> 8 WARNING: braces {} are not necessary for single statement
> blocks
> 6 ERROR: spaces required around that '?' (ctx:VxE)
> 6 ERROR: space required before the open parenthesis '('
> 5 WARNING: suspect code indent for conditional statements (15,
> 23)
> 5 ERROR: Macros with complex values should be enclosed in
> parenthesis
> 5 ERROR: Don't use kernel_thread(): see
> Documentation/feature-removal-schedule.txt
> 5 ERROR: do not initialise externals to 0 or NULL
> 4 WARNING: suspect code indent for conditional statements (2, 4)
> 4 ERROR: spaces required around that ':' (ctx:VxE)
> 4 ERROR: space required after that ';' (ctx:VxV)
> 3 WARNING: suspect code indent for conditional statements (8,
> 17)
> 3 WARNING: suspect code indent for conditional statements (7,
> 15)
> 3 WARNING: suspect code indent for conditional statements (24,
> 33)
> 3 WARNING: suspect code indent for conditional statements (0, 0)
> 3 ERROR: trailing statements should be on next line
> 3 ERROR: that open brace { should be on the previous line
> 3 ERROR: spaces required around that '>=' (ctx:WxV)
> 3 ERROR: spaces required around that '&&' (ctx:VxV)
> 3 ERROR: spaces required around that '=' (ctx:VxV)
> 3 ERROR: do not initialise statics to 0 or NULL
> 2 WARNING: Use #include <linux/types.h> instead of <asm/types.h>
> 2 WARNING: suspect code indent for conditional statements (8, 0)
> 2 WARNING: suspect code indent for conditional statements (16,
> 25)
> 2 WARNING: consider using strict_strtol in preference to
> simple_strtol
> 2 ERROR: need consistent spacing around '*' (ctx:WxV)
> 1 WARNING: Use #include <linux/byteorder.h> instead of
> <asm/byteorder.h>
> 1 WARNING: trailing semicolon indicates no statements, indent
> implies otherwise
> 1 WARNING: suspect code indent for conditional statements (9,
> 13)
> 1 WARNING: suspect code indent for conditional statements (8,
> 15)
> 1 WARNING: suspect code indent for conditional statements (6,
> 15)
> 1 WARNING: suspect code indent for conditional statements (4, 6)
> 1 WARNING: suspect code indent for conditional statements (33,
> 41)
> 1 WARNING: suspect code indent for conditional statements (32,
> 41)
> 1 WARNING: suspect code indent for conditional statements (16,
> 23)
> 1 WARNING: suspect code indent for conditional statements (16,
> 16)
> 1 WARNING: suspect code indent for conditional statements (0, 4)
> 1 WARNING: consider using strict_strtoull in preference to
> simple_strtoull
> 1 ERROR: spaces required around that ':' (ctx:WxV)
> 1 ERROR: spaces required around that '=' (ctx:WxV)
> 1 ERROR: spaces required around that '<' (ctx:VxV)
> 1 ERROR: spaces required around that '==' (ctx:VxE)
> 1 ERROR: spaces prohibited around that '->' (ctx:VxW)
> 1 ERROR: space required before that '!' (ctx:VxE)
> 1 ERROR: space prohibited after that '!' (ctx:BxW)
> 1 ERROR: space prohibited after that '-' (ctx:BxW)
> 1 ERROR: open brace '{' following union go on the same line
> 1 ERROR: Missing Signed-off-by: line(s)
> 1 ERROR: "(foo*)" should be "(foo *)"
> 1 ERROR: "foo * bar" should be "foo *bar"
>
> 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
>
On Thu, Dec 11, 2008 at 8:58 PM, Nicholas A. Bellinger
<[email protected]> wrote:
>
> On Thu, 2008-12-11 at 20:24 +0100, Bart Van Assche wrote:
>> On Thu, Dec 11, 2008 at 4:14 AM, Nicholas A. Bellinger
>> <[email protected]> wrote:
>> > Things are moving quickly for Target_Core_Mod/ConfigFS and LIO-Target
>> > v3.0 development, and I invite folks to have a look at the code. To
>> > dive into the rest of the v3.0 code via git-web, check out:
>> >
>> > http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=tree;f=drivers/lio-core
>>
>> I didn't have a look at the LIO code, but I ran it through checkpatch.
>> The result summary is as follows:
>>
> Thanks for the note, but checkpatch output on the entire tree (not just
> the patchs I posted) does not really help the discussion. Obviously
> this is on my list before submission for Target_Core_Mod/ConfigFS for
> v2.6.29. However, what would be nice if you actually sent a kernel
> patch for these issues.
I ran checkpatch on the files in drivers/lio-core ONLY. I did this by
converting all the files in drivers/lio-core through a script into a
patch, and by running checkpatch on the generated patch.
And as you already know I'm not going to work on LIO.
Bart.
On Fri, 2008-12-12 at 08:21 +0100, Bart Van Assche wrote:
> On Thu, Dec 11, 2008 at 8:58 PM, Nicholas A. Bellinger
> <[email protected]> wrote:
> >
> > On Thu, 2008-12-11 at 20:24 +0100, Bart Van Assche wrote:
> >> On Thu, Dec 11, 2008 at 4:14 AM, Nicholas A. Bellinger
> >> <[email protected]> wrote:
> >> > Things are moving quickly for Target_Core_Mod/ConfigFS and LIO-Target
> >> > v3.0 development, and I invite folks to have a look at the code. To
> >> > dive into the rest of the v3.0 code via git-web, check out:
> >> >
> >> > http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=tree;f=drivers/lio-core
> >>
> >> I didn't have a look at the LIO code, but I ran it through checkpatch.
> >> The result summary is as follows:
> >>
> > Thanks for the note, but checkpatch output on the entire tree (not just
> > the patchs I posted) does not really help the discussion. Obviously
> > this is on my list before submission for Target_Core_Mod/ConfigFS for
> > v2.6.29. However, what would be nice if you actually sent a kernel
> > patch for these issues.
>
> I ran checkpatch on the files in drivers/lio-core ONLY. I did this by
> converting all the files in drivers/lio-core through a script into a
> patch, and by running checkpatch on the generated patch.
>
> And as you already know I'm not going to work on LIO.
>
Too bad, because you are missing out on the most advanced ConfigFS
enabled storage engine on the planet.
Best regards,
--nab
> Bart.
>
> --~--~---------~--~----~------------~-------~--~----~
> You received this message because you are subscribed to the Google Groups "Linux-iSCSI.org Target Development" group.
> To post to this group, send email to [email protected]
> To unsubscribe from this group, send email to [email protected]
> For more options, visit this group at http://groups.google.com/group/linux-iscsi-target-dev?hl=en
> -~----------~----~----~----~------~----~------~--~---
>
On Fri, 12 Dec 2008 01:40:09 PST, "Nicholas A. Bellinger" said:
> Too bad, because you are missing out on the most advanced ConfigFS
> enabled storage engine on the planet.
OK. I *have* to ask.. ;)
What's the *second* most advanced configfs-enabled storage engine?
The only in-tree users of configfs I could find (grepping for the string
'config_item_init') were drivers/net/netconsole, fs/ocfs2, and fs/dlm, so
"most advanced configfs" is well into "world's tallest midget" territory...
On Sat, 2008-12-13 at 01:20 -0500, [email protected] wrote:
> On Fri, 12 Dec 2008 01:40:09 PST, "Nicholas A. Bellinger" said:
>
> > Too bad, because you are missing out on the most advanced ConfigFS
> > enabled storage engine on the planet.
>
> OK. I *have* to ask.. ;)
>
> What's the *second* most advanced configfs-enabled storage engine?
>
> The only in-tree users of configfs I could find (grepping for the string
> 'config_item_init') were drivers/net/netconsole, fs/ocfs2, and fs/dlm, so
> "most advanced configfs" is well into "world's tallest midget" territory...
>
It is advanced in the sense that it is the only user of ConfigFS that
spans across multiple kernel modules. In the context of the generic
target engine, this means that ConfigFS symlinks are used to create
Target Ports from the $FABRIC_MOD (like LIO-Target) to registered
$STORAGE_OBJECTS (storage devices available from Linux/SCSI, Linux/BLOCK
and Linux/VFS) within the generic target engine. The idea is that any
$FABRIC_MOD can create ConfigFS symlinks to the same $STORAGE_OBJECT,
and said $STORAGE_OBJECT can be shared across multiple $FABRIC_MODs.
Also, I cannot emphesize enough how much I think ConfigFS is the proper
direction for controlling a generic kernel level target engine, and
$FABRIC_MODs. It has made my work in lio-core-2.6.git simpler, and the
code easier to maintain and improve. I have implemented my own IOCTL,
Proc and SYSFS code for drivers over the years, and ConfigFS is the best
method that I have found to date for controlling a complex kernel
infrastructure across multiple plugin modules where config is driven by
userspace.
So I really invite folks to take a look at the ConfigFS for the generic
engine to see for themselves:
http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=blob;f=drivers/lio-core/target_core_configfs.c
and the LIO-Target iSCSI $FABRIC_MOD:
http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=blob;f=drivers/lio-core/iscsi_target_configfs.c
Unfortuately the detractors of ConfigFS on this list (who obviously have
their own motivations) can only offer hypothetical scenarios for
problems they forsee without writing a single line of ConfigFS code. I
however have no problems with the above running ConfigFS code, but these
folks have no interest in debating real running ConfigFS code, only
handwaving about their perceived limitiations of ConfigFS without
attempting to take the techincal points to linuxfs-devel, or contact the
ConfigFS author, etc.
Anyways, I appericate the comments, and invite you to have a look at the
running code for yourself.
Many thanks for your most valuable of time,
--nab
On Sat, Dec 13, 2008 at 7:20 AM, <[email protected]> wrote:
> On Fri, 12 Dec 2008 01:40:09 PST, "Nicholas A. Bellinger" said:
>
>> Too bad, because you are missing out on the most advanced ConfigFS
>> enabled storage engine on the planet.
>
> OK. I *have* to ask.. ;)
>
> What's the *second* most advanced configfs-enabled storage engine?
Please don't let Nicholas Bellinger mislead you. The most important
aspect of a storage engine is stability. Last January I ran
performance tests on IET, STGT, SCST and LIO. I quickly found out not
only that SCST had the highest throughput and the lowest latency, but
also that the performance tests on LIO triggered several kernel
crashes. I have reported these crashes to the linux-iscsi-target-dev
mailing list, but even as of today, it's not yet clear whether all of
these crashes have been fixed. During the last ten years I have filed
bug reports for many open source projects, and the LIO project is the
only project where the (kernel) crashes I reported were not addressed
immediately.
See also:
* February 1, 2008, LIO kernel panic during configuration,
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/74c8b37f24b84e59/d94c07626bd20521?lnk=gst&q=kernel+panic#d94c07626bd20521.
* February 8, 2008, kernel crash triggered by LIO,
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/30835aede1028188/5708e16a23367fb4?lnk=gst&q=kernel+crash#5708e16a23367fb4.
* February 13, 2008, LIO target kernel code triggers memory
corruption, http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/ddc1bf7666372972/2150a09f9ed3d1cd?lnk=gst&q=ipoib#2150a09f9ed3d1cd.
* February 18, 2008, LIO target makes entire system hang,
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/6a76f9efd9409fc5/55bd8840b6a5f757?lnk=gst&q=lio+target+hangs#55bd8840b6a5f757.
Bart.
On Sat, 2008-12-13 at 10:41 +0100, Bart Van Assche wrote:
> On Sat, Dec 13, 2008 at 7:20 AM, <[email protected]> wrote:
> > On Fri, 12 Dec 2008 01:40:09 PST, "Nicholas A. Bellinger" said:
> >
> >> Too bad, because you are missing out on the most advanced ConfigFS
> >> enabled storage engine on the planet.
> >
> > OK. I *have* to ask.. ;)
> >
> > What's the *second* most advanced configfs-enabled storage engine?
>
> Please don't let Nicholas Bellinger mislead you. The most important
> aspect of a storage engine is stability. Last January I ran
> performance tests on IET, STGT, SCST and LIO. I quickly found out not
> only that SCST had the highest throughput and the lowest latency, but
> also that the performance tests on LIO triggered several kernel
> crashes. I have reported these crashes to the linux-iscsi-target-dev
> mailing list, but even as of today, it's not yet clear whether all of
> these crashes have been fixed. During the last ten years I have filed
> bug reports for many open source projects, and the LIO project is the
> only project where the (kernel) crashes I reported were not addressed
> immediately.
>
> See also:
> * February 1, 2008, LIO kernel panic during configuration,
> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/74c8b37f24b84e59/d94c07626bd20521?lnk=gst&q=kernel+panic#d94c07626bd20521.
> * February 8, 2008, kernel crash triggered by LIO,
> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/30835aede1028188/5708e16a23367fb4?lnk=gst&q=kernel+crash#5708e16a23367fb4.
> * February 13, 2008, LIO target kernel code triggers memory
> corruption, http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/ddc1bf7666372972/2150a09f9ed3d1cd?lnk=gst&q=ipoib#2150a09f9ed3d1cd.
> * February 18, 2008, LIO target makes entire system hang,
> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/6a76f9efd9409fc5/55bd8840b6a5f757?lnk=gst&q=lio+target+hangs#55bd8840b6a5f757.
>
Heh.
I have no idea why why you keep bringing up a minor BUG (completely
unrelated to Target_Core_Mod/ConfigFS and LIO-Target v3.0 btw) that was
fixed 10 months ago..? Perhaps if you spent half the time looking at
actual lio-core-2.6.git code that you do bringing up minor closed bugs
from months ago, you and Vlad would actually understand how ConfigFS
works by now.
Best Regards,
--nab
On Sat, Dec 13, 2008 at 11:08 AM, Nicholas A. Bellinger <[email protected]> wrote:
>> See also:
>> * February 1, 2008, LIO kernel panic during configuration,
>> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/74c8b37f24b84e59/d94c07626bd20521?lnk=gst&q=kernel+panic#d94c07626bd20521.
>> * February 8, 2008, kernel crash triggered by LIO,
>> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/30835aede1028188/5708e16a23367fb4?lnk=gst&q=kernel+crash#5708e16a23367fb4.
>> * February 13, 2008, LIO target kernel code triggers memory
>> corruption, http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/ddc1bf7666372972/2150a09f9ed3d1cd?lnk=gst&q=ipoib#2150a09f9ed3d1cd.
>> * February 18, 2008, LIO target makes entire system hang,
>> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/6a76f9efd9409fc5/55bd8840b6a5f757?lnk=gst&q=lio+target+hangs#55bd8840b6a5f757.
>>
>
> I have no idea why why you keep bringing up a minor BUG (completely
> unrelated to Target_Core_Mod/ConfigFS and LIO-Target v3.0 btw) that was
> fixed 10 months ago..? Perhaps if you spent half the time looking at
> actual lio-core-2.6.git code that you do bringing up minor closed bugs
> from months ago [ ... ]
I won't comment on the fact that you consider a kernel crash or a
system hang as a minor bug.
I reported four bugs instead of one. Only two of these have been
reported to be fixed.
Bart.
On Sat, 2008-12-13 at 11:23 +0100, Bart Van Assche wrote:
> On Sat, Dec 13, 2008 at 11:08 AM, Nicholas A. Bellinger <[email protected]> wrote:
> >> See also:
> >> * February 1, 2008, LIO kernel panic during configuration,
> >> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/74c8b37f24b84e59/d94c07626bd20521?lnk=gst&q=kernel+panic#d94c07626bd20521.
> >> * February 8, 2008, kernel crash triggered by LIO,
> >> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/30835aede1028188/5708e16a23367fb4?lnk=gst&q=kernel+crash#5708e16a23367fb4.
> >> * February 13, 2008, LIO target kernel code triggers memory
> >> corruption, http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/ddc1bf7666372972/2150a09f9ed3d1cd?lnk=gst&q=ipoib#2150a09f9ed3d1cd.
> >> * February 18, 2008, LIO target makes entire system hang,
> >> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/6a76f9efd9409fc5/55bd8840b6a5f757?lnk=gst&q=lio+target+hangs#55bd8840b6a5f757.
> >>
> >
> > I have no idea why why you keep bringing up a minor BUG (completely
> > unrelated to Target_Core_Mod/ConfigFS and LIO-Target v3.0 btw) that was
> > fixed 10 months ago..? Perhaps if you spent half the time looking at
> > actual lio-core-2.6.git code that you do bringing up minor closed bugs
> > from months ago [ ... ]
>
> I won't comment on the fact that you consider a kernel crash or a
> system hang as a minor bug.
>
The problem is that you like to handwave on the technical issues, just
like you are doing here. :-) Of course I fix bugs when people report
them, but when people like yourself yell and scream and handwave, it
makes me not want to fix it as quickly if someone wrote a nice and
thoughful email and said 'thank you'.
Anyways, I am not going to debate the development process with you, and
as folks on the LIO-devel list can tell you, I am very quick to produce
patches when a issue is located.
> I reported four bugs instead of one. Only two of these have been
> reported to be fixed.
>
Considering you said earlier that you have not actually looked at any
recently LIO code, how could you know these bugs are fixed..? Back
here in the land of reality, these *TWO* bugs where fixed in back Feb.
One was related to iSCSI discovery, and one related to v2.6.24 kernel
breakage and struct scatterlist->page_link, so what..? Do you honestly
think handwaving about bugs from 10 months ago will get you anywhere
here..? If you are so certain these bugs still exist or have any effect
on my upstream work, then please, go ahead and prove it. No..? I did
not think so.
What does any of this have to do with lio-core-2.6.git,
Target_Core_Mod/ConfigFS and LIO-Target v3.0 btw..? Are you actually
going to write a thoughtful or relivent comment on the v3.0 design
and/or code, because that would be nice out of your for once. Otherwise
I am going to ignore you again, just the like conversation where you
said:
"Zero-copy means that data is copied as few times as possible".
when I was attempting to explain the finer pointers of
Target_Core_Mod/ConfigFS design to you and Vlad. Remember that one..?
http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/8cff61671cd2de6b/37ade00e607dd8c8
Perhaps you should clarify the techincal points I raised in that thread
(that where never answered) when I actually explained *WHY* I ended up
not using SCST as a base for Target_Core_Mod/ConfigFS. Perhaps folks
would like know why I found SCST Core's memory mapping and allocating
algortihms to be incomplete for my purposes, and that I had to give up
the discussion because neither you or Vlad would acknowledge the issues,
and would rather handwave and steamroll.
Perhaps you would like to address these points here and now, considering
you seem so jazzed on bringing up threads previously threads. How does
it make sense for you to bring up bugs from 10 months ago that where
fixed then months ago, when you guys don't even want to acknowledge any
techinical issues I bring up to you..?
Are you going to continue to refuse to acknowledge techincal concerns
about design fundementals about your memory pre-allocation and mapping
logic now that you have posted your patches to LKML..? Care to address
the issues in public where everyone can see, or would you rather keep
that on the SCST and LIO lists..?
Surely this type of behaviour is frowned upon by a subsystem maintainer.
Best regards,
--nab
> Bart.
>
> --~--~---------~--~----~------------~-------~--~----~
> You received this message because you are subscribed to the Google Groups "Linux-iSCSI.org Target Development" group.
> To post to this group, send email to [email protected]
> To unsubscribe from this group, send email to [email protected]
> For more options, visit this group at http://groups.google.com/group/linux-iscsi-target-dev?hl=en
> -~----------~----~----~----~------~----~------~--~---
>
On Sat, Dec 13, 2008 at 12:18 PM, Nicholas A. Bellinger
<[email protected]> wrote:
> Of course I fix bugs when people report them.
Things have changed then since the beginning of this year. As anyone
can see in the threads I referred to, you have done your best to deny
that the crashes and system hangs were caused by LIO, although I had
posted exact instructions on how to reproduce the bugs. Regarding
kernel integration and subsystem maintainership: one of the important
tasks of a maintainer is to verify whether reported bugs are
reproducible, and if so, to resolve them. I'm happy none of the
current kernel maintainers has the habitude of denying bug reports
that are 100% reproducible and which contain exact instructions about
how to reproduce the bug.
> "Zero-copy means that data is copied as few times as possible".
>
> when I was attempting to explain the finer pointers of
> Target_Core_Mod/ConfigFS design to you and Vlad. Remember that one..?
>
> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/8cff61671cd2de6b/37ade00e607dd8c8
You are going off topic -- the above statement has nothing to do with
this thread.
Regarding the statement itself: it's incorrect to quote that sentence
out of its context. In that thread, as anyone can see who looks up the
URL, I was using the word copies to refer to transfers between
hardware and RAM, not to copying data from RAM to RAM. Looking at that
statement now I agree that that was misleading, and that I should have
written that statement in another way.
Do you think people like it to discuss with you when you try to make
them appear ridiculous all the time ?
And what I do not understand is that you are playing games with the CC
list all of the time. It's considered impolite to leave out people
from a CC list unless someone asked explicitly to be left out.
Bart.
On Sat, 2008-12-13 at 12:56 +0100, Bart Van Assche wrote:
> On Sat, Dec 13, 2008 at 12:18 PM, Nicholas A. Bellinger
> <[email protected]> wrote:
> > Of course I fix bugs when people report them.
>
> Things have changed then since the beginning of this year. As anyone
> can see in the threads I referred to, you have done your best to deny
> that the crashes and system hangs were caused by LIO, although I had
> posted exact instructions on how to reproduce the bugs. Regarding
> kernel integration and subsystem maintainership: one of the important
> tasks of a maintainer is to verify whether reported bugs are
> reproducible, and if so, to resolve them. I'm happy none of the
> current kernel maintainers has the habitude of denying bug reports
> that are 100% reproducible and which contain exact instructions about
> how to reproduce the bug.
Heh, so I guess that means that you cannot show me where in
lio-core-2.6.git that the issues still exist. Why am I not suprised..?
Can you at least even back up your claim that the 10 month old BUGs you
posted have not been fixed, or was that just handwaving as well. Again
Bart, the only reason took a bit of extra time responding to you
because:
1) I was busy helping other people, and
2) The of the disrespectful tone of your emails turns me off
If this is the reasoning you cite for ignoring and badmouthing
Target_Core_Mod/ConfigFS, LIO-Target v3.0 and myself to the kernel
community at large, and making false claims about LIO stability (without
looking at an code, by your own admission!?), all I can say is: WoW!!
>
> > "Zero-copy means that data is copied as few times as possible".
> >
> > when I was attempting to explain the finer pointers of
> > Target_Core_Mod/ConfigFS design to you and Vlad. Remember that one..?
> >
> > http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/8cff61671cd2de6b/37ade00e607dd8c8
>
> You are going off topic -- the above statement has nothing to do with
> this thread.
>
Heh, and posting URLs to 10 month old BUGs that have been fixed 10
months ago in v2.9 on-topic for the Target_Core_Mod/ConfigFS v3.0
discussion..?
Like I said, if you would spend a fraction of the time you use trying to
discredit LIO code on trying to understand ConfigFS, or my points why I
decided not to use SCST as a base for Target_Core_Mod, perhaps you would
be apple to make real comments on worthwhile comments here, instead of
just your usual handwaving.
> Regarding the statement itself: it's incorrect to quote that sentence
> out of its context. In that thread, as anyone can see who looks up the
> URL, I was using the word copies to refer to transfers between
> hardware and RAM, not to copying data from RAM to RAM. Looking at that
> statement now I agree that that was misleading, and that I should have
> written that statement in another way.
>
The point is that neither you nor Vlad would acknowledge any of the
issues on that thread, or communicate with me in a professional manner
without the handwaving. How do you expect me to work with you if you
can't even acknowledge the short comings, or offer me a roadmap to get
them resolved when all you guys do is handwave..?
> Do you think people like it to discuss with you when you try to make
> them appear ridiculous all the time ?
>
I think that your definition of Zero-copy as "data is copied as few
times as possible" speaks for itself. I don't exactly know how that can
be taken out of context. Of course, that was just the tip of the
iceberg on that thread. Lets not even get into how you claimed RDMA
meant only userspace ops on virtual memory addresses using a vendor
specific API, or that RDMA using virtual addresses would be
communicating with drivers/scsi or block/ (which obviously use struct
page).
So honestly, I was much more concerned about the responses I got from
you and Vlad about core SCST core lacking important functionality for
$FABRIC_MODs using generic target infrastructure, and your inability to
acknowledge anything that you cannot or do not want to understand when
it comes to interface with linux storage subsystems in the context on
generic target mod.
Do you actually think people care about hearing that it look me an extra
day to address your issue 10 months ago..? :-) If you honestly think me
talking an extra day to address your issue (while I was busy with other
things at the same time) has any relivance to the upstream code
discussion, your are sorely mistaken.
> And what I do not understand is that you are playing games with the CC
> list all of the time. It's considered impolite to leave out people
> from a CC list unless someone asked explicitly to be left out.
>
Bart, no one wants to hear us argue, that is why I am saving them the
pain and removing them from the CC list. Any until you can start
showing me some real techincal content in a professional manner, I am
going to simply ignore your clumsy attempts at attacking me and m work.
--nab
> 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
>
On Sat, 2008-12-13 at 13:50 +0100, Bart Van Assche wrote:
> On Sat, Dec 13, 2008 at 1:33 PM, Nicholas A. Bellinger
> <[email protected]> wrote:
> > The point is that neither you nor Vlad would acknowledge any of the
> > issues on that thread.
>
> What that thread started with is whether or not higher-order
> allocations would help the performance of a storage target. I replied
> that the final argument in any discussion about performance are
> performance measurements. You failed to publish any performance
> numbers in that thread, which is why I stopped replying.
>
This was just one of the items that I mentioned that is implemented in
Target_Core_Mod/ConfigFS v3.0 that is lacking in SCST Core. The list
(which has not changed) is the following:
<SNIP>
The fudemental limitiation I ran into wrt SCST core has to do with
memory allocation (or rather, lack there of). The problem is that for
the upstream generic kernel target, the requirements are the following:
A single codepath memory allocating *AND* mapping for:
I) Every type of device_type
II) Every combination of max_sectors and sector_size with per PAGE_SIZE
segments and multiple contigious PAGE_SIZE memory segments
III) Every combination of I and II while receiving a CDB with
sector_count > $STORAGE_OBJECT->max_sectors
IV) Allocating multiple contigious struct page from the memory allocator
for I, II, III.
So, if we are talking about these first two, both target_core_mod and
SCST core have them (at least I think SCST has #2 with PAGE_SIZE memory
segments). I know that target_core_mod has #3 (because I explictly
designed it this way), and you have some patch for SCST to do this,
great!. However, your algortims currently assume PAGE_SIZE by default,
which is a problem for not just II, III, and IV above. :-(
V) Accept pre-registered memory segments being passed into
target_core_mod that are then mapped (NOT MEMCPY!!) to struct
scatterlist->page_link handling all cases (I, II, III, and IV) for
zero-copy DMA with multiple contigious PAGE_SIZE memory segments (Very
fast!!) using a *SINGLE* codepath down to every Linux Storage Subsystem
past, present and future. This is the target_core_mod design that has
existed since 2006.
</SNIP>
So, you are talking about IV) above, which is just one of the items. As
mentioned,the big item for me is V), which means you are going to have
to do some fundemental changes to SCST core to make this work. As
previously mentioned, these five design requirements have been part of
LIO-Target v2.x and Target_Core_Mod/ConfigFS v3.0 from the start.
> > Lets not even get into how you claimed RDMA
> > meant only userspace ops on virtual memory addresses using a vendor
> > specific API, or that RDMA using virtual addresses would be
> > communicating with drivers/scsi or block/ (which obviously use
> struct
> > page).
>
> I never claimed that RDMA is only possible from user space -- that was
> a misinterpretation of your side.
>
> I never referred to any vendor specific RDMA API.
>
> But I agree that the following paragraph I cited from Intel's VIA
> architecture document may be misleading:
>
> The VI provider is directly responsible for a number of functions nor-
> mally supplied by the operating system. The VI provider manages the
> pro-
> tected sharing of the network controller, virtual to physical
> translation of
> buffer addresses, and the synchronization of completed work via
> interrupts.
> The VI provider also provides a reliable transport service, with the
> level of re-
> liability depending upon the capabilities of the underlying network.
>
> I guess the above paragraph means that RDMA hardware must have
> scatter/gather support.
I have no idea why you keep mentioning Intel's VIA in the context of
RDMA and generic target mode. This API has *NOTHING* to do with a
target mode engine using generic algorithms for zero-copy struct page
mapping from RDMA capable hardware into Linux/SCSI, Linux/BLOCK or
Linux/VFS subsystems.
--nab
>
> Bart.
>
On Sat, 2008-12-13 at 12:56 +0100, Bart Van Assche wrote:
> On Sat, Dec 13, 2008 at 12:18 PM, Nicholas A. Bellinger
> <[email protected]> wrote:
> > Of course I fix bugs when people report them.
>
> Things have changed then since the beginning of this year. As anyone
> can see in the threads I referred to, you have done your best to deny
> that the crashes and system hangs were caused by LIO, although I had
> posted exact instructions on how to reproduce the bugs. Regarding
> kernel integration and subsystem maintainership: one of the important
> tasks of a maintainer is to verify whether reported bugs are
> reproducible, and if so, to resolve them. I'm happy none of the
> current kernel maintainers has the habitude of denying bug reports
> that are 100% reproducible and which contain exact instructions about
> how to reproduce the bug.
OK, All of you on this thread, why don't you take time out to step back
and think about the effects this descent into trench warfare is having
on your observers.
1. You're both saying the other side isn't production ready ...
it's not a stretch for the rest of us to take this at face
value ... about both of you.
2. This ideological opposition to features the other side
implements tells me that if it came to a choice, by going with
either one of you I'd get an incomplete feature set.
3. Making obvious partisans of your user base also tells me that if
I had to make a choice, whatever it was I'd piss off a large
number of people who'd be very vocal about it.
Since we have a working target solution in the kernel already (STGT),
why on earth would I be stupid enough to want to even consider either of
you, coming as you do with the above mentioned baggage?
So stop fighting ... you're not going to backstab your way to inclusion.
The only identified failing of STGT (and it's theoretical, not
demonstrated, although I can agree the theory looks correct) is that the
user space packet processing may cause performance problems on high
speed networks. We know from practical tests that these networks have
to be above 1Gbit because the results were identical for STGT and SCST
on a 1G network, so it's infiniband or 10Gbit ethernet.
So, what it comes down to is that if we had a kernel side protocol
accelerator for STGT, the project would no longer suffer from this
theoretical failing. *Both* of you have such a thing embedded in your
respective submissions (all 74k LOC of them) so can't you just enhance
STGT with whichever one is better ... actually, if you'd both bury the
hatchet and work on the enhancement together taking the best of each
project, we'd have something that worked much better and a unified user
base and neither side would be able to claim sole credit ... just a
thought.
James
Nicholas A. Bellinger wrote:
> On Sat, 2008-12-13 at 11:23 +0100, Bart Van Assche wrote:
>> On Sat, Dec 13, 2008 at 11:08 AM, Nicholas A. Bellinger <[email protected]> wrote:
>>>> See also:
>>>> * February 1, 2008, LIO kernel panic during configuration,
>>>> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/74c8b37f24b84e59/d94c07626bd20521?lnk=gst&q=kernel+panic#d94c07626bd20521.
>>>> * February 8, 2008, kernel crash triggered by LIO,
>>>> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/30835aede1028188/5708e16a23367fb4?lnk=gst&q=kernel+crash#5708e16a23367fb4.
>>>> * February 13, 2008, LIO target kernel code triggers memory
>>>> corruption, http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/ddc1bf7666372972/2150a09f9ed3d1cd?lnk=gst&q=ipoib#2150a09f9ed3d1cd.
>>>> * February 18, 2008, LIO target makes entire system hang,
>>>> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/6a76f9efd9409fc5/55bd8840b6a5f757?lnk=gst&q=lio+target+hangs#55bd8840b6a5f757.
>>>>
>>> I have no idea why why you keep bringing up a minor BUG (completely
>>> unrelated to Target_Core_Mod/ConfigFS and LIO-Target v3.0 btw) that was
>>> fixed 10 months ago..? Perhaps if you spent half the time looking at
>>> actual lio-core-2.6.git code that you do bringing up minor closed bugs
>>> from months ago [ ... ]
>> I won't comment on the fact that you consider a kernel crash or a
>> system hang as a minor bug.
>>
>
> The problem is that you like to handwave on the technical issues, just
> like you are doing here. :-) Of course I fix bugs when people report
> them, but when people like yourself yell and scream and handwave, it
> makes me not want to fix it as quickly if someone wrote a nice and
> thoughful email and said 'thank you'.
>
> Anyways, I am not going to debate the development process with you, and
> as folks on the LIO-devel list can tell you, I am very quick to produce
> patches when a issue is located.
>
>> I reported four bugs instead of one. Only two of these have been
>> reported to be fixed.
>>
>
> Considering you said earlier that you have not actually looked at any
> recently LIO code, how could you know these bugs are fixed..? Back
> here in the land of reality, these *TWO* bugs where fixed in back Feb.
> One was related to iSCSI discovery, and one related to v2.6.24 kernel
> breakage and struct scatterlist->page_link, so what..? Do you honestly
> think handwaving about bugs from 10 months ago will get you anywhere
> here..? If you are so certain these bugs still exist or have any effect
> on my upstream work, then please, go ahead and prove it. No..? I did
> not think so.
>
> What does any of this have to do with lio-core-2.6.git,
> Target_Core_Mod/ConfigFS and LIO-Target v3.0 btw..? Are you actually
> going to write a thoughtful or relivent comment on the v3.0 design
> and/or code, because that would be nice out of your for once. Otherwise
> I am going to ignore you again, just the like conversation where you
> said:
>
> "Zero-copy means that data is copied as few times as possible".
>
> when I was attempting to explain the finer pointers of
> Target_Core_Mod/ConfigFS design to you and Vlad. Remember that one..?
>
> http://groups.google.com/group/linux-iscsi-target-dev/browse_thread/thread/8cff61671cd2de6b/37ade00e607dd8c8
You know, that thread was finished when you refused to name which real
life tasks you are going to solve with your super advanced features.
I.e. answer, why are they needed at all?
Vlad
On Sat, 2008-12-13 at 09:24 -0600, James Bottomley wrote:
> On Sat, 2008-12-13 at 12:56 +0100, Bart Van Assche wrote:
> > On Sat, Dec 13, 2008 at 12:18 PM, Nicholas A. Bellinger
> > <[email protected]> wrote:
> > > Of course I fix bugs when people report them.
Hi James,
> OK, All of you on this thread, why don't you take time out to step back
> and think about the effects this descent into trench warfare is having
> on your observers.
>
Yes, I think this reflects very poorly on both of us. I would like to
apologize for my part in thread, and would like to start making forward
progress again. (eg: more patch bombs :-)
> 1. You're both saying the other side isn't production ready ...
> it's not a stretch for the rest of us to take this at face
> value ... about both of you.
So, as some of these larger LIO-Target customers come online in 2009
(including a 500 TB -> 1 PB installation), folks will be able to hear
for themselves how Linux-iSCSI.org code is being used for production.
Also, there will be full-time engineering resources put on
Target_Core_Mod/ConfigFS and LIO_Target v3.0 code in 2009, as we
continue to work towards code submission.
> 2. This ideological opposition to features the other side
> implements tells me that if it came to a choice, by going with
> either one of you I'd get an incomplete feature set.
Well, I have no ideological opposition to SCST, just technical concerns
that never get to be addressed. Which is fine with me, as the pieces I
need for LIO-iSER for generic struct page zero-copy mapping using a
single codepath to Linux storage subsystem that can be shared across
$FABRIC_MODs are already in place for Target_Core_Mod/ConfigFS. Its
simply a matter of hooking them (the $FABRIC_MODs) up now.
On the other side however, the SCST guys are completely opposed to any
usage of ConfigFS, which is a shame because it really does rock for this
type of usage: lots of groups, attributes, and $STORAGE_OBJECTS
representing Target Ports across multiple $FABRIC_MODs using ConfigFS
symlinks. Having written a bunch of ConfigFS code that has been up and
running for a few months (most of which was done at LPC), and yet still
some people make noise on their precieved limitiations of ConfigFS.,
without actually ever looking at the code in question. This obviously
is a bit of a let down.
The good news however is that there are folks in the kernel community
who have given me positive feedback about the use of ConfigFS for target
mode (and the actual code), and related to me that they believe ConfigFS
is the right choice moving forward. (A big thanks to people :-)
> 3. Making obvious partisans of your user base also tells me that if
> I had to make a choice, whatever it was I'd piss off a large
> number of people who'd be very vocal about it.
>
Honestly, I think our respective userbases just want the best upstream
kernel code possible, which is the only thing I am interested in.
> Since we have a working target solution in the kernel already (STGT),
> why on earth would I be stupid enough to want to even consider either of
> you, coming as you do with the above mentioned baggage?
>
> So stop fighting ... you're not going to backstab your way to inclusion.
>
Definately not.
> The only identified failing of STGT (and it's theoretical, not
> demonstrated, although I can agree the theory looks correct) is that the
> user space packet processing may cause performance problems on high
> speed networks. We know from practical tests that these networks have
> to be above 1Gbit because the results were identical for STGT and SCST
> on a 1G network, so it's infiniband or 10Gbit ethernet.
>
As I mentioned during our discussion @ LPC, I am still interested in
including support for STGT and userspace fabric modules in
Target_Core_Mod/ConfigFS using existing STGT code.
So, I am currently at the point where I am bringing LIO-Target v3.0
online using a generic target API on top of Target_Core_Mod/ConfigFS.
At this point, I am creating my own 'template' of function pointers that
generic target code calls into $FABRIC_MOD. However, I have been
considering using an existing 'template', and I am leaning to use the
existing the STGT template (moved up to kernel space). I think that
using an existing 'template' would help $FABRIC_MODs maintainers port
over their code. Same goes for STGT's struct iscsi_transport for the
LIO-iSER bits..
> So, what it comes down to is that if we had a kernel side protocol
> accelerator for STGT, the project would no longer suffer from this
> theoretical failing. *Both* of you have such a thing embedded in your
> respective submissions (all 74k LOC of them) so can't you just enhance
> STGT with whichever one is better ..
Ok, in terms of LOC, I am looking at ~22k LOC for
Target_Core_Mod/Configfs (which will be submitted first), and ~18K LOC
for LIO-Target v3.0 (submitted later)
> . actually, if you'd both bury the
> hatchet and work on the enhancement together taking the best of each
> project, we'd have something that worked much better and a unified user
> base and neither side would be able to claim sole credit ... just a
> thought.
>
I really do apperciate your thoughtful comments, but as long as ConfigFS
is being used, I do not believe the SCST folks have any interest in
working with me, which is OK. On the bit about claiming sole credit, I
very much understand that it 'takes a village' for something as complex
as what we are discussing here, and I am very much willing to share
credit where credit is due. I am 100% commited for the long haul to the
best generic kernel target infrastructure and $FABRIC_MODs, and am very
much looking forward to working with people who share this same passion
to produce the best possible code.
Many thanks for your most valuable of time,
--nab
On Sat, Dec 13, 2008 at 11:50 PM, Nicholas A. Bellinger
<[email protected]> wrote:
> On the other side however, the SCST guys are completely opposed to any
> usage of ConfigFS [ ... ]
It's sad to see that a few hours after James Bottomley asked to stop
fighting, that you are again spreading half truths about Vladislav and
myself. We are not opposed to any usage of configfs, but as Vladislav
carefully explained, we consider it unfortunate that the current
version of configfs doesn't suit SCST's needs.
Bart.
James Bottomley wrote:
> On Sat, 2008-12-13 at 12:56 +0100, Bart Van Assche wrote:
>> On Sat, Dec 13, 2008 at 12:18 PM, Nicholas A. Bellinger
>> <[email protected]> wrote:
>>> Of course I fix bugs when people report them.
>> Things have changed then since the beginning of this year. As anyone
>> can see in the threads I referred to, you have done your best to deny
>> that the crashes and system hangs were caused by LIO, although I had
>> posted exact instructions on how to reproduce the bugs. Regarding
>> kernel integration and subsystem maintainership: one of the important
>> tasks of a maintainer is to verify whether reported bugs are
>> reproducible, and if so, to resolve them. I'm happy none of the
>> current kernel maintainers has the habitude of denying bug reports
>> that are 100% reproducible and which contain exact instructions about
>> how to reproduce the bug.
>
> OK, All of you on this thread, why don't you take time out to step back
> and think about the effects this descent into trench warfare is having
> on your observers.
James,
I'm sorry you needed to intervene in such a manner. I don't want to
continue that LIO vs SCST fight, but I see in your message some
important misunderstandings about SCST, on which, I feel I need to reply
to clean them up.
> 1. You're both saying the other side isn't production ready ...
> it's not a stretch for the rest of us to take this at face
> value ... about both of you.
I listed in http://lkml.org/lkml/2008/12/10/245 the exact things, why
LIO is far from being production ready and can continue that list. In
fact, if to call things their real names, LIO is an iSCSI target which
in past few months in a hurry is being converted to a generic target
engine and which has a lo-o-ong way to go to complete the conversion.
I.e., in other words, LIO might be good as an iSCSI target, but as a
generic iSCSI target engine at the moment it simply *does not exist* yet.
Relating to SCST being not production ready, can Nicholas Bellinger
support his claims against SCST with something concrete? So far,
everything he has written was empty words not supported by any real
facts. For instance, he failed to describe for what all those "missed"
in SCST features are needed.
> 2. This ideological opposition to features the other side
> implements tells me that if it came to a choice, by going with
> either one of you I'd get an incomplete feature set.
There's no ideological opposition between SCST and LIO. Both engines are
built around basically the same ideology. The opposition is in
completely different and non-technical area.
> 3. Making obvious partisans of your user base also tells me that if
> I had to make a choice, whatever it was I'd piss off a large
> number of people who'd be very vocal about it.
Unfortunately, being based on an Open Source product isn't something
many people want to be proud of..
But here is the list of companies taken from scst-devel mailing list who
are working on SCST based products and made contributions in the past
half a year:
@storwize.com
@open-e.com
@enjellic.com
In the earlier time there were also contributions from @hp.com and
@systemfabricworks.com.
Also, I've already mentioned Mellanox, who developed SRP target driver
and now selling based on it product.
Also, there is a target driver development for Marvell SAS hardware by
an anonymous company, see
http://sourceforge.net/mailarchive/message.php?msg_id=e938503f0809260211r2d4ec37bt293c75c80960eadd%40mail.gmail.com
If you need more, I'll ask permissions from companies who already
selling SCST based products (BTW, 2 of them - user space VTLs, which can
be made on STGT, but those companies chose SCST).
It's worth to note here, that scst-devel mailing list has 134
subscribers. Many of them are from well known storage related companies.
Unfortunately, other sf.net statistics permanently loose data, hence not
trustworthy, so I can't refer to it.
> So stop fighting ... you're not going to backstab your way to inclusion.
>
> The only identified failing of STGT (and it's theoretical, not
> demonstrated, although I can agree the theory looks correct) is that the
> user space packet processing may cause performance problems on high
> speed networks. We know from practical tests that these networks have
> to be above 1Gbit because the results were identical for STGT and SCST
> on a 1G network, so it's infiniband or 10Gbit ethernet.
I thought that SRP measurements in http://lkml.org/lkml/2008/12/10/245
are sufficient to remove all your doubts. If you don't object, I'll
remind: there was a >50% improvement in IOPS on 4K writes (~150K vs
~100K), which relates to >200MB/s throughput increase, when, where
possible, processing was moved from kernel threads to tasklets. For STGT
any processing can't be moved to tasklets by design and context switches
between user space threads are a bit heavier, than between kernel
threads, + STGT has some syscall entry/exit overheads, hence for the
same processing done in STGT, the difference would be even more.
Thus, those measurements give the low boundary estimation of the
performance increase. Having such a huge increase on 4K block sizes is a
big advantage for any latency bound applications, like databases.
What else should we do to convince you?
Also, what I can't understand, why you don't want to count the
architectural advantages of SCST over STGT. Namely: overall simplicity,
possibility to implement many impossible for STGT features, like
complete pass-through and zero-copy cache IO. In fact, one such feature
has already been implemented: zero-copy transmit in iSCSI target. From
user space this is impossible, but for kernel I implemented it by very
small and simple patch.
> So, what it comes down to is that if we had a kernel side protocol
> accelerator for STGT, the project would no longer suffer from this
> theoretical failing. *Both* of you have such a thing embedded in your
> respective submissions (all 74k LOC of them) so can't you just enhance
> STGT with whichever one is better ... actually, if you'd both bury the
> hatchet and work on the enhancement together taking the best of each
> project, we'd have something that worked much better and a unified user
> base and neither side would be able to claim sole credit ... just a
> thought.
James, just think as if SCST in the current state is STGT in which all
the possible enhancements are already incorporated. It simply has been
cooking outside of the kernel for too long, so you didn't see the
intermediate steps. I'm not joking. I'm absolutely serious. And it is
true. Developing scst_user module I carefully studied STGT and scst_user
has everything it could take from it.
When you ask us to improve STGT step by step and implement a kernel side
protocol accelerator for it, you ask us to go back by 2+ years. For the
kernel side acceleration STGT needs to move the SCSI target state
machine and memory management into the kernel, which effectively means
to convert it to SCST. What should I do to make it clear for you?
Also, current integration of STGT with Linux (initiator) SCSI subsystem
should have a better design, I explained why in
http://lkml.org/lkml/2008/12/10/245. SCSI initiator and target has
almost nothing to share, so they should be separated.
I always open for any possible cooperation. Particularly, I'm always
willing to make with SCST any necessary changes, which will lead to
better target engine in Linux. But before doing any change I, as any
sane engineer, need to have answers on several simple questions.
Basically, there are 2 such questions:
1. For what the proposed action is needed? I.e., which real life task is
it going to solve?
2. Why is the proposed change the best one among possible implementation
alternatives?
If you simply take from
http://scst.sourceforge.net/patches/scst_combined.patch the combined
SCST patch, which has all 23 patches I submitted combined in a single
file (BTW, it has 46K LOC, not 76K), then patch some 2.6.27 tree and
spend a little time looking at it, you will soon find out that
converting STGT to SCST is the worst possible alternative. Simply try to
find out places, where STGT in-kernel core is better, than SCST core, or
has a feature, which SCST core doesn't have. There is only one such
feature: OSD support, i.e. bidirectional transfers, large CDBs, etc. It
wasn't implemented in SCST so far, because there was no demand for it
(hence, no way to test). But (1) this feature doesn't have any in-kernel
user, so nobody will be affected if STGT moved to be user space only,
and (2) there is nothing hard to add that feature to SCST, if there is
such demand.
I have been closely following development of both STGT and LIO since
their beginning, so my words based on close examination of their source
code, not on my rejection to look at it. They both inferior to SCST in
all main areas. I believe, there is no point to spend time improving
kernel side of STGT. Better to put effort to better integrate user space
part of STGT with scst_local SCST module as I described in
http://lkml.org/lkml/2008/12/10/245. If you don't agree with me, can you
answer on the question (2) above, please?
From everything I know SCST at the moment is the best open source SCSI
target engine in the world and no other target engines, including
Solaris's COMSTAR, can match it in functionality, performance and
stability areas.
James, you offered by already *completed* work, where everything
possible to improve STGT was already done, so why not simply accept it?
I'm an engineer, not a sales man, and there are no sales men in SCST
team to advertise it. We believe that the source code, its quality,
performance and feature completeness should speak theirself. It has been
in Linux so far and we hope will be so in this case. Just let the code
speak!
Sorry for taking your time by one more huge e-mail. I did my best to be
as laconic as possible.
Thanks,
Vlad