Re: [PATCH] consolidating data direction tables
Jeremy Higdon wrote:
>
> [snip]
>
> I was reading Documentation/scsi-generic.txt in 2.4.1. I didn't see
> anything about direction in the "new sg_header". Is there something
> newer?
>
> | The new sg_header offered in this driver is:
> | #define SG_MAX_SENSE 16
> | struct sg_header
> | {
Jeremy,
The Documentation/scsi-generic.txt file in lk 2.4.1 is a
carry over from the lk 2.2 series and documents what I
have called the "version 2" sg interface. This is essentially
stretching the "struct sg_header" based interface that has
been around since 1992.
There is a document called scsi-generic_v3.txt at the
sg website. It contains extra information about the
"struct sg_io_hdr" based interface that has been added
in the lk 2.4 series. The "v3" document assumes knowledge
of the scsi-generic.txt document. So I need to consolidate
them into one document for lk 2.4 . There is also a
page about the "v3" interface at the sg website
(www.torque.net/sg).
There is also information to be found in lk 2.4's
/usr/src/linux/include/scsi/sg.h header file (arguably
too much information). Interestingly, in the glibc 2.2
release I have, the /usr/include/scsi/sg.h file, while
appropriate for sg in lk 2.4, has most of this
information removed.
Doug Gilbert
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
On Wed, Feb 14, 2001 at 08:16:30AM +0100, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > > However, in light of other email in this thread, it looks like maybe the > > table can go altogether, so hopefully this question is now moot. > > Oh yes, I saw the mail you are refering to. > Let me put it this way: If we get rid of 'direction unknown' > then I'd be comfortable with killing it. > I'll put a printk into the drivers I use the test this. I don't know if you looked at my patch, but all the drivers in question assume that if they don't know a command is a write, then it's a read command. There are only eight drivers that do things this way, and they don't seem to be particularly new ones, in general, though I might be wrong about that with a few of them. So I think that trusting what the kernel thinks is the direction might be as good as trusting what the driver thinks is the direction currently. On the other hand, given that MESH at least locks up hard when the driver guesses wrong, it's important to verify that the kernel is right at least as often as the drivers currently are about the direction. -Daniel -- Daniel E. Eisenbud [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
> However, in light of other email in this thread, it looks like maybe the > table can go altogether, so hopefully this question is now moot. Oh yes, I saw the mail you are refering to. Let me put it this way: If we get rid of 'direction unknown' then I'd be comfortable with killing it. I'll put a printk into the drivers I use the test this. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
> > This means that it would be compiled into the kernel several times. > > You should add it to the core scsi code and _not_ declare it static. > > I considered doing it this way. As I saw it, it's a tradeoff between > including the code even if none of the drivers in question are > configured in, or compiling it multiple times if several of them are. I > guess there's also the possibility of an ifdef so that it will be > compiled if at least one of drivers is configured in. > > However, in light of other email in this thread, it looks like maybe the > table can go altogether, so hopefully this question is now moot. I think this will not be the case. Alan is right. Compatibilty should not be lightly broken. Given that most new drivers will probably need a direction I still think that you should add it to the scsi core. You don't increase the core code by much, but you make some loadable modules consume a page less. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
On Feb 13, 9:40am, Douglas Gilbert wrote:
>
> Daniel,
> Does that driver take any account of Scsi_Cmnd::sc_data_direction_flag?
> All upper level drivers (including sg) in the lk 2.4 series set
> the corresponding Scsi_Request::sr_data_direction_flag .
>
> In the case of sg in lk 2.4, it supports 2 interfaces:
> - In the newer one (not yet used by cdrecord) the data
>direction flag is given explicitly. [There could be
>PCI bus problems if the app gets the direction wrong].
> - In the older interface the internal data direction flag
>is deduced from the sizes of the data buffers given.
>There is a slight ambiguity when the write (to device)
>buffer and the read (from device) buffer are both
>given; a read from device data direction flag is assumed.
>[BTW The SCSI_IOCTL_SEND_COMMAND ioctl() makes a similar
> deduction.]
>
> If cdrecord is sending a vendor specific command that
> writes data to the device, but sets the read buffer
> up as well and thus tricks the above logic, then we get
> cdrecord changed. Cdrecord has a library called libscg
> that interfaces to many OSes and the generic structures
> within libscg have a data direction flag.
>
> > The other drivers in question may or may not be able
> > to recover from the missing item in their tables, but the fact that
> > these tables are in the drivers at all implies that they work best if
> > the tables are correct.
>
> Data direction tables shouldn't be needed in lk 2.4 .
>
> Doug Gilbert
I was reading Documentation/scsi-generic.txt in 2.4.1. I didn't see
anything about direction in the "new sg_header". Is there something
newer?
| The new sg_header offered in this driver is:
| #define SG_MAX_SENSE 16
| struct sg_header
| {
| int pack_len;/* [o] reply_len (ie useless) ignored as input */
| int reply_len; /* [i] max length of expected reply (inc. sg_header) */
| int pack_id; /* [io] id number of packet (use ints >= 0) */
| int result; /* [o] 0==ok, else (+ve) Unix errno (best ignored) */
| unsigned int twelve_byte:1;
| /* [i] Force 12 byte command length for group 6 & 7 commands */
| unsigned int target_status:5; /* [o] scsi status from target */
| unsigned int host_status:8; /* [o] host status (see "DID" codes) */
| unsigned int driver_status:8; /* [o] driver status+suggestion */
|
| unsigned int other_flags:10;/* unused */
| unsigned char sense_buffer[SG_MAX_SENSE]; /* [o] Output in 3 cases:
|when target_status is CHECK_CONDITION or
|when target_status is COMMAND_TERMINATED or
|when (driver_status & DRIVER_SENSE) is true. */
| }; /* This structure is 36 bytes long on i386 */
thanks
jeremy
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
On Tue, Feb 13, 2001 at 10:24:14AM +0100, Oliver Neukum <[EMAIL PROTECTED]> wrote: > > > The attached patch does two things: it creates a new header file called > > > scsi_dataout.h, which has a single copy of the switch statement (as a > > > static function -- is that all right?) and is included by the relevant > > > drivers. I checked that all the drivers had exactly equivalent switch > > This means that it would be compiled into the kernel several times. > You should add it to the core scsi code and _not_ declare it static. I considered doing it this way. As I saw it, it's a tradeoff between including the code even if none of the drivers in question are configured in, or compiling it multiple times if several of them are. I guess there's also the possibility of an ifdef so that it will be compiled if at least one of drivers is configured in. However, in light of other email in this thread, it looks like maybe the table can go altogether, so hopefully this question is now moot. -Daniel -- Daniel E. Eisenbud [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
On Tue, Feb 13, 2001 at 09:40:36AM -0500, Douglas Gilbert <[EMAIL PROTECTED]> wrote: > Daniel Eisenbud wrote: > > > > Eight different SCSI drivers have large switch statements to determine > > the direction in which data will be transferred for a given SCSI > > command. I discovered this when trying to figure out why the MESH > > (powermac SCSI) driver locked up when (and only when) trying to burn a > > CD-R in disk-at-once mode. It turns out that there's a command that's > > not in any of these tables (cdrecord refers to it as send_cue_sheet) and > > it's absence is enough to kill the MESH driver (and apparently also the > > mac53c94 driver.) > > Daniel, > Does that driver take any account of Scsi_Cmnd::sc_data_direction_flag? > All upper level drivers (including sg) in the lk 2.4 series set > the corresponding Scsi_Request::sr_data_direction_flag . [...] The driver appears not to use sc_data_directon_flag yet. I will look into making it do so (and the other drivers in question too, possibly, though I have no way of testing any but mesh.c and mac53c94.c). > Data direction tables shouldn't be needed in lk 2.4 . Great! As for 2.2, would the equivalent of the patch that I sent be acceptable for inclusion? If that's too big a change, what about a patch that just adds SEND_CUE_SHEET to include/scsi/scsi.h and each of the case statements in the drivers in question? -Daniel -- Daniel E. Eisenbud [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
Daniel Eisenbud wrote: > > Eight different SCSI drivers have large switch statements to determine > the direction in which data will be transferred for a given SCSI > command. I discovered this when trying to figure out why the MESH > (powermac SCSI) driver locked up when (and only when) trying to burn a > CD-R in disk-at-once mode. It turns out that there's a command that's > not in any of these tables (cdrecord refers to it as send_cue_sheet) and > it's absence is enough to kill the MESH driver (and apparently also the > mac53c94 driver.) Daniel, Does that driver take any account of Scsi_Cmnd::sc_data_direction_flag? All upper level drivers (including sg) in the lk 2.4 series set the corresponding Scsi_Request::sr_data_direction_flag . In the case of sg in lk 2.4, it supports 2 interfaces: - In the newer one (not yet used by cdrecord) the data direction flag is given explicitly. [There could be PCI bus problems if the app gets the direction wrong]. - In the older interface the internal data direction flag is deduced from the sizes of the data buffers given. There is a slight ambiguity when the write (to device) buffer and the read (from device) buffer are both given; a read from device data direction flag is assumed. [BTW The SCSI_IOCTL_SEND_COMMAND ioctl() makes a similar deduction.] If cdrecord is sending a vendor specific command that writes data to the device, but sets the read buffer up as well and thus tricks the above logic, then we get cdrecord changed. Cdrecord has a library called libscg that interfaces to many OSes and the generic structures within libscg have a data direction flag. > The other drivers in question may or may not be able > to recover from the missing item in their tables, but the fact that > these tables are in the drivers at all implies that they work best if > the tables are correct. Data direction tables shouldn't be needed in lk 2.4 . Doug Gilbert - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
> That could be done, but you'd never be able to drop these tables. And ? > I'd prefer a clean break to the new interface. It's better. > Doug Gilbert is working on it. You drop them after 2.6, much more polite - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
> > Has a direction bit been considered for Linux SCSI infrastructure? > > It would be useful to be able to specify this starting at the sg > > level on through to the hba layer. > > It's there but not visible through the sg driver. That would make sense for 2.5 > > We currently support a RAID which uses vendor unique commands to do > > device management. We need special versions of drivers that understand > > the implied direction of the vendor unique commands. Under Irix, the > > ds interface explicitly specifies a direction, so no driver code need > > be changed to support this device. > > That's the problem. You'd break the interface. > Nevertheless, it should be done in 2.5. Breaking the old API isnt required. There is what seems to be a full direction table in the IBM serveraid driver. If you hit an unknown on the old ABI just error it. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
On Dienstag, 13. Februar 2001 10:42, Jeremy Higdon wrote: > On Feb 13, 10:24am, Oliver Neukum wrote: > > > We currently support a RAID which uses vendor unique commands to do > > > device management. We need special versions of drivers that understand > > > the implied direction of the vendor unique commands. Under Irix, the > > > ds interface explicitly specifies a direction, so no driver code need > > > be changed to support this device. > > > > That's the problem. You'd break the interface. > > Nevertheless, it should be done in 2.5. > > What if two bits were used: > > 0 = direction implied > 1 = read from device > 2 = write to device > 3 = reserved/bidirectional > > Then you could maintain compatibility as long as older programs zero > out the unused flags bits. That could be done, but you'd never be able to drop these tables. I'd prefer a clean break to the new interface. It's better. Doug Gilbert is working on it. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
RE: [PATCH] consolidating data direction tables
I support the idea of the 2 bit. The EATA protocol requires to specify either DATA_IN or DATA_OUT or DATA_NONE. The data_out table by itself is not enough. -db -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]] Sent: Tuesday, February 13, 2001 10:42 AM To: Oliver Neukum; Daniel Eisenbud; [EMAIL PROTECTED] Subject: Re: [PATCH] consolidating data direction tables On Feb 13, 10:24am, Oliver Neukum wrote: > > > We currently support a RAID which uses vendor unique commands to do > > device management. We need special versions of drivers that understand > > the implied direction of the vendor unique commands. Under Irix, the > > ds interface explicitly specifies a direction, so no driver code need > > be changed to support this device. > > That's the problem. You'd break the interface. > Nevertheless, it should be done in 2.5. What if two bits were used: 0 = direction implied 1 = read from device 2 = write to device 3 = reserved/bidirectional Then you could maintain compatibility as long as older programs zero out the unused flags bits. jeremy - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
On Feb 13, 10:24am, Oliver Neukum wrote: > > > We currently support a RAID which uses vendor unique commands to do > > device management. We need special versions of drivers that understand > > the implied direction of the vendor unique commands. Under Irix, the > > ds interface explicitly specifies a direction, so no driver code need > > be changed to support this device. > > That's the problem. You'd break the interface. > Nevertheless, it should be done in 2.5. What if two bits were used: 0 = direction implied 1 = read from device 2 = write to device 3 = reserved/bidirectional Then you could maintain compatibility as long as older programs zero out the unused flags bits. jeremy - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
> > mac53c94 driver.) The other drivers in question may or may not be able > > to recover from the missing item in their tables, but the fact that > > these tables are in the drivers at all implies that they work best if > > the tables are correct. Some drivers need a direction. If you don't know the command you have a 50/50 chance. The current system is a mess. > > The attached patch does two things: it creates a new header file called > > scsi_dataout.h, which has a single copy of the switch statement (as a > > static function -- is that all right?) and is included by the relevant > > drivers. I checked that all the drivers had exactly equivalent switch This means that it would be compiled into the kernel several times. You should add it to the core scsi code and _not_ declare it static. > Has a direction bit been considered for Linux SCSI infrastructure? > It would be useful to be able to specify this starting at the sg > level on through to the hba layer. It's there but not visible through the sg driver. > We currently support a RAID which uses vendor unique commands to do > device management. We need special versions of drivers that understand > the implied direction of the vendor unique commands. Under Irix, the > ds interface explicitly specifies a direction, so no driver code need > be changed to support this device. That's the problem. You'd break the interface. Nevertheless, it should be done in 2.5. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
Re: [PATCH] consolidating data direction tables
On Feb 12, 8:49pm, Daniel Eisenbud wrote: > > Eight different SCSI drivers have large switch statements to determine > the direction in which data will be transferred for a given SCSI > command. I discovered this when trying to figure out why the MESH > (powermac SCSI) driver locked up when (and only when) trying to burn a > CD-R in disk-at-once mode. It turns out that there's a command that's > not in any of these tables (cdrecord refers to it as send_cue_sheet) and > it's absence is enough to kill the MESH driver (and apparently also the > mac53c94 driver.) The other drivers in question may or may not be able > to recover from the missing item in their tables, but the fact that > these tables are in the drivers at all implies that they work best if > the tables are correct. > > The attached patch does two things: it creates a new header file called > scsi_dataout.h, which has a single copy of the switch statement (as a > static function -- is that all right?) and is included by the relevant > drivers. I checked that all the drivers had exactly equivalent switch > statements. The new switch statement is that switch statement with > SEND_CUE_SHEET (which I've added to include/scsi/scsi.h) added. This > way if other commands need to be added to the switch statements in the > future, it will be easy to do it in one central place. > > Clearly it is a problem that the mesh and mac53c94 drivers actually lock > up hard when they get a command that they haven't seen before. When I > have some time, I will learn more about the SCSI layer and see if I can > fix this. But in the meantime, this patch prevents the known case that > locks up, and is a cleanup to boot. Please let me know if there's > anything I need to do differently to have a chance of this getting added > to the kernel: I'm rather new to kernel hacking altogether. Has a direction bit been considered for Linux SCSI infrastructure? It would be useful to be able to specify this starting at the sg level on through to the hba layer. Since the only person who knows for sure the direction of data transfer is the person writing a user app (in the case of sg), it seems that it would make sense for the user app to be able to specify the direction. We currently support a RAID which uses vendor unique commands to do device management. We need special versions of drivers that understand the implied direction of the vendor unique commands. Under Irix, the ds interface explicitly specifies a direction, so no driver code need be changed to support this device. jeremy - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED]
[PATCH] consolidating data direction tables
Eight different SCSI drivers have large switch statements to determine
the direction in which data will be transferred for a given SCSI
command. I discovered this when trying to figure out why the MESH
(powermac SCSI) driver locked up when (and only when) trying to burn a
CD-R in disk-at-once mode. It turns out that there's a command that's
not in any of these tables (cdrecord refers to it as send_cue_sheet) and
it's absence is enough to kill the MESH driver (and apparently also the
mac53c94 driver.) The other drivers in question may or may not be able
to recover from the missing item in their tables, but the fact that
these tables are in the drivers at all implies that they work best if
the tables are correct.
The attached patch does two things: it creates a new header file called
scsi_dataout.h, which has a single copy of the switch statement (as a
static function -- is that all right?) and is included by the relevant
drivers. I checked that all the drivers had exactly equivalent switch
statements. The new switch statement is that switch statement with
SEND_CUE_SHEET (which I've added to include/scsi/scsi.h) added. This
way if other commands need to be added to the switch statements in the
future, it will be easy to do it in one central place.
Clearly it is a problem that the mesh and mac53c94 drivers actually lock
up hard when they get a command that they haven't seen before. When I
have some time, I will learn more about the SCSI layer and see if I can
fix this. But in the meantime, this patch prevents the known case that
locks up, and is a cleanup to boot. Please let me know if there's
anything I need to do differently to have a chance of this getting added
to the kernel: I'm rather new to kernel hacking altogether.
Thanks,
Daniel Eisenbud
--
Daniel E. Eisenbud
[EMAIL PROTECTED]
diff -durpN linuxppc_2_4/drivers/acorn/scsi/acornscsi.c
linuxppc_2_4-scsifix/drivers/acorn/scsi/acornscsi.c
--- linuxppc_2_4/drivers/acorn/scsi/acornscsi.c Fri Jan 12 16:57:57 2001
+++ linuxppc_2_4-scsifix/drivers/acorn/scsi/acornscsi.c Sat Feb 3 12:02:04 2001
@@ -152,6 +152,7 @@
#include
#include "../../scsi/scsi.h"
+#include "../../scsi/scsi_dataout.h"
#include "../../scsi/hosts.h"
#include "../../scsi/constants.h"
#include "acornscsi.h"
@@ -600,34 +601,6 @@ cmdtype_t acornscsi_cmdtype(int command)
}
/*
- * Prototype: int acornscsi_datadirection(int command)
- * Purpose : differentiate between commands that have a DATA IN phase
- * and a DATA OUT phase
- * Params : command - command to interpret
- * Returns : DATADIR_OUT - data out phase expected
- * DATADIR_IN - data in phase expected
- */
-static
-datadir_t acornscsi_datadirection(int command)
-{
-switch (command) {
-case CHANGE_DEFINITION:case COMPARE: case COPY:
-case COPY_VERIFY: case LOG_SELECT:case MODE_SELECT:
-case MODE_SELECT_10: case SEND_DIAGNOSTIC: case WRITE_BUFFER:
-case FORMAT_UNIT: case REASSIGN_BLOCKS: case RESERVE:
-case SEARCH_EQUAL: case SEARCH_HIGH: case SEARCH_LOW:
-case WRITE_6: case WRITE_10: case WRITE_VERIFY:
-case UPDATE_BLOCK: case WRITE_LONG:case WRITE_SAME:
-case SEARCH_HIGH_12: case SEARCH_EQUAL_12: case SEARCH_LOW_12:
-case WRITE_12: case WRITE_VERIFY_12: case SET_WINDOW:
-case MEDIUM_SCAN: case SEND_VOLUME_TAG: case 0xea:
- return DATADIR_OUT;
-default:
- return DATADIR_IN;
-}
-}
-
-/*
* Purpose : provide values for synchronous transfers with 33C93.
* Copyright: Copyright (c) 1996 John Shifflett, GeoLog Consulting
* Modified by Russell King for 8MHz WD33C93A
@@ -2552,7 +2525,7 @@ int acornscsi_queuecmd(Scsi_Cmnd *SCpnt,
SCpnt->host_scribble = NULL;
SCpnt->result = 0;
SCpnt->tag = 0;
-SCpnt->SCp.phase = (int)acornscsi_datadirection(SCpnt->cmnd[0]);
+SCpnt->SCp.phase = Scsi_data_goes_out(SCpnt);
SCpnt->SCp.sent_command = 0;
SCpnt->SCp.scsi_xferred = 0;
SCpnt->SCp.Status = 0;
diff -durpN linuxppc_2_4/drivers/acorn/scsi/acornscsi.h
linuxppc_2_4-scsifix/drivers/acorn/scsi/acornscsi.h
--- linuxppc_2_4/drivers/acorn/scsi/acornscsi.h Fri Jan 12 16:57:57 2001
+++ linuxppc_2_4-scsifix/drivers/acorn/scsi/acornscsi.h Fri Feb 2 18:36:48 2001
@@ -288,14 +288,6 @@ typedef enum { /* command
type */
CMD_MISC, /* Others
*/
} cmdtype_t;
-/*
- * Data phase direction
- */
-typedef enum { /* Data direction
*/
-DATADIR_IN,/* Data in phase expected
*/
-DATADIR_OUT/* Data out phase expected
*/
-} datadir_t;
-
#include "queue.h"
#include "msgqueue.h"
diff -durpN linux
