[PATCH] SCSI Core patches

[PATCH] SCSI Core patches

[Luben Tuikov]

The three incremental patches which I posted got mutilated
when pasted into Mozilla from the X clipboard buffer.
(TAB char got 2 spaces.)

The patches are also available, in pristine form, here:
http://www.splentec.com/~luben/patches.html

-- 
Luben

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Tue, Jan 07, 2003 at 08:56:42AM -0500, Luben Tuikov wrote:
> The three incremental patches which I posted got mutilated
> when pasted into Mozilla from the X clipboard buffer.
> (TAB char got 2 spaces.)
> 
> The patches are also available, in pristine form, here:
> http://www.splentec.com/~luben/patches.html

For use with multi-path, it is very useful to have the path in the
scsi_cmnd, and generally to not have the lldd's know about the scsi_device
at all, such that we can pick a path, and send a scsi_cmnd to a lldd.

Putting path information only in scsi_device makes it hard to do
multi-path below (or even in) the block layer, and makes it hard to
multi-path non block devices (like tape).

I would prefer we keep the current scsi_cmnd with its
host/channel/target[id]/lun.

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Luben Tuikov]

Patrick Mansfield wrote:
> 
> For use with multi-path, it is very useful to have the path in the
> scsi_cmnd, and generally to not have the lldd's know about the scsi_device
> at all, such that we can pick a path, and send a scsi_cmnd to a lldd.

It shouldn't know about the path either. :-)
No one is forcing LLDD to peek into the device struct (but it will
have to, see below).

> Putting path information only in scsi_device makes it hard to do
> multi-path below (or even in) the block layer, and makes it hard to
> multi-path non block devices (like tape).
> 
> I would prefer we keep the current scsi_cmnd with its
> host/channel/target[id]/lun.

For multipathing there should be *no* distinction between
char and block devices sitting in fabric or on a SCSI Bus.

Conceptually you'd want to say SEND(C,D), where C is the
command and D is the device, or if the device D is a property
of the command C, you can do SEND(C), and the device is C::D.

A path or paths to a device is a property of the device D.
 From any point of view (user, kernel), a command C needs to
be sent to the device D, or simply the command C needs to
be sent, (where the device is C::D).  The SEND() will check
the paths which are property of the device and will
perform the desired action to send it (C) along the chosen
path P.  I.e. D::A is a collection of paths P, (0/1:N), and is a
property of the device, thus you get C::D::A.
This is object oriented design.

Currently, host/channel/target/lun *is* D::A all in *one*,
i.e. it is a device and a single path in one ugly lump
in the command struct. Thus, the more reason to get rid of
it (at least partially).

There are other advantages to these patches:
1. No redundant data (OOD).
2. Transparent command struct allocation/deallocaton
    via scsi_get_command() / scsi_put_command().
3. Improved queuing logic for SCSI Core, not yet ready,
    I'm just waiting for more input on this current stuff.

There must be enough research papers on multipathing, and
OS IO books should have a chapter on it. I just don't
have time to go to my alma mater's sci. library and do the
research since I do all this in my spare time.

-- 
Luben

Re: [PATCH] SCSI Core patches

[Doug Ledford]

On Tue, Jan 07, 2003 at 10:21:27AM -0800, Patrick Mansfield wrote:
> On Tue, Jan 07, 2003 at 08:56:42AM -0500, Luben Tuikov wrote:
> > The three incremental patches which I posted got mutilated
> > when pasted into Mozilla from the X clipboard buffer.
> > (TAB char got 2 spaces.)
> > 
> > The patches are also available, in pristine form, here:
> > http://www.splentec.com/~luben/patches.html
> 
> For use with multi-path, it is very useful to have the path in the
> scsi_cmnd, and generally to not have the lldd's know about the scsi_device
> at all, such that we can pick a path, and send a scsi_cmnd to a lldd.
> 
> Putting path information only in scsi_device makes it hard to do
> multi-path below (or even in) the block layer, and makes it hard to
> multi-path non block devices (like tape).

Not true at all.  This is completely an implementation issue.  If you 
implement scsi multipath in certain ways, then this is true.  My preferred 
implementation for something like this wouldn't have this problem at all.  
My preferred implementation would have one scsi_device struct per path, 
but would only register the first/primary device with the block layer (or 
char layer as the case may be), and during scsi_scan detection the 
duplicate paths would be added to the primary path by using a list_struct 
item to link the devices.  Then you pick the path by picking the device 
struct you need.  The primary path goes away, you change the device 
registration to point to a new primary path scsi_device struct.  So I 
don't see the difficulty at all myself.

> I would prefer we keep the current scsi_cmnd with its
> host/channel/target[id]/lun.

One of the goals of this change is to eliminate scsi_build_commandblocks() 
entirely and do away with keeping a per-device list of pre-allocated and 
pre-filled in commands.  We could put the data items back into the struct, 
but that would then mean that scsi_get_command() would have to init the 
items from the device struct in question, then the lldd would init their 
command from these items.  In short, it would save us having to change all 
the lldd (which Luben has already done), but would cost us double init'ing 
items for no real purpose.  I would prefer to leave them out.

-- 
  Doug Ledford <dledford@redhat.com>     919-754-3700 x44233
         Red Hat, Inc. 
         1801 Varsity Dr.
         Raleigh, NC 27606
  

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Tue, Jan 07, 2003 at 02:23:32PM -0500, Luben Tuikov wrote:
> Patrick Mansfield wrote:
> > 
> > For use with multi-path, it is very useful to have the path in the
> > scsi_cmnd, and generally to not have the lldd's know about the scsi_device
> > at all, such that we can pick a path, and send a scsi_cmnd to a lldd.
> 
> It shouldn't know about the path either. :-)
> No one is forcing LLDD to peek into the device struct (but it will
> have to, see below).

Removing channel/target/lun from scsi_cmnd forces the LLDD to use the
scsi_device (scmd->device-> channel/id/lun).

> > Putting path information only in scsi_device makes it hard to do
> > multi-path below (or even in) the block layer, and makes it hard to
> > multi-path non block devices (like tape).
> > 
> > I would prefer we keep the current scsi_cmnd with its
> > host/channel/target[id]/lun.
> 
> For multipathing there should be *no* distinction between
> char and block devices sitting in fabric or on a SCSI Bus.

Agreed. Although retries must be handled differently for sequential and
direct-access devices.

> Conceptually you'd want to say SEND(C,D), where C is the
> command and D is the device, or if the device D is a property
> of the command C, you can do SEND(C), and the device is C::D.
> 
> A path or paths to a device is a property of the device D.
>  From any point of view (user, kernel), a command C needs to
> be sent to the device D, or simply the command C needs to
> be sent, (where the device is C::D).  The SEND() will check
> the paths which are property of the device and will
> perform the desired action to send it (C) along the chosen
> path P.  I.e. D::A is a collection of paths P, (0/1:N), and is a
> property of the device, thus you get C::D::A.
> This is object oriented design.

In order to allow selection of a path, we should abstract out the path
information, not lump it into a single pointer reference (i.e. not
scmd->device->lun) spread throughout the scsi mid and LLDD. If we had a
single macro or function to access the host/channel/target/lun, the issue
goes away, as there is only one point that we have to change for
multi-path. But if they are hardcoded into scsi_device, I'll have to
revert the changes for multi-path support.

Again, this is true whether we have multi-path support in the scsi
mid-level code or in the block layer.

> Currently, host/channel/target/lun *is* D::A all in *one*,
> i.e. it is a device and a single path in one ugly lump
> in the command struct. Thus, the more reason to get rid of
> it (at least partially).

For scsi_device yes, but the LLDD does not have to look at scsi_device to
send a command, it can currently use the values found in scsi_cmnd.

> There are other advantages to these patches:
> 1. No redundant data (OOD).
> 2. Transparent command struct allocation/deallocaton
>     via scsi_get_command() / scsi_put_command().
> 3. Improved queuing logic for SCSI Core, not yet ready,
>     I'm just waiting for more input on this current stuff.

I don't see how 2) and 3) are tied into your changes. I can understand
your changes leading to fewer changes for a common pool of all
scsi_cmnd's.  But, we could also have a single pool if the
host/channel/target/lun were set separtely from the allocation.

