Skip to contentSkip to conversation

SCSI Core patches

PATCH In review Luben Tuikov 7 participants 20 messages Jan 7, 2003 – Jan 14, 2003 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 SCSI Core patches

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

Signature · 1 line
Luben
#1 lore raw
Quoted 6 lines · 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

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

#2 lore raw
Quoted 4 lines · Patrick Mansfield

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).

Quoted 6 lines

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.

Signature · 1 line
Luben
#3 lore raw
Quoted 9 lines · Luben Tuikov
Quoted 4 lines · Patrick Mansfield

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).

Quoted 9 lines
Quoted 6 lines

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.

Quoted 4 lines

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.

Quoted 9 lines

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.

Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

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

#4 lore raw
Quoted 3 lines · Patrick Mansfield

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.

Quoted 7 lines

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).

Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 2 lines

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.

Signature · 1 line
Luben
#5 lore raw
Quoted 4 lines · 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.

Quoted 2 lines

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.

Quoted 2 lines

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).

Quoted 2 lines

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).

Quoted 9 lines
Quoted 2 lines

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

#6 lore raw
Quoted 7 lines · Patrick Mansfield
Quoted 3 lines

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.

Quoted 4 lines

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...

Quoted 7 lines

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.

Quoted 2 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Signature · 1 line
Luben
#7 lore raw
In reply to Patrick Mansfield
Quoted 4 lines · Patrick Mansfield

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.

Quoted 7 lines
Quoted 2 lines

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).

#8 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Patrick Mansfield

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.

Quoted 10 lines
Quoted 7 lines
Quoted 2 lines

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

#9 lore raw
Quoted 6 lines · Patrick Mansfield

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.

Quoted 5 lines

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

Signature · 1 line
Luben
#10 lore raw
Quoted 5 lines · Luben Tuikov

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

#11 lore raw
Quoted 11 lines · Patrick Mansfield
Quoted 4 lines · Luben Tuikov

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...

Quoted 2 lines

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/.

Quoted 1 line

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.

Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 2 lines

(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.

Quoted 4 lines

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 :-) )

Quoted 3 lines

(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.

Quoted 7 lines

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.

Quoted 6 lines

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.

Signature · 1 line
Luben
#12 lore raw
In reply to Patrick Mansfield
Quoted 8 lines · Patrick Mansfield
Quoted 6 lines · 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

Quoted 7 lines

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.

Quoted 2 lines

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.

Signature · 4 lines
  Doug Ledford <[email protected]>     919-754-3700 x44233
         Red Hat, Inc. 
         1801 Varsity Dr.
         Raleigh, NC 27606
#13 lore raw
Quoted 10 lines · Doug Ledford
Quoted 7 lines · Patrick Mansfield

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).

Quoted 11 lines

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.

Quoted 11 lines
Quoted 2 lines

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

#14 lore raw
Quoted 6 lines · Patrick Mansfield

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.

Quoted 2 lines

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

Implementation issue.

Quoted 1 line

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

Implementation issue.

Quoted 5 lines

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.

Quoted 5 lines

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.

Quoted 2 lines

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.

Signature · 1 line
Luben
#15 lore raw
Quoted 8 lines · Luben Tuikov [[email protected]] wrote:
Quoted 5 lines

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.

Quoted 29 lines

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.

Quoted 9 lines
Quoted 5 lines

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.

Quoted 2 lines

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
[email protected]

#16 lore raw

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

Quoted 91 lines · Mike Anderson
Quoted 8 lines · Luben Tuikov [[email protected]] wrote:
Quoted 5 lines

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.

Quoted 29 lines

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.

Quoted 9 lines
Quoted 5 lines

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.

Quoted 2 lines

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
[email protected]

-
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

#17 lore raw
In reply to Mike Anderson

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

#18 lore raw
Quoted 5 lines · 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.

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().

Quoted 1 line

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.

Quoted 1 line

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.

Signature · 1 line
Luben
#19 lore raw
Quoted 4 lines · [email protected] 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().

Quoted 3 lines

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.

Quoted 7 lines

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