2002-11-19 06:13:32

by Andre Hedrick

[permalink] [raw]
Subject: linux-2.4.18-modified-scsi-h.patch


Greetings Doug et al.

Please consider the addition of this simple void ptr to the scsi_request
struct. The addition of this simple void pointer allows one to map any
and all request execution caller the facility to search for a specific
operation without having to run in circles. Hunting for these details
over the global device list of all HBA's is silly and one of the key
reasons why there error recovery path is so painful.


Scsi_Request *req = sc_cmd->sc_request;
blah_blah_t *trace = NULL;

trace = (blah_blah_t *)req->trace_ptr;


Therefore the specific transport invoking operations via the midlayer will
have the ablity to track and trace any operation.

It will save everyone headaches.

Cheers,


Andre Hedrick, CTO & Founder
iSCSI Software Solutions Provider
http://www.PyXTechnologies.com/


Attachments:
linux-2.4.18-modified-scsi-h.patch (599.00 B)

2002-11-19 06:22:31

by Jeff Garzik

[permalink] [raw]
Subject: Re: linux-2.4.18-modified-scsi-h.patch

Andre Hedrick wrote:

> Greetings Doug et al.
>
> Please consider the addition of this simple void ptr to the scsi_request
> struct. The addition of this simple void pointer allows one to map any
> and all request execution caller the facility to search for a specific
> operation without having to run in circles. Hunting for these details
> over the global device list of all HBA's is silly and one of the key
> reasons why there error recovery path is so painful.
>
>
> Scsi_Request *req = sc_cmd->sc_request;
> blah_blah_t *trace = NULL;
>
> trace = (blah_blah_t *)req->trace_ptr;
>
>
> Therefore the specific transport invoking operations via the midlayer will
> have the ablity to track and trace any operation.
>
> It will save everyone headaches.
>
> Cheers,
>
>
> Andre Hedrick, CTO & Founder
> iSCSI Software Solutions Provider
> http://www.PyXTechnologies.com/
>
>
> ------------------------------------------------------------------------
>
> --- linux/drivers/scsi/scsi.h.orig 2002-10-31 01:45:39.000000000 -0800
> +++ linux/drivers/scsi/scsi.h 2002-10-31 01:46:31.000000000 -0800
> @@ -667,8 +667,11 @@
> unsigned short sr_sglist_len; /* size of malloc'd scatter-gather list */
> unsigned sr_underflow; /* Return error if less than
> this amount is transferred */
> + void *trace_ptr; /* capable of cmd-cmnd-error tracing */


ok


> };
>
> +#define MODIFIED_SCSI_H


This falls into C style :) Instead of this I would do

#define HAVE_TRACE_PTR 1

just like we already do HAVE_xxx in include/linux/netdevice.h and other
places.

Jeff



2002-11-19 06:40:11

by Andre Hedrick

[permalink] [raw]
Subject: Re: linux-2.4.18-modified-scsi-h.patch

On Tue, 19 Nov 2002, Jeff Garzik wrote:

> Andre Hedrick wrote:
>
> > Greetings Doug et al.
> >
> > Please consider the addition of this simple void ptr to the scsi_request
> > struct. The addition of this simple void pointer allows one to map any
> > and all request execution caller the facility to search for a specific
> > operation without having to run in circles. Hunting for these details
> > over the global device list of all HBA's is silly and one of the key
> > reasons why there error recovery path is so painful.
> >
> >
> > Scsi_Request *req = sc_cmd->sc_request;
> > blah_blah_t *trace = NULL;
> >
> > trace = (blah_blah_t *)req->trace_ptr;
> >
> >
> > Therefore the specific transport invoking operations via the midlayer will
> > have the ablity to track and trace any operation.
> >
> > It will save everyone headaches.
> >
> > Cheers,
> >
> >
> > Andre Hedrick, CTO & Founder
> > iSCSI Software Solutions Provider
> > http://www.PyXTechnologies.com/
> >
> >
> > ------------------------------------------------------------------------
> >
> > --- linux/drivers/scsi/scsi.h.orig 2002-10-31 01:45:39.000000000 -0800
> > +++ linux/drivers/scsi/scsi.h 2002-10-31 01:46:31.000000000 -0800
> > @@ -667,8 +667,11 @@
> > unsigned short sr_sglist_len; /* size of malloc'd scatter-gather list */
> > unsigned sr_underflow; /* Return error if less than
> > this amount is transferred */
> > + void *trace_ptr; /* capable of cmd-cmnd-error tracing */
>
>
> ok
>
>
> > };
> >
> > +#define MODIFIED_SCSI_H
>
>
> This falls into C style :) Instead of this I would do
>
> #define HAVE_TRACE_PTR 1
>
> just like we already do HAVE_xxx in include/linux/netdevice.h and other
> places.
>
> Jeff