> There must be enough research papers on multipathing, and
> OS IO books should have a chapter on it. I just don't
> have time to go to my alma mater's sci. library and do the
> research since I do all this in my spare time.

I haven't found any good research papers, if anyone knows of some please
let me know (at least searching online, I should look in a real library).

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Luben Tuikov]

Patrick Mansfield wrote:
> 
> Removing channel/target/lun from scsi_cmnd forces the LLDD to use the
> scsi_device (scmd->device-> channel/id/lun).

It forces them just as much as it forces them to use
cmd->{lun, target, channel}. I.e. none at all.

All LLDD have been converted to use cmd->device->{lun, id, channel}.

You must agree that the tuple (COMMAND, (lun, target, channel))
doesn't say as much as (COMMAND, DEVICE). Anywhich way you look at it,
this is a better abstraction. LUN is a property of a *SCSI* device,
and not of a SCSI command.

SCSI LLDD provide transport, this is their function and existence. We
need to represent the devices they provide and struct scsi_device is
good for it. Multipathing shouldn't be stuffed into it. More below.

> In order to allow selection of a path, we should abstract out the path
> information, not lump it into a single pointer reference (i.e. not
> scmd->device->lun) spread throughout the scsi mid and LLDD. If we had a
> single macro or function to access the host/channel/target/lun, the issue
> goes away, as there is only one point that we have to change for
> multi-path. But if they are hardcoded into scsi_device, I'll have to
> revert the changes for multi-path support.

Right, right, but this is the *first* step; let's get it out of scsi command
struct first. This opens up enough opportunities already. I.e. we're moving
the pertinent information *up* where it belongs.

BTW, in order to be able to do C::D::A(P) stuff as I mentioned in my previous
letter, you'd have to have the device being an *abstract* device, and
here we're dealing with SCSI devices which always have a LUN, for example.
This is in SAM-3 and is unavoidable.

For this reason you might consider the suggestions by Doug, or research moving
multipathing higher up into the char/block layer, or thereabouts.

SCSI Core is *SCSI*, it's function well defined in delivery/receipt of SCSI
commands and doing discovery of devices.

Somewhere between write(fd, buf, count) and scsi_request_fn(), multipathing
will have to have already occured (ideally).

> For scsi_device yes, but the LLDD does not have to look at scsi_device to
> send a command, it can currently use the values found in scsi_cmnd.

You see, by abstractizing this way, you get rid of redundancies,
which let's you do all those nice things which Doug mentioned.

Getting rid of redundancies in data design and code design is important.

> I don't see how 2) and 3) are tied into your changes. I can understand
> your changes leading to fewer changes for a common pool of all
> scsi_cmnd's.  But, we could also have a single pool if the
> host/channel/target/lun were set separtely from the allocation.

2 says that you can just grab the command and ``hook'' it
on the device via its list member. The mechanism is very
versatile -- take a look at it.

3 comes from discussions with ppl, some research and my own
implementation of a mini-scsi-core, but basically playing
with the list member. Putting command on different queues
depending on its status, etc.

> I haven't found any good research papers, if anyone knows of some please
> let me know (at least searching online, I should look in a real library).

A real library is your best bet. Papers are sometimes not available online,
but only in their respective periodicals due to copyright.

-- 
Luben

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Tue, Jan 07, 2003 at 02:44:45PM -0500, Doug Ledford wrote:
> On Tue, Jan 07, 2003 at 10:21:27AM -0800, Patrick Mansfield wrote:

> > For use with multi-path, it is very useful to have the path in the
> > scsi_cmnd, and generally to not have the lldd's know about the scsi_device
> > at all, such that we can pick a path, and send a scsi_cmnd to a lldd.
> > 
> > Putting path information only in scsi_device makes it hard to do
> > multi-path below (or even in) the block layer, and makes it hard to
> > multi-path non block devices (like tape).
> 
> Not true at all.  This is completely an implementation issue.  If you 
> implement scsi multipath in certain ways, then this is true.  My preferred 
> implementation for something like this wouldn't have this problem at all.  
> My preferred implementation would have one scsi_device struct per path, 
> but would only register the first/primary device with the block layer (or 
> char layer as the case may be), and during scsi_scan detection the 
> duplicate paths would be added to the primary path by using a list_struct 
> item to link the devices.  Then you pick the path by picking the device 
> struct you need.  The primary path goes away, you change the device 
> registration to point to a new primary path scsi_device struct.  So I 
> don't see the difficulty at all myself.

The hard part is making sure all of the references to scsi_device values
are OK - that we correctly use and set the values across all of the
scsi_devices that represent a single scsi device. It would be easy for a
LLDD to screw this up, unless we have some sort of interface to get/set
every value in scsi_device.

Plus we have the overhead of redundant data for the fields: type,
scsi_level, inquiry, vendor, etc.

And the atomic device_active might have to be re-done with SMP locking.

I'm arguing for a linked list of structs that hold the minimum data we
need to use them as a path - much like a scsi_device with the redundant
fields removed (and if needed it can also point to the actual
scsi_device). And then plugging this data (or a pointer) into a scsi_cmnd
for use by the LLDD.

> > I would prefer we keep the current scsi_cmnd with its
> > host/channel/target[id]/lun.
> 
> One of the goals of this change is to eliminate scsi_build_commandblocks() 
> entirely and do away with keeping a per-device list of pre-allocated and 
> pre-filled in commands.  We could put the data items back into the struct, 
> but that would then mean that scsi_get_command() would have to init the 
> items from the device struct in question, then the lldd would init their 
> command from these items.  In short, it would save us having to change all 
> the lldd (which Luben has already done), but would cost us double init'ing 
> items for no real purpose.  I would prefer to leave them out.

As long as we have an interface (function or macro), I'm not strongly
opposed to the above. We can keep the scsi_allocate_device (in dire need
of a new name), and just have it allocate and init (or not) any fields as
needed, so there is only one place to change the init of the allocated
scsi_cmnd.

Adding a new interface to get the host/channel/target/lun is not so easy,
but IMO worthwhile.

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Tue, Jan 07, 2003 at 05:14:19PM -0500, Luben Tuikov wrote:
> Patrick Mansfield wrote:

> You must agree that the tuple (COMMAND, (lun, target, channel))
> doesn't say as much as (COMMAND, DEVICE). 

Yes, but the current code simply sends a scsi_cmnd to the LLDD, and the
LLDD need only reference the scsi_cmnd.

> Anywhich way you look at it,
> this is a better abstraction. 

Having the LLDD use more data structures does not imply a better
abstraction.  Again, a better abstraction would be to use interfaces to
get the host/channel/target/lun. This could add overhead depending on the
implementation.

> For this reason you might consider the suggestions by Doug, or research moving
> multipathing higher up into the char/block layer, or thereabouts.

I have looked into putting multipath in the block layer, I've looked at
md, at the T3 code, and at the qlogic multi-path code. Code in the block
layer would be interesting, but might not solve problems with char
devices, and would need further abstractions on top of some multi-path 
in the scsi layer (someone has to find and represent the devices and
paths, the block layer will not want to know about host/chan/target/lun,
nor will the block layer want to know details about scsi device or
transport errors).

> Somewhere between write(fd, buf, count) and scsi_request_fn(), multipathing
> will have to have already occured (ideally).

Putting the multipath abstraction into scsi_request_fn is IMO the best way
to go (as I've argued before) at least until other non-scsi block and
character devices support multi-pathing (maybe there are already
multi-path SATA devices, I don't know).

> > For scsi_device yes, but the LLDD does not have to look at scsi_device to
> > send a command, it can currently use the values found in scsi_cmnd.
> 
> You see, by abstractizing this way, you get rid of redundancies,
> which let's you do all those nice things which Doug mentioned.

> 
> Getting rid of redundancies in data design and code design is important.
> 

As mentioned, using a scsi_device both for a list of paths and as a
representation of scsi device duplicates data in multiple places.

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Luben Tuikov]

Patrick Mansfield wrote:
>
>>You must agree that the tuple (COMMAND, (lun, target, channel))
>>doesn't say as much as (COMMAND, DEVICE). 
> 
> 
> Yes, but the current code simply sends a scsi_cmnd to the LLDD, and the
> LLDD need only reference the scsi_cmnd.

No, it doesn't necessarily have to. The advantages of
cmd->device->{lun, id, channel} far outweights
``LLDD need only reference the scsi_cmnd'',
as we've mentioned numerous times.

I think the networking code has similar well thought out abstractions.

> Having the LLDD use more data structures does not imply a better
> abstraction.  Again, a better abstraction would be to use interfaces to
> get the host/channel/target/lun. This could add overhead depending on the
> implementation.

Aaaah, this seems like a lost cause. You seem to be arguing this just
because of sake of your changing your multipath code.

Why can't you just make those macros in your code and see how
it works out -- who knows, maybe you'll get a better design idea...
 
> I have looked into putting multipath in the block layer, I've looked at
> md, at the T3 code, and at the qlogic multi-path code. Code in the block
> layer would be interesting, but might not solve problems with char
> devices, and would need further abstractions on top of some multi-path 
> in the scsi layer (someone has to find and represent the devices and
> paths, the block layer will not want to know about host/chan/target/lun,
> nor will the block layer want to know details about scsi device or
> transport errors).

And this is *not* an excuse to leave lun, target and channel into the
scsi command. A command belongs to a device, and a device belongs
to a host, which belongs to a pci bus, etc...

Remember we're talking about SCSI devices, not generic devices.
SCSI devices, as mentioned in my previous email have and have not
certain properties.

> Putting the multipath abstraction into scsi_request_fn is IMO the best way
> to go (as I've argued before) at least until other non-scsi block and

Please keep in mind what SCSI Core is all about. Please don't mix and
match.

>>Getting rid of redundancies in data design and code design is important.
>>
> 
> 
> As mentioned, using a scsi_device both for a list of paths and as a
> representation of scsi device duplicates data in multiple places.

We are *not* discussing multipathing here.

-- 
Luben

Re: [PATCH] SCSI Core patches

[Luben Tuikov]

Patrick Mansfield wrote:
 >
> The hard part is making sure all of the references to scsi_device values
> are OK - that we correctly use and set the values across all of the
> scsi_devices that represent a single scsi device. It would be easy for a
> LLDD to screw this up, unless we have some sort of interface to get/set
> every value in scsi_device.

An LLDD cannot screw up.  It will be fixed in the testing phase and
given that so many eyes will check it out before it is even considered.
Plus, vendors usually test LLDDs long before they are considered
into a test tree.

> Plus we have the overhead of redundant data for the fields: type,
> scsi_level, inquiry, vendor, etc.

Implementation issue.

> And the atomic device_active might have to be re-done with SMP locking.

Implementation issue.

> I'm arguing for a linked list of structs that hold the minimum data we
> need to use them as a path - much like a scsi_device with the redundant
> fields removed (and if needed it can also point to the actual
> scsi_device). And then plugging this data (or a pointer) into a scsi_cmnd
> for use by the LLDD.

This may be an easier approach, but a scsi_cmnd stands for SCSI command.

Its a good thing to have multipathing represented as an ADT (abstract
data type).  Say something like a linked list of structs with
cost/weight member and a pointer to an actual low-level device (the infrastructure
will be more involved since multipathing will need to be notified
when a device goes away, but read further...).  Now this ADT may be embeded
into a block device and char device structs -- i.e. highest hierarchy.
(The assumption is that there won't be a device which will present
a char and block interface at the same time.)

Now since this ADT is embeded into the block/char device, we do get
notification when it goes away. So this should be ok.

The effect is that when write(fd, buf, count) is done, somewhere along
the way *before* the actual physical device (ide,scsi,etc) is referenced
multipathing has already been accomplished.  This preserves as much as possible
the current infrastructure of the kernel.

So when a low-level device (LLD) says that it cannot satisfy the request,
you *may* try another path.

In effect a low-level device *as seen from the multipath code* is the tuple
(PATH, DEVICE), and this is where a request is sent, i.e. to the tuple
(PATH, DEVICE). Where for each PATH, there can be exactly one DEVICE, since
a PATH describes a device (or access to it), but the opposite is not necessarily
true. I.e. for each DEVICE there can be zero/one to many PATHs.

> As long as we have an interface (function or macro), I'm not strongly
> opposed to the above. We can keep the scsi_allocate_device (in dire need
> of a new name), and just have it allocate and init (or not) any fields as
> needed, so there is only one place to change the init of the allocated
> scsi_cmnd.

The SCSI Core has a well-defined funtion. I'm not so sure that we
should muck it up with other well-defined functions, like multipathing.

> Adding a new interface to get the host/channel/target/lun is not so easy,
> but IMO worthwhile.

Changing all LLDD, SCSI Core, usb storage, message/fusion to use
cmd->device->{lun, id, channel, host} and scsi_get_command(),
scsi_put_command(), scsi_getset_command() was also not easy, but
doable.

There, you have a project. Please note, that this is only worthwhile
if multipathing is to go into SCSI Core.

It is *inevitable* that multipathing will be moved up into the
generic device char and block structs; sooner or later.

-- 
Luben

Re: [PATCH] SCSI Core patches

[Mike Anderson]

Luben Tuikov [luben@splentec.com] wrote:
> >I'm arguing for a linked list of structs that hold the minimum data we
> >need to use them as a path - much like a scsi_device with the redundant
> >fields removed (and if needed it can also point to the actual
> >scsi_device). And then plugging this data (or a pointer) into a scsi_cmnd
> >for use by the LLDD.
> 
> This may be an easier approach, but a scsi_cmnd stands for SCSI command.
> 

but struct scsi_device currently stands for more than the LLDD
scsi_device. It stands for a block request queue, scsi mid data, and
scsi host device data. 

While in the short term struct scsi_device already exists and appears to
be the right next progression to use. Future progression of abstraction
should look at some separation of the LLDD nexus device data and the
other struct scsi_device data. Maybe in the future part of struct scsi_device
will be a nexus handle given to the mid layer by the LLDD to represent
its object which will be plugged into the scsi_cmnd.

> Its a good thing to have multipathing represented as an ADT (abstract
> data type).  Say something like a linked list of structs with
> cost/weight member and a pointer to an actual low-level device (the 
> infrastructure
> will be more involved since multipathing will need to be notified
> when a device goes away, but read further...).  Now this ADT may be embeded
> into a block device and char device structs -- i.e. highest hierarchy.
> (The assumption is that there won't be a device which will present
> a char and block interface at the same time.)
> 
> Now since this ADT is embeded into the block/char device, we do get
> notification when it goes away. So this should be ok.
> 
> The effect is that when write(fd, buf, count) is done, somewhere along
> the way *before* the actual physical device (ide,scsi,etc) is referenced
> multipathing has already been accomplished.  This preserves as much as 
> possible
> the current infrastructure of the kernel.
> 
> So when a low-level device (LLD) says that it cannot satisfy the request,
> you *may* try another path.
> 
> In effect a low-level device *as seen from the multipath code* is the tuple
> (PATH, DEVICE), and this is where a request is sent, i.e. to the tuple
> (PATH, DEVICE). Where for each PATH, there can be exactly one DEVICE, since
> a PATH describes a device (or access to it), but the opposite is not 
> necessarily
> true. I.e. for each DEVICE there can be zero/one to many PATHs.
> 
> >As long as we have an interface (function or macro), I'm not strongly
> >opposed to the above. We can keep the scsi_allocate_device (in dire need
> >of a new name), and just have it allocate and init (or not) any fields as
> >needed, so there is only one place to change the init of the allocated
> >scsi_cmnd.
> 
> The SCSI Core has a well-defined funtion. I'm not so sure that we
> should muck it up with other well-defined functions, like multipathing.
> 

I would counter that moving device and transport knowledge into the
upper level might also muck up that level.

While it is not difficult to link paths together inside an object it
becomes more difficult to make good decisions on how to use the paths
beyond simple fail-over. There are device and platform specific
characteristics that effect performance and error policy. These
personalities exist in multipath OS core support in AIX, MS, and
DYNIX/ptx and vendor specific multi-path support drivers available for
most OS's. While the implementation is unique to the OS environment the
need for path personalities is common.

These type of issues are touched on in the mid-multipath document.

> It is *inevitable* that multipathing will be moved up into the
> generic device char and block structs; sooner or later.

The inevitable hopefully will wait until we get a few more versions of
the mid-level patch out and discuss code specific pro / cons of mid vs
upper layer implementations.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [PATCH] SCSI Core patches

[Andre Hedrick]

How about just a simple two layer with looping between the tow to hang
personalities on, and finally hooking the HBA's to handle exceptions only
against a native bus-phase (want to say state machine) fuzzy mung of, erm
SAM3 ??

Oh well not that important to model the transport/physical boundaries.

Cheers,

Andre Hedrick
LAD Storage Consulting Group

On Wed, 8 Jan 2003, Mike Anderson wrote:

> Luben Tuikov [luben@splentec.com] wrote:
> > >I'm arguing for a linked list of structs that hold the minimum data we
> > >need to use them as a path - much like a scsi_device with the redundant
> > >fields removed (and if needed it can also point to the actual
> > >scsi_device). And then plugging this data (or a pointer) into a scsi_cmnd
> > >for use by the LLDD.
> > 
> > This may be an easier approach, but a scsi_cmnd stands for SCSI command.
> > 
> 
> but struct scsi_device currently stands for more than the LLDD
> scsi_device. It stands for a block request queue, scsi mid data, and
> scsi host device data. 
> 
> While in the short term struct scsi_device already exists and appears to
> be the right next progression to use. Future progression of abstraction
> should look at some separation of the LLDD nexus device data and the
> other struct scsi_device data. Maybe in the future part of struct scsi_device
> will be a nexus handle given to the mid layer by the LLDD to represent
> its object which will be plugged into the scsi_cmnd.
> 
> > Its a good thing to have multipathing represented as an ADT (abstract
> > data type).  Say something like a linked list of structs with
> > cost/weight member and a pointer to an actual low-level device (the 
> > infrastructure
> > will be more involved since multipathing will need to be notified
> > when a device goes away, but read further...).  Now this ADT may be embeded
> > into a block device and char device structs -- i.e. highest hierarchy.
> > (The assumption is that there won't be a device which will present
> > a char and block interface at the same time.)
> > 
> > Now since this ADT is embeded into the block/char device, we do get
> > notification when it goes away. So this should be ok.
> > 
> > The effect is that when write(fd, buf, count) is done, somewhere along
> > the way *before* the actual physical device (ide,scsi,etc) is referenced
> > multipathing has already been accomplished.  This preserves as much as 
> > possible
> > the current infrastructure of the kernel.
> > 
> > So when a low-level device (LLD) says that it cannot satisfy the request,
> > you *may* try another path.
> > 
> > In effect a low-level device *as seen from the multipath code* is the tuple
> > (PATH, DEVICE), and this is where a request is sent, i.e. to the tuple
> > (PATH, DEVICE). Where for each PATH, there can be exactly one DEVICE, since
> > a PATH describes a device (or access to it), but the opposite is not 
> > necessarily
> > true. I.e. for each DEVICE there can be zero/one to many PATHs.
> > 
> > >As long as we have an interface (function or macro), I'm not strongly
> > >opposed to the above. We can keep the scsi_allocate_device (in dire need
> > >of a new name), and just have it allocate and init (or not) any fields as
> > >needed, so there is only one place to change the init of the allocated
> > >scsi_cmnd.
> > 
> > The SCSI Core has a well-defined funtion. I'm not so sure that we
> > should muck it up with other well-defined functions, like multipathing.
> > 
> 
> I would counter that moving device and transport knowledge into the
> upper level might also muck up that level.
> 
> While it is not difficult to link paths together inside an object it
> becomes more difficult to make good decisions on how to use the paths
> beyond simple fail-over. There are device and platform specific
> characteristics that effect performance and error policy. These
> personalities exist in multipath OS core support in AIX, MS, and
> DYNIX/ptx and vendor specific multi-path support drivers available for
> most OS's. While the implementation is unique to the OS environment the
> need for path personalities is common.
> 
> These type of issues are touched on in the mid-multipath document.
> 
> > It is *inevitable* that multipathing will be moved up into the
> > generic device char and block structs; sooner or later.
> 
> The inevitable hopefully will wait until we get a few more versions of
> the mid-level patch out and discuss code specific pro / cons of mid vs
> upper layer implementations.
> 
> -andmike
> --
> Michael Anderson
> andmike@us.ibm.com
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] SCSI Core patches

[James Bottomley]

I got around to looking at the patch.

I think the idea of slab allocation for Scsi_Cmnds is generally worth the 
effort.  There are several issues with the current patch, though:

1) The command for the device has to be allocated in space that respects the 
dma_mask for the device (on a non MMIO bus).  Pretty much for scsi, this 
translates to using GFP_DMA as the allocation flag if unchecked_isa_dma is 
set.  GFP_DMA has to be passed in at kmem_cache_create time.  We therefore 
probably need the capacity to use a different kmem_cache on a per Scsi_Host 
basis and the ability to set up a GFP_DMA kmem_cache if a host with 
unchecked_isa_dma appears.