Works for me, just need to test the state of intelligence of the SCSI
stack and if it is possible to teach the old dog a new trick. The ablitly
to logically seek an operation without chasing, the link list of devices
and outstanding commands, it tail!

Cheers,

Andre Hedrick, CTO & Founder
iSCSI Software Solutions Provider
http://www.PyXTechnologies.com/

2002-11-19 10:06:38

by Douglas Gilbert

[permalink] [raw]
Subject: Re: linux-2.4.18-modified-scsi-h.patch

Andre Hedrick wrote:
> Greetings Doug et al.
>
> Please consider the addition of this simple void ptr to the scsi_request
> struct. The addition of this simple void pointer allows one to map any
> and all request execution caller the facility to search for a specific
> operation without having to run in circles. Hunting for these details
> over the global device list of all HBA's is silly and one of the key
> reasons why there error recovery path is so painful.
>
>
> Scsi_Request *req = sc_cmd->sc_request;
> blah_blah_t *trace = NULL;
>
> trace = (blah_blah_t *)req->trace_ptr;
>
>
> Therefore the specific transport invoking operations via the midlayer will
> have the ablity to track and trace any operation.

Andre,
No need to convince me: I have already put a similar pointer
in that structure in lk 2.5 (for either sd, st, sr or sg to use).
In sg case's it saved some ugly looping in (what was formerly
called) the bottom half handler. Sounds like your motivation is
similar.

Doug Gilbert


2002-11-19 12:09:45

by Andre Hedrick

[permalink] [raw]
Subject: Re: linux-2.4.18-modified-scsi-h.patch

On Tue, 19 Nov 2002, Douglas Gilbert wrote:

> Andre Hedrick wrote:
> > Greetings Doug et al.
> >
> > Please consider the addition of this simple void ptr to the scsi_request
> > struct. The addition of this simple void pointer allows one to map any
> > and all request execution caller the facility to search for a specific
> > operation without having to run in circles. Hunting for these details
> > over the global device list of all HBA's is silly and one of the key
> > reasons why there error recovery path is so painful.
> >
> >
> > Scsi_Request *req = sc_cmd->sc_request;
> > blah_blah_t *trace = NULL;
> >
> > trace = (blah_blah_t *)req->trace_ptr;
> >
> >
> > Therefore the specific transport invoking operations via the midlayer will
> > have the ablity to track and trace any operation.
>
> Andre,
> No need to convince me: I have already put a similar pointer
> in that structure in lk 2.5 (for either sd, st, sr or sg to use).
> In sg case's it saved some ugly looping in (what was formerly
> called) the bottom half handler. Sounds like your motivation is
> similar.
>
> Doug Gilbert

Hey there!

Well it needs to be in all kernels regardless, and if it is in the
scsi_request it is transparent to any given personality device and the
caller may reserve the option to include other key information. Simple
stats of what the queue depth of a given device is and a means to flush
out the commands to force a failure in the case of calling a device reset.

Without being to obvious or rude about the driver model being top down and
not the converse, hunting for given set of operation(s) will almost insure
a driver/device deadlock when racing against the done function.

I am pleased you and I are thinking in the same direction again!

Cheers,

Andre Hedrick, CTO & Founder
iSCSI Software Solutions Provider
http://www.PyXTechnologies.com/


2002-11-19 18:34:29

by Patrick Mansfield

[permalink] [raw]
Subject: Re: linux-2.4.18-modified-scsi-h.patch