2) scsi_get_command takes the host lock when obtaining a command from it's 
free_list.  Unfortunately, scsi_get_command looks to be called from places 
(like the prep_fn) where the lock is already held => deadlock.

3) The free_list employs all the list machinery for potentially storing 
multiple commands, but in practice it looks like it only ever stores one of 
them.  Is there a plan to make this free_list size tuneable (probably tuneable 
per device)?

James

Re: [PATCH] SCSI Core patches

[Luben Tuikov]

James Bottomley wrote:
> I got around to looking at the patch.
> 
> I think the idea of slab allocation for Scsi_Cmnds is generally worth the 
> effort.  There are several issues with the current patch, though:
> 
> 1) The command for the device has to be allocated in space that respects the 
> dma_mask for the device (on a non MMIO bus).  Pretty much for scsi, this 
> translates to using GFP_DMA as the allocation flag if unchecked_isa_dma is 
> set.  GFP_DMA has to be passed in at kmem_cache_create time.  We therefore 
> probably need the capacity to use a different kmem_cache on a per Scsi_Host 
> basis and the ability to set up a GFP_DMA kmem_cache if a host with 
> unchecked_isa_dma appears.

Some of our machines have 128 SCSI hosts.

I think that the slab cache can acknowledge a GFP_DMA flag even after
creation of the slab, but not 100% sure on this. I.e. in scsi_get_command()
a check of host->unchecked_isa_dma can be performed and the kmem_flags
OR-ed with GFP_DMA.

We can also have 2 slabs -- one for DMA capable hosts (current) and
another for non-DMA capable hosts.

Actually, I had this in the 2.5.52 version of this patch but decided that
the LLDD code will run as normal kernel code and that the actual PCI HOST
will never have to access the struct scsi_cmnd directly over the PCI bus;
but will only need to access the sg list. For this reason I decided to leave
it out. I.e. struct scsi_cmnd will alway be accessed in normal operation
of the kernel, and thus no need for GFP_DMA.
(I'd so much rather it be that way, so much... :-)

I've no problem changing the slab alloc. just let's decide if we
want a slab per host (kind of inefficient) or 2 slabs (DMA, and non-DMA)
or OR-ing the mask upon scsi_get_command().

> 2) scsi_get_command takes the host lock when obtaining a command from it's 
> free_list.  Unfortunately, scsi_get_command looks to be called from places 
> (like the prep_fn) where the lock is already held => deadlock.

Absolutely correct.

In this case I'll create a new spinlock which will be specifically for use
with the slab allocation. In fact, my preferred choice -- ``a lock for an object''.

In my version of mini-scsi-core I do not use spinlocks when accessing the slab
cache, but instead set the flag to GFP_ATOMIC or GFP_KERNEL as need be.
I can easily imagine a deadlock when we access it when our own spinlock
is obtained but the flag is GFP_KERNEL.

But the spinlock will be needed only to acess the backup store of command
structs (list), so I've no problem creating a scsi_cache_lock. Consider it done.

> 3) The free_list employs all the list machinery for potentially storing 
> multiple commands, but in practice it looks like it only ever stores one of 
> them.  Is there a plan to make this free_list size tuneable (probably tuneable 
> per device)?

Yes -- this was my intention (0:N commands in the backup store), and this was my
plan to make this tuneable in the (far) future.

Though I left it at one command when setting up the host, since I couldn't
assume much.

The problem is that the number of backup stored commands depends on quite a few
factors, and that will be one hell of a heuristic. So I'd imagine
it has to do with the available memory and the throughput (both in and out) of
commands between SCSI Core and LLDD. I.e. that number will be dynamically changing.
For this reason I left it at one.

-- 
Luben

Re: [PATCH] SCSI Core patches

[James Bottomley]

luben@splentec.com said:
> I think that the slab cache can acknowledge a GFP_DMA flag even after
> creation of the slab, but not 100% sure on this. I.e. in
> scsi_get_command() a check of host->unchecked_isa_dma can be performed
> and the kmem_flags OR-ed with GFP_DMA.

Unfortunately, not.  That's why the kmem caches that are used by kmalloc have 
two separate caches for each buddy size (one for zone normal and one for zone 
dma)---you see this in cat /proc/slabinfo.

The essential problem is that the zone flags must be known at the time slab 
gets fed from __get_free_pages().

> I've no problem changing the slab alloc. just let's decide if we want
> a slab per host (kind of inefficient) or 2 slabs (DMA, and non-DMA) or
> OR-ing the mask upon scsi_get_command(). 

The latter, I think.  We want the *ability* to have one slab per host, but 
we'd then assign each host either to the zone DMA or zone normal allocator.  
Then, when the wierd device that has a strange dma_mask comes along, it may 
use its own slab if necessary.

> Actually, I had this in the 2.5.52 version of this patch but decided
> that the LLDD code will run as normal kernel code and that the actual
> PCI HOST will never have to access the struct scsi_cmnd directly over
> the PCI bus; but will only need to access the sg list. For this reason
> I decided to leave it out. I.e. struct scsi_cmnd will alway be
> accessed in normal operation of the kernel, and thus no need for
> GFP_DMA. (I'd so much rather it be that way, so much... :-) 

I'd be happy about this too.  Just assure me that no legacy ISA drivers do dma 
from the Scsi_Cmnd structure and we can forget about the separate allocators 
and revoke the current promise that Scsi_Cmnd will be in memory dma'able by 
the driver.

James

Re: [PATCH] SCSI Core patches

[Christoph Hellwig]

On Tue, Jan 07, 2003 at 05:36:33PM -0800, Patrick Mansfield wrote:
> Having the LLDD use more data structures does not imply a better
> abstraction.  Again, a better abstraction would be to use interfaces to
> get the host/channel/target/lun. This could add overhead depending on the
> implementation.

Not having the cmnd replicate information in the device implies a better
abstraction :)  The LLDD knows about struct scsi_device anyway.

> > For this reason you might consider the suggestions by Doug, or research moving
> > multipathing higher up into the char/block layer, or thereabouts.
> 
> I have looked into putting multipath in the block layer, I've looked at
> md, at the T3 code, and at the qlogic multi-path code. Code in the block
> layer would be interesting, but might not solve problems with char
> devices,

Scsi character drivers _do_ go through the block layer.  (through
scsi_do_req).

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Sat, Jan 11, 2003 at 06:12:24PM +0000, Christoph Hellwig wrote:
> On Tue, Jan 07, 2003 at 05:36:33PM -0800, Patrick Mansfield wrote:
> > Having the LLDD use more data structures does not imply a better
> > abstraction.  Again, a better abstraction would be to use interfaces to
> > get the host/channel/target/lun. This could add overhead depending on the
> > implementation.
> 
> Not having the cmnd replicate information in the device implies a better
> abstraction :)  The LLDD knows about struct scsi_device anyway.

I'm not against removing the replication, but a scsi_get_lun etc. does
not imply one implementation or the other.

This is _not_ just for multi-path. The current scsi code does not properly
size the target and lun based on the transport. For SPI the target and lun
need only be one byte; for FCP they are 4 and 8 bytes; for iSCSI I don't
know what. (Our current four byte host-ordered lun does not match anything
in any SCSI spec.)

Having a scsi_nexus or such that is host adapter specific (something like
Mike A. posted about) possibly with common transport code (for FCP, SPI,
and iSCSI) would be useful, and would make it easier to add multi-pathing
or support for future transports. The scsi_device current_tag, sdtr, wdtr,
and ppr would be moved to a scsi_nexus since they are SPI specific.

The abstraction of the channel should not even be part of scsi_device or
the scsi mid-layer.

> > > For this reason you might consider the suggestions by Doug, or research moving
> > > multipathing higher up into the char/block layer, or thereabouts.
> > 
> > I have looked into putting multipath in the block layer, I've looked at
> > md, at the T3 code, and at the qlogic multi-path code. Code in the block
> > layer would be interesting, but might not solve problems with char
> > devices,
> 
> Scsi character drivers _do_ go through the block layer.  (through
> scsi_do_req).

Okay - the only blk code the char drivers use is the blk_insert_request
call, the char drivers never call the generic_make_request. So any
implementation that relies on make_request (like md) won't currently work
with char devices.

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Luben Tuikov]

Patrick Mansfield wrote:
 >
> This is _not_ just for multi-path. The current scsi code does not properly
> size the target and lun based on the transport. For SPI the target and lun
> need only be one byte; for FCP they are 4 and 8 bytes; for iSCSI I don't
> know what. (Our current four byte host-ordered lun does not match anything
> in any SCSI spec.)

I brought this up last year sometime, either privately or here (search
the archives if anyone is interested) and the idea of using a 64-bit LUN
was shot down as in ``we don't need so many bits''.

The problem is that due to its hierarchical structure all 64 bits are
used. If SPI wants to use 5 bits, let it use 5 bits, if iSCSI wants
to use 64, we've got this covered too.

But it would not be interpreted by SCSI Core, i.e. SCSI Core will simply
copy around the LUN.

BTW, a LUN is a well defined object, cf. SAM-3, 4.9.

> Having a scsi_nexus or such that is host adapter specific (something like
> Mike A. posted about) possibly with common transport code (for FCP, SPI,
> and iSCSI) would be useful, and would make it easier to add multi-pathing
> or support for future transports. The scsi_device current_tag, sdtr, wdtr,
> and ppr would be moved to a scsi_nexus since they are SPI specific.

SCSI Core should know as little as possible about the transport, IMHO.

Also I'd get rid of ``current_tag'', and leave only information as to
what kind of task management the device supports (BQue and CmdQue bits
in INQUIRY data). Also see:

http://MARC.10East.com/?l=linux-scsi&m=103488799412511&w=2

-- 
Luben

Re: [PATCH] SCSI Core patches

[Martin Peschke3]

Luben Tuikov <luben@splentec.com>@vger.kernel.org on 01/13/2003 10:30:05 PM

Sent by:    linux-scsi-owner@vger.kernel.org


To:    patmans@us.ltcfwd.linux.ibm.com
cc:    Christoph Hellwig <hch@infradead.org>, linux-scsi@vger.kernel.org
Subject:    Re: [PATCH] SCSI Core patches


Luben Tuitkov wrote:
> Patrick Mansfield wrote:
> >
> > This is _not_ just for multi-path. The current scsi code does not
properly
> > size the target and lun based on the transport. For SPI the target and
lun
> > need only be one byte; for FCP they are 4 and 8 bytes; for iSCSI I
don't
> > know what. (Our current four byte host-ordered lun does not match
anything
> > in any SCSI spec.)
>
> I brought this up last year sometime, either privately or here (search
> the archives if anyone is interested) and the idea of using a 64-bit LUN
> was shot down as in ``we don't need so many bits''.

I've been pleading for 64 bit values, too.
The FCP driver for zSeries I am maintaining for 2.4 uses a mapping
between 64 bit and 32 bit values. This is ugly and I'd like to get
rid of it in its 2.5 port if possible.

Such a change would raise an issue related to device discovery.
A sparsely populated name space for SCSI ID and LUN makes SCSI
device detection harder. For LUN, this should be fixed by the usage
of REPORT_LUNS. Additionally, lldd's could specify the size or
range of SCSI IDs and LUNs which apply to them in order to reduce
discovery overhead.