On Tue, Nov 19, 2002 at 09:11:47PM +1100, Douglas Gilbert wrote:
> Andre Hedrick wrote:
> > Greetings Doug et al.
> >
> > Please consider the addition of this simple void ptr to the scsi_request
> > struct. The addition of this simple void pointer allows one to map any
> > and all request execution caller the facility to search for a specific
> > operation without having to run in circles. Hunting for these details
> > over the global device list of all HBA's is silly and one of the key
> > reasons why there error recovery path is so painful.
> >
> >
> > Scsi_Request *req = sc_cmd->sc_request;
> > blah_blah_t *trace = NULL;
> >
> > trace = (blah_blah_t *)req->trace_ptr;
> >
> >
> > Therefore the specific transport invoking operations via the midlayer will
> > have the ablity to track and trace any operation.
>
> Andre,
> No need to convince me: I have already put a similar pointer
> in that structure in lk 2.5 (for either sd, st, sr or sg to use).
> In sg case's it saved some ugly looping in (what was formerly
> called) the bottom half handler. Sounds like your motivation is
> similar.
>
> Doug Gilbert

So we should name it the same in 2.4 as in 2.5: upper_private_data, not
trace_ptr (thought it should really have been sr_upper_private_data,
like all the other fields in scsi_request).

I don't see why we need the #define, or is that another patch?

-- Patrick Mansfield

2002-11-19 18:42:13

by Jeff Garzik

[permalink] [raw]
Subject: Re: linux-2.4.18-modified-scsi-h.patch

Patrick Mansfield wrote:

> I don't see why we need the #define, or is that another patch?



The define exists for the same reason that HAVE_xxx exists in
include/linux/netdevice.h and other headers: a feature test macro, so
code using this pointer can detect its presence or absence. The world
of drivers is not all in the kernel tarball, ya know ;-)

But as I said, the macro is misnamed, it should be
HAVE_UPPER_PRIVATE_DATA or similar.

Jeff



2002-11-19 18:46:16

by Andre Hedrick

[permalink] [raw]
Subject: Re: linux-2.4.18-modified-scsi-h.patch

On Tue, 19 Nov 2002, Jeff Garzik wrote:

> Patrick Mansfield wrote:
>
> > I don't see why we need the #define, or is that another patch?
>
>
>
> The define exists for the same reason that HAVE_xxx exists in
> include/linux/netdevice.h and other headers: a feature test macro, so
> code using this pointer can detect its presence or absence. The world
> of drivers is not all in the kernel tarball, ya know ;-)
>
> But as I said, the macro is misnamed, it should be
> HAVE_UPPER_PRIVATE_DATA or similar.
>
> Jeff

Hey Jeff!

Works for me, has each HBA has the need to reserve local structs and not
pollute the basic kernel w/ bloat!

Cheers,

Andre Hedrick, CTO & Founder
iSCSI Software Solutions Provider
http://www.PyXTechnologies.com/

2002-11-19 18:55:09

by Andre Hedrick

[permalink] [raw]
Subject: Re: linux-2.4.18-modified-scsi-h.patch

On Tue, 19 Nov 2002, Patrick Mansfield wrote:

> On Tue, Nov 19, 2002 at 09:11:47PM +1100, Douglas Gilbert wrote:
> > Andre Hedrick wrote:
> > > Greetings Doug et al.
> > >
> > > Please consider the addition of this simple void ptr to the scsi_request
> > > struct. The addition of this simple void pointer allows one to map any
> > > and all request execution caller the facility to search for a specific
> > > operation without having to run in circles. Hunting for these details
> > > over the global device list of all HBA's is silly and one of the key
> > > reasons why there error recovery path is so painful.
> > >
> > >
> > > Scsi_Request *req = sc_cmd->sc_request;
> > > blah_blah_t *trace = NULL;
> > >
> > > trace = (blah_blah_t *)req->trace_ptr;
> > >
> > >
> > > Therefore the specific transport invoking operations via the midlayer will
> > > have the ablity to track and trace any operation.
> >
> > Andre,
> > No need to convince me: I have already put a similar pointer
> > in that structure in lk 2.5 (for either sd, st, sr or sg to use).
> > In sg case's it saved some ugly looping in (what was formerly
> > called) the bottom half handler. Sounds like your motivation is
> > similar.
> >
> > Doug Gilbert
>
> So we should name it the same in 2.4 as in 2.5: upper_private_data, not
> trace_ptr (thought it should really have been sr_upper_private_data,
> like all the other fields in scsi_request).
>
> I don't see why we need the #define, or is that another patch?

Hi Patrick,

The #define was as to determine or notify the driver at build time if it
had an enhanced capablity or legacy restrictions. Obviously once it is
adopted regardless of form then a simple version check will work.

It was for testing purposes.


Cheers,

Andre Hedrick, CTO & Founder
iSCSI Software Solutions Provider
http://www.PyXTechnologies.com/