Furthermore, does anybody know what the largest IDs are which are
defined by any SCSI transport?

I think, the SCSI ID for FCP should also be 64 bit in size and
ought to be comprised of the World-Wide Port Name (WWPN) rather
than the more volatile 24 bit Destination ID (D_ID).

> > The problem is that due to its hierarchical structure all 64 bits are
> > used. If SPI wants to use 5 bits, let it use 5 bits, if iSCSI wants
> > to use 64, we've got this covered too.

Yes.

> > Having a scsi_nexus or such that is host adapter specific (something
like
> > Mike A. posted about) possibly with common transport code (for FCP,
SPI,
> > and iSCSI) would be useful, and would make it easier to add
multi-pathing
> > or support for future transports. The scsi_device current_tag, sdtr,
wdtr,
> > and ppr would be moved to a scsi_nexus since they are SPI specific.

Although the 2.5 stack has much improved, it is still too SPI-centric.
Changes to SCSI ID and SCSI LUN might evolve into such helper functions
concerned with transport specifics. Code for other characteristics
might be moved by degrees then too.


Martin Peschke

RE: [PATCH] SCSI Core patches

[Tony Battersby]

> Furthermore, does anybody know what the largest IDs are which are
> defined by any SCSI transport?

iSCSI identifies targets and initiators by world-wide unique name strings up
to 223 bytes in length.  An iSCSI initiator is additionally qualified by a
6-byte ISID value, and an iSCSI target is additionally qualified by a 16-bit
Portal Group Tag.  A full iSCSI I_T nexus is identified by the initiator
name string, the ISID, the target name string, and the portal group tag.

Anthony J. Battersby
Cybernetics

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Mon, Jan 13, 2003 at 04:30:05PM -0500, Luben Tuikov wrote:

> 
> I brought this up last year sometime, either privately or here (search
> the archives if anyone is interested) and the idea of using a 64-bit LUN
> was shot down as in ``we don't need so many bits''.
> 

Luben - 

I didn't mean we should never change to 64-bits, more along the lines of I
didn't find the effort of coding the change worth supporting more than
(our current) 32 bit lun. 

IMO a 64-bit lun and the hierarchy stuff (see the SAM spec, page 49 of
ftp://ftp.t10.org/t10/drafts/sam3/sam3r04.pdf) is a waste, but my opinion
won't change the spec. (BTW has anyone ever seen a target that supports
such a hierarchy, and is visible from the host, much like a switch?). IP
addresses are _still_ primarily 32 bits, can you imagine anyone with a
_single_ target (like a disk array) with 2^62 LUNs?  If there was no
target ID, a 64-bit lun might be sensible.

(And, it is a bit odd that the SCSI specs have a 64-bit LUN for interfaces
that only support 5 bit LUNs.)

With the current scsi code, going to a 64-bit lun means changing sdev->lun
and conditionally changing references to it to use a 64-bit struct
scsilun, or (mainly in the lldd) to call a conversion function if you must
use less than 64-bits. (Currently, no open source FCP adapters use an 8
byte lun to talk to the host adapter hardware, not even the qlogic
adapter.)

We could change scsilun_to_int to be a:

	scsilun_convert(biglun, size, &smaller_lun).

SPI could use something like: scsilun_convert(biglun, 1, &one_byte_lun)

The qlogic drivers would use: scislun_convert(biglun, 2, &two_byte_lun)

Then, there is an issue with shost->max_lun, it's currently an int.
max_lun or some other method (a shost function and/or flag) is needed so
the scsi scan code can figure out when to stop a serial scan, or when to
skip a LUN value via REPORT LUNS scanning if it exceeds the size supported
by the adapter. Then, always checking max_lun is wrong (for lldd with
64-bit LUN support), and a 64-bit max_lun seems wrong.

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Tue, Jan 14, 2003 at 05:19:30PM +0100, Martin Peschke3 wrote:
> 
> I've been pleading for 64 bit values, too.
> The FCP driver for zSeries I am maintaining for 2.4 uses a mapping
> between 64 bit and 32 bit values. This is ugly and I'd like to get
> rid of it in its 2.5 port if possible.
> 
> Such a change would raise an issue related to device discovery.
> A sparsely populated name space for SCSI ID and LUN makes SCSI
> device detection harder. For LUN, this should be fixed by the usage
> of REPORT_LUNS. Additionally, lldd's could specify the size or
> range of SCSI IDs and LUNs which apply to them in order to reduce
> discovery overhead.

Martin -

The current 2.5 scsi code uses REPORT LUNS, and has a 64 bit to 32 bit
converter (scsi 64 bit lun to linux sdev->lun, the sdev->lun is host byte
ordered), look at scsi_scan.c::scsilun_to_int. scsilun_to_int (it doesn't
handle the address method in the first two bits of the LUN, probably a bug
for some targets).

You can set the shost->max_id and shost->max_lun, and as an ugly hack set
them before the scsi scan occurrs (after scsi_register, before returning
from the detect or before calling the new for 2.5 scsi_add_host).

> I think, the SCSI ID for FCP should also be 64 bit in size and
> ought to be comprised of the World-Wide Port Name (WWPN) rather
> than the more volatile 24 bit Destination ID (D_ID).

I would rather the SCSI target ID be like an address rather than an
identifier (such as might be in a scsi_nexus), right now we have a
sdev->name that currently holds a device id.

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Luben Tuikov]

Patrick Mansfield wrote:
> On Mon, Jan 13, 2003 at 04:30:05PM -0500, Luben Tuikov wrote:
> 
> 
>>I brought this up last year sometime, either privately or here (search
>>the archives if anyone is interested) and the idea of using a 64-bit LUN
>>was shot down as in ``we don't need so many bits''.
>>
> 
> I didn't mean we should never change to 64-bits, more along the lines of I
> didn't find the effort of coding the change worth supporting more than
> (our current) 32 bit lun. 

You have to closely listen to the FC and iSCSI folks.

64-bit lun is *well* worth the effort.  As well as the primary
method of discovering luns being REPORT LUNS.  Anything which doesn't
support REPORT LUNS, but has more than a single LUN, should be considered
wierd/black listed/gray listed, etc.  Yes, I realise this is quite new,
but this is where things are going...

> IMO a 64-bit lun and the hierarchy stuff (see the SAM spec, page 49 of
> ftp://ftp.t10.org/t10/drafts/sam3/sam3r04.pdf) is a waste, but my opinion

I've read SAM-3 dosens of times, and the hierarchy organization is *not*
a waste when you think /storage networks/.

> won't change the spec. (BTW has anyone ever seen a target that supports

Exactly -- it won't change the spec.  64-bit lun is here to stay.
It is a real pain in the a*s to convert 64-bit lun to whatever the kernel
has decided that day to code LUNs as.

> such a hierarchy, and is visible from the host, much like a switch?). IP

Ideally, the ``host'' (old name), or ``initiator port'' (new name) should
not try to interpret LUN structure (a few exceptions of course).  LUN, as I
mentioned, should be blindly copied around by the kernel.  Application
clients and multipathing may be interested in LUN structure, but in general
not SCSI Core.

> addresses are _still_ primarily 32 bits, can you imagine anyone with a
> _single_ target (like a disk array) with 2^62 LUNs?  If there was no
> target ID, a 64-bit lun might be sensible.

Right, there's not going to be a ``target ID'' in SPI sense.  It gets
substituted with Target port/device name and identifiers.  SCSI Core
will decide what to use once those are finalised by T10.

BTW, this is still in discussion and development. If anyone is interested,
please see T10 document T10/02-419r1 or any later revision.

> (And, it is a bit odd that the SCSI specs have a 64-bit LUN for interfaces
> that only support 5 bit LUNs.)

But that's the *whole* point -- unification of transports.  This is what
SAM-3 is all about.  I.e. the transport should be transparent to the
application client.

> With the current scsi code, going to a 64-bit lun means changing sdev->lun
> and conditionally changing references to it to use a 64-bit struct
> scsilun, or (mainly in the lldd) to call a conversion function if you must
> use less than 64-bits.

Those are implementation issues, let's not go there right now, as they
are highly volatile and subject to change. (like gas and gas prices :-) )

> (Currently, no open source FCP adapters use an 8
> byte lun to talk to the host adapter hardware, not even the qlogic
> adapter.)

Well, give the SCSI/FC manufacturers a breather, they'll catch up.
BTW, any iSCSI device uses 64 bit luns.

The whole point is that the transport could be a *network* of transports
and one would want a unified representation.

> We could change scsilun_to_int to be a:
> 
> 	scsilun_convert(biglun, size, &smaller_lun).
> 
> SPI could use something like: scsilun_convert(biglun, 1, &one_byte_lun)
> 
> The qlogic drivers would use: scislun_convert(biglun, 2, &two_byte_lun)

I'd actually say just ``lun'' instead of ``biglun''.

Those two functions of course are to support only older LLDD, who never
knew about 64-bit luns.

Newer LLDD, even SPI and anything else, will do *their own* conversion
from 64-bit LUNS to whatever the transport uses. This will be transparent
to SCSI Core.

> Then, there is an issue with shost->max_lun, it's currently an int.
> max_lun or some other method (a shost function and/or flag) is needed so
> the scsi scan code can figure out when to stop a serial scan, or when to
> skip a LUN value via REPORT LUNS scanning if it exceeds the size supported
> by the adapter. Then, always checking max_lun is wrong (for lldd with
> 64-bit LUN support), and a 64-bit max_lun seems wrong.

Yes, one big mishmash of older and newer, broken and fixed hardware,
and patchwork solutions.

Right, there's no such thing as max_lun in 64-bit LUN space, since it
is not linear/enumerable, but hierarchical (space).

A couple of tiny notes is that, either a scsi device has a single LUN,
or it must support REPORT_LUNS (SPC-3); if a device reports the wrong
number LUNS from REPORT_LUNS, it is broken.

-- 
Luben

Re: [PATCH] SCSI Core patches

[Martin Peschke3]

Patrick,

> With the current scsi code, going to a 64-bit lun means changing
sdev->lun
> and conditionally changing references to it to use a 64-bit struct
> scsilun, or (mainly in the lldd) to call a conversion function if you
must
> use less than 64-bits.

Why would a conversion function be required?
Have we a conversion function for our current 32 bit LUNS which is larger
than required by our Parallel SCSI HBA drivers?
What would chang for those drivers if we go to 64 bit LUNs?

>(Currently, no open source FCP adapters use an 8
> byte lun to talk to the host adapter hardware, not even the qlogic
> adapter.)

Here you are under a misapprehension
http://www-124.ibm.com/developerworks/opensource/linux390
/current2_4_19-may2002.shtml#kernel20021125

Using 8 byte LUNs for FCP is not a question of driver-to-HBA communication
but is required by FCP.
ftp://ftp.t11.org/t10/drafts/fcp2/fcp2r07a.pdf
An HBA dd has to put an 8 byte LUN into the control block used for
SCSI command transfer (FCP_CMND IU), for example.
I am wondering how those drivers make up an 8 byte LUN
from a 32 bit LUN.


Martin Peschke

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Tue, Jan 14, 2003 at 09:01:36PM +0100, Martin Peschke3 wrote:
> 
> Patrick,
> 
> > With the current scsi code, going to a 64-bit lun means changing
> sdev->lun
> > and conditionally changing references to it to use a 64-bit struct
> > scsilun, or (mainly in the lldd) to call a conversion function if you
> must
> > use less than 64-bits.
> 
> Why would a conversion function be required?

Because a lot of conversions are done via truncation via int -> char
conversions done by the compiler, or explicit use of an int assuming that
it is a host ordered integer, and that won't work for a 64-bit scsilun.
You cannot pull one byte (at a fixed location) out of the 64-bit of the
scsilun, mainly AFAICT because of the addressing modes. Maybe we could
have a char *lun in scsi_device, and point to somewhere in the 64-bit lun,
but this would still require code changes.

> Have we a conversion function for our current 32 bit LUNS which is larger
> than required by our Parallel SCSI HBA drivers?

Not an explicit one, just what the C code does for us.

> What would chang for those drivers if we go to 64 bit LUNs?

So they would have to use a function or a pointer to lun, if that would
really work, but I prefer a function.

> >(Currently, no open source FCP adapters use an 8
> > byte lun to talk to the host adapter hardware, not even the qlogic
> > adapter.)
> 
> Here you are under a misapprehension
> http://www-124.ibm.com/developerworks/opensource/linux390
> /current2_4_19-may2002.shtml#kernel20021125

Sorry :)

> Using 8 byte LUNs for FCP is not a question of driver-to-HBA communication
> but is required by FCP.

Yes, so the driver-to-HBA interface does not have to be use 8 byte LUNs
(not that I agree with that approach), but FCP must use 8 bytes.

> ftp://ftp.t11.org/t10/drafts/fcp2/fcp2r07a.pdf
> An HBA dd has to put an 8 byte LUN into the control block used for
> SCSI command transfer (FCP_CMND IU), for example.
> I am wondering how those drivers make up an 8 byte LUN
> from a 32 bit LUN.

The qlogic uses a 2 byte lun for host -> adapter communication, even
though it sends an 8 byte lun to the target. For most targets, just using
the upper two bytes is OK, I don't know what they do for the different
scsi addressing modes.

PS: My (ibm's?) linux-scsi feed seems slow or dead (again).

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Martin Peschke3]

Patrick,

> The current 2.5 scsi code uses REPORT LUNS,

ok, I've been aware of that

> and has a 64 bit to 32 bit
> converter (scsi 64 bit lun to linux sdev->lun, the sdev->lun is host byte
> ordered)

What is the reasoning behind that? Why does it exchange bytes 0+1 and
bytes 2+3?

> look at scsi_scan.c::scsilun_to_int. scsilun_to_int (it doesn't
> handle the address method in the first two bits of the LUN, probably a
bug
> for some targets).

It is a bug, I think.
When sending SCSI commands, an FCP device expects me to provide exactly
that 8 byte LUN that is reported by REPORT_LUNS. How is it possible
for an HBA driver that needs a working 8 byte LUN to revert all these
modifications done by scsilun_to_int?
Ok, it would be easy to exchange bytes again. But it is impossible to
recover from the loss of 4 bytes and the 2 most significant bits.
The scsilun_to_int routine does not make sense to me.
We have to go with the actual LUNs retrieved by means of
the REPORT_LUNS command.

> > I think, the SCSI ID for FCP should also be 64 bit in size and
> > ought to be comprised of the World-Wide Port Name (WWPN) rather
> > than the more volatile 24 bit Destination ID (D_ID).
>
> I would rather the SCSI target ID be like an address rather than an
> identifier (such as might be in a scsi_nexus), right now we have a
> sdev->name that currently holds a device id.

How persistent is a SCSI ID expected to be?
Do SCSI applications in a Linux 2.5 environment, e.g. your favourite
burning
software, depend on persistent SCSI ID's in order to address the right
device?
Or, are all issues related to persistency solved by the combination of
sysfs
and hotplug, especially when considering SCSI support?

Thanks advance for your answers. They might be helpfull for me
when trying to solve the 32 bit vs. 64 bit issue in our HBA driver.


Martin Peschke

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

Martin -

On Tue, Jan 14, 2003 at 09:37:13PM +0100, Martin Peschke3 wrote:
> > and has a 64 bit to 32 bit
> > converter (scsi 64 bit lun to linux sdev->lun, the sdev->lun is host byte
> > ordered)
> 
> What is the reasoning behind that? Why does it exchange bytes 0+1 and
> bytes 2+3?

I wanted the low two bytes to be the same as we would see for sequential
scans, so no adapter driver code had to change. As you noted, if someone
wanted to use the other two bytes, they would have to convert them. The
qlogicfc.c driver has this code to ensure big endian byte order:

	cmd->expanded_lun = cpu_to_le16(Cmnd->lun);

> > look at scsi_scan.c::scsilun_to_int. scsilun_to_int (it doesn't
> > handle the address method in the first two bits of the LUN, probably a
> bug
> > for some targets).
> 
> It is a bug, I think.

Yes, I'm not sure how to fix it other than to shove the address method in
there, and handle the "Extended logical unit addressing method" (11b)
address mode properly. Someone with affected hardware will have to patch
and test it.

> When sending SCSI commands, an FCP device expects me to provide exactly
> that 8 byte LUN that is reported by REPORT_LUNS. How is it possible
> for an HBA driver that needs a working 8 byte LUN to revert all these
> modifications done by scsilun_to_int?

> Ok, it would be easy to exchange bytes again. But it is impossible to
> recover from the loss of 4 bytes and the 2 most significant bits.
> The scsilun_to_int routine does not make sense to me.
> We have to go with the actual LUNs retrieved by means of
> the REPORT_LUNS command.

It's not possible for all cases, if the scsi_report_lun_scan function
finds a LUN value out of range it should output a warning.  This really
depends on the target device, and possibly the adapter. If the target only
uses the upper four bytes of the scsi lun (or the first and second levels
of the address), and the addressing mode is "Peripheral device addressing
method" (00b), you can convert that back to an 8 byte lun zero filling the
other four bytes.

> > I would rather the SCSI target ID be like an address rather than an
> > identifier (such as might be in a scsi_nexus), right now we have a
> > sdev->name that currently holds a device id.
> 
> How persistent is a SCSI ID expected to be?
> Do SCSI applications in a Linux 2.5 environment, e.g. your favourite
> burning
> software, depend on persistent SCSI ID's in order to address the right
> device?
> Or, are all issues related to persistency solved by the combination of
> sysfs
> and hotplug, especially when considering SCSI support?

(Assuming you mean what we put into sdev->name right now not a target ID.)

There was discussion about this last week or so, generally we aren't using
it now, and we can't rely on the ID, we need a white list, gregkh is
planning to work on sysfs hotplug device naming like support:

http://marc.theaimsgroup.com/?t=104189838600004&r=1&w=2

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Martin Peschke3]

Patrick,

> Yes, so the driver-to-HBA interface does not have to be use 8 byte LUNs
> (not that I agree with that approach), but FCP must use 8 bytes.

No. This is a contradiction in terms.
Still, I can't see how to have a valid 8 byte LUN at hand for SCSI
devices using 8 byte LUNs if we apply a lossy compression to LUNs
in the SCSI stack.

> The qlogic uses a 2 byte lun for host -> adapter communication, even
> though it sends an 8 byte lun to the target. For most targets, just using
> the upper two bytes is OK, I don't know what they do for the different
> scsi addressing modes.

However this is done in detail, it is guessing in place of reliable
addressing.

I would like to come to an agreement that the method of scsilun_to_int
is wrong and that we have to care for 64 bit LUNs in the SCSI core,
i.e. store LUNs as they are provided by the REPORT_LUNs response and
passing them unchanged to HBA drivers.

The discussion about the implementation should be done after that as
the second step.


Martin Peschke

Re: [PATCH] SCSI Core patches

[Patrick Mansfield]

On Tue, Jan 14, 2003 at 10:29:16PM +0100, Martin Peschke3 wrote:
> 
> Patrick,
> 
> > Yes, so the driver-to-HBA interface does not have to be use 8 byte LUNs
> > (not that I agree with that approach), but FCP must use 8 bytes.
> 
> No. This is a contradiction in terms.
> Still, I can't see how to have a valid 8 byte LUN at hand for SCSI
> devices using 8 byte LUNs if we apply a lossy compression to LUNs
> in the SCSI stack.

As long as the truncated values are to be zero, it is not a problem.

> > The qlogic uses a 2 byte lun for host -> adapter communication, even
> > though it sends an 8 byte lun to the target. For most targets, just using
> > the upper two bytes is OK, I don't know what they do for the different
> > scsi addressing modes.
> 
> However this is done in detail, it is guessing in place of reliable
> addressing.
> 
> I would like to come to an agreement that the method of scsilun_to_int
> is wrong and that we have to care for 64 bit LUNs in the SCSI core,
> i.e. store LUNs as they are provided by the REPORT_LUNs response and
> passing them unchanged to HBA drivers.

I agree with that, I'm not planning on creating a patch for it. Someone
with the proper adapter and storage should probably work on a patch, and
there is currently (AFAIK!) only one open source driver that can handle an
8 byte lun :)
 
> The discussion about the implementation should be done after that as
> the second step.

I'd rather see a patch that has been tested with actual hardware.

Doug Gilbert has pointed out that we could have dual support for short
(current style) and long luns, and not bother fixing adapters that do not
or cannot support an 8 byte lun (all of SPI, and the current qlogic ones).

-- Patrick Mansfield

Re: [PATCH] SCSI Core patches

[Martin Peschke3]

Patrick,

> > I would like to come to an agreement that the method of scsilun_to_int
> > is wrong and that we have to care for 64 bit LUNs in the SCSI core,
> > i.e. store LUNs as they are provided by the REPORT_LUNs response and
> > passing them unchanged to HBA drivers.
>
> I agree with that, I'm not planning on creating a patch for it. Someone
> with the proper adapter and storage should probably work on a patch, and
> there is currently (AFAIK!) only one open source driver that can handle
an
> 8 byte lun :)

Ok, I've got it. I'll try to come up with a proposal and an appropriate
patch
as a base for further discussion. I have the hardware environment ready
at hand. But prior to testing such a patch, some more work needs to be
done in our HBA driver in order to heave it from 2.4 to 2.5.

If anybody on the list has objections against 64 bit LUNs, it is now time
to
speak out.


Martin Peschke

Re: [PATCH] SCSI Core patches

[James Bottomley]

MPESCHKE@de.ibm.com said:
> If anybody on the list has objections against 64 bit LUNs, it is now
> time to speak out. 

I don't have any objections, but I do have a suggestion:

It would be nice to be able to use opaque quantites for target and LUN so that 
the FC devices don't have to do this mapping dance between targed and internal 
name.  i.e. if a FC driver wants to use the WWN or port for the target there 
should be nothing preventing it.

All that really matters is that the target and LUN can be rendered to and from 
a string (for printing out and for accepting from add/remove-single-device).  
While we're in the middle of pulling these fields out of Scsi_Cmnd is a good 
point to start on this.

Most of the mid-layer uses Scsi_Device and doesn't care what the values are, 
the only piece that will need careful thought is the legacy scanning code.

James

Re: [PATCH] SCSI Core patches

[Mike Anderson]

James Bottomley [James.Bottomley@steeleye.com] wrote:
> I don't have any objections, but I do have a suggestion:
> 
> It would be nice to be able to use opaque quantites for target and LUN so that 
> the FC devices don't have to do this mapping dance between targed and internal 
> name.  i.e. if a FC driver wants to use the WWN or port for the target there 
> should be nothing preventing it.
> 

I believe back in this thread I was trying to say this with my nexus
object, but maybe poorly. 

Dynix/ptx had a similar model with its unit numbers. Every node in
the config tree had one but only the object knew how to decode it. It
was just passed around as part of the config object.

> All that really matters is that the target and LUN can be rendered to
> and from a string (for printing out and for accepting from
> add/remove-single-device).  

The kobjects are close to what your are talking about (see the
kobject.txt doc in the kernel). The have a few entries that we might
not need, but the do not need to be exported to sysfs. They carry a
kobj_type pointer that provides objs to show or store the values in the
object. A transport specific lib could contain common routines
which the LLDD could use or provide there own ops.

-andmike
--
Michael Anderson
andmike@us.ibm.com

Re: [PATCH] SCSI Core patches

[Luben Tuikov]

Mike Anderson wrote:
 >
> object. A transport specific lib could contain common routines
> which the LLDD could use or provide there own ops.

Uuuuh, I think that this will be too bulky for its purpose.

All we want is to present the target port/device name/identifier,
aka ``target ID'', and the LUN.  Both of those are fairly simple
and trivial to implement.

I do not think that the LLDD should be burdened with more callbacks
for this.  This makes it too messy and too complicated for no reason.

As was suggested before by someone, and I concurred, SCSI Core can
do translations for old and dumb drivers, but newer LLDD, would
accept an actual LUN (64 bits), and an actual target identifier.

I.e. the LLDD shouldn't know what hit them.

Latest news is that SPI is completely going away from SPC-3 and SAM-3
which would make it more worthwhile to get this going.

-- 
Luben