One more change pushed to bk tree

One more change pushed to bk tree

[Doug Ledford]

Like last time, this has been through a full compile test, but not a boot 
test.  I'm heading up to the office next so the boot test will come 
shortly.  But, because it touches lots of files, again, I thought I would 
go ahead and push it up for people to see.  If there are no objections to 
this or my previous patches, then once the boot test is complete and I'm 
satisfied things are working properly, I'll push the whole lot to Linus 
(which will necessarily include the scsi-misc-2.5 changes because they are 
in my tree as well).

bk pull bk://linux-scsi.bkbits.net/scsi-dledford

New Changeset added since last night:
ChangeSet@1.852, 2002-11-22 12:59:10-05:00, dledford@dledford.theledfords.org
  Convert from host->host_queue to host->my_devices list usage
  Add in usage of new same_target_siblings list
  Add scsi_release_commandblocks() call to scsi_free_sdev()
  Make all scsi device freeing use scsi_free_sdev()


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

Re: One more change pushed to bk tree

[Christoph Hellwig]

On Fri, Nov 22, 2002 at 01:36:03PM -0500, Doug Ledford wrote:
> Like last time, this has been through a full compile test, but not a boot 
> test.  I'm heading up to the office next so the boot test will come 
> shortly.  But, because it touches lots of files, again, I thought I would 
> go ahead and push it up for people to see.  If there are no objections to 
> this or my previous patches, then once the boot test is complete and I'm 
> satisfied things are working properly, I'll push the whole lot to Linus 
> (which will necessarily include the scsi-misc-2.5 changes because they are 
> in my tree as well).

Heh, now you managed to get stuff commited before me and I need to merge
up :)  But your changes help me a lot in fact so I surely won't complain.

Two comments: 

 o as you already had to change all callers of host->host_queue/
   host->my_devices could you add a lock to properly protect it?  That's
   pretty important for hotplugging/etc.. (like the fc hotplug patch
   from montavista)
 o what's the reason in continuing after an error from scsi_add_host
   in scsi_add_host?  IMHO it doesn't make much sense to continue the
   bus scan in that case.

Re: One more change pushed to bk tree

[Doug Ledford]

On Fri, Nov 22, 2002 at 08:45:38PM +0000, Christoph Hellwig wrote:
> On Fri, Nov 22, 2002 at 01:36:03PM -0500, Doug Ledford wrote:
> > Like last time, this has been through a full compile test, but not a boot 
> > test.  I'm heading up to the office next so the boot test will come 
> > shortly.  But, because it touches lots of files, again, I thought I would 
> > go ahead and push it up for people to see.  If there are no objections to 
> > this or my previous patches, then once the boot test is complete and I'm 
> > satisfied things are working properly, I'll push the whole lot to Linus 
> > (which will necessarily include the scsi-misc-2.5 changes because they are 
> > in my tree as well).
> 
> Heh, now you managed to get stuff commited before me and I need to merge
> up :)  But your changes help me a lot in fact so I surely won't complain.

Quoting from my email yesterday:

It's a race Charlie Brown!

;-)

> Two comments: 
> 
>  o as you already had to change all callers of host->host_queue/
>    host->my_devices could you add a lock to properly protect it?  That's
>    pretty important for hotplugging/etc.. (like the fc hotplug patch
>    from montavista)

I'm actually thinking that the locking of the devices is not needed as 
long as the next step is to A) make sure that all people walking the 
my_devices list already are holding a reference to the parent host vi 
host_get_*() and B) that proper locking for hosts is actually implemented 
in the host_get*() routines instead of the stub comments that are there 
now.

>  o what's the reason in continuing after an error from scsi_add_host
>    in scsi_add_host?  IMHO it doesn't make much sense to continue the
>    bus scan in that case.

There's no guarantee that devices later on in the chain will fail to 
attach, so I saw little reason to leave things half done.  If they all 
fail after the first failure, then we are no worse off then by bailing 
early, but if some actually succeed then we are better off.

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

Re: One more change pushed to bk tree

[Doug Ledford]

On Fri, Nov 22, 2002 at 01:36:03PM -0500, Doug Ledford wrote:
> Like last time, this has been through a full compile test, but not a boot 
> test.  I'm heading up to the office next so the boot test will come 
> shortly.

BTW, boot and run tests were successful.  No I'm trying to isolate why 
certain modules blow up when you load them, unload them, then reload them.  
Right now, the aic7xxx_old, sd_mod, and scsi_mod modules are working.  The 
st module is blowing chunks.  I haven't tried sr_mod yet.  I also haven't 
tried any other lldd modules yet.

Anyway, back to hacking st.c

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

Re: One more change pushed to bk tree

[Christoph Hellwig]

On Fri, Nov 22, 2002 at 05:22:47PM -0500, Doug Ledford wrote:
> > Two comments: 
> > 
> >  o as you already had to change all callers of host->host_queue/
> >    host->my_devices could you add a lock to properly protect it?  That's
> >    pretty important for hotplugging/etc.. (like the fc hotplug patch
> >    from montavista)
> 
> I'm actually thinking that the locking of the devices is not needed as 
> long as the next step is to A) make sure that all people walking the 
> my_devices list already are holding a reference to the parent host vi 
> host_get_*() and B) that proper locking for hosts is actually implemented 
> in the host_get*() routines instead of the stub comments that are there 
> now.

Yupp.

Re: One more change pushed to bk tree

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]

On Fri, Nov 22, 2002 at 05:24:59PM -0500, Doug Ledford wrote:
> On Fri, Nov 22, 2002 at 01:36:03PM -0500, Doug Ledford wrote:
> > Like last time, this has been through a full compile test, but not a boot 
> > test.  I'm heading up to the office next so the boot test will come 
> > shortly.
> 
> BTW, boot and run tests were successful.  No I'm trying to isolate why 
> certain modules blow up when you load them, unload them, then reload them.  
> Right now, the aic7xxx_old, sd_mod, and scsi_mod modules are working.  The 
> st module is blowing chunks.  I haven't tried sr_mod yet.  I also haven't 
> tried any other lldd modules yet.
> 
> Anyway, back to hacking st.c

st.c problem solved.  At this point, these changes are ready for sucking 
up by Linus.

bk pull bk://linux-scsi.bkbits.net/scsi-dledford

when you have network connectivity again Linus ;-)

There are two more new Changesets at the top of the changeset list 
relative to my last email, plus the tree has been merged with 2.4.49 
since my last email.  The total local changeset list is attached.


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

[-- Attachment #2: changes.list --]
[-- Type: text/plain, Size: 5782 bytes --]

ChangeSet@1.855, 2002-11-22 18:26:02-05:00, dledford@aladin.rdu.redhat.com
  fs/proc/inode.c: Make proc use new module loader semantics so that
  	touching a /proc/* file doesn't pin a module in memory

ChangeSet@1.854, 2002-11-22 18:25:05-05:00, dledford@aladin.rdu.redhat.com
  st.c:   Clean up init failure paths
  	Fix the detach code so it doesn't call sysfs unregister with
  		a lock held

ChangeSet@1.853, 2002-11-22 17:53:11-05:00, dledford@flossy.devel.redhat.com
  Merge bk://linux.bkbits.net/linux-2.5
  into flossy.devel.redhat.com:/usr/local/home/dledford/bk/linus-2.5

ChangeSet@1.852, 2002-11-22 12:59:10-05:00, dledford@dledford.theledfords.org
  Convert from host->host_queue to host->my_devices list usage
  Add in usage of new same_target_siblings list
  Add scsi_release_commandblocks() call to scsi_free_sdev()
  Make all scsi device freeing use scsi_free_sdev()

ChangeSet@1.851, 2002-11-22 00:36:31-05:00, dledford@dledford.theledfords.org
  scsi: Update lldd API to slave_alloc/slave_configure/slave_destroy interface
  	instead of slave_attach/slave_detach interface.  Change all users
  	of existing interface to new interface, update scan code for new
  	device attachement semantics

ChangeSet@1.842.29.10, 2002-11-21 11:59:45-06:00, patmans@us.ibm.com
  [PATCH] cleanup code for /proc add/remove single device
  
  On Thu, Nov 21, 2002 at 02:23:14AM +0100, Christoph Hellwig wrote:
  > two new helpers in scsi_scan.c: scsi_add_single_device and
  > scsi_remove_single_device that do all hard work.  Thanks to
  > beeing in scsi_scan.c and using proper helpers they're a lot
  > smaller and cleaner.
  >
  >
  > --- 1.44/drivers/scsi/scsi.h	Sun Nov 17 16:44:35 2002
  > +++ edited/drivers/scsi/scsi.h	Thu Nov 21 01:05:39 2002
  > -
  > -		err = -ENOSYS;
  > -		if (sdev)
  > -			goto out;	/* We do not yet support unplugging */
  > -
  > -		scan_scsis(shost, 1, channel, id, lun);
  > -		err = length;
  > -		goto out;
  > -	}
  > +		err = scsi_add_single_device(host, channel, id, lun);
  > +		if (err >= 0)
  > +			err = length;
  
  > ===== drivers/scsi/scsi_scan.c 1.38 vs edited =====
  > --- 1.38/drivers/scsi/scsi_scan.c	Sun Nov 17 16:47:20 2002
  > +++ edited/drivers/scsi/scsi_scan.c	Thu Nov 21 01:16:03 2002
  > @@ -1862,6 +1862,69 @@
  >
  >  }
  >
  > +int scsi_add_single_device(uint host, uint channel, uint id, uint lun)
  > +{
  > +	struct scsi_device *sdevscan, *sdev;
  > +	struct Scsi_Host *shost;
  > +	int error = -ENODEV;
  > +
  > +	shost = scsi_host_hn_get(host);
  > +	if (!shost)
  > +		return -ENODEV;
  > +	sdev = scsi_find_device(shost, channel, id, lun);
  > +	if (!sdev)
  > +		goto out;
  > +
  
  James/Christoph -
  
  I had to change the above to a "if (sdev)" per the following patch (against
  current scsi-misc-2.5) to get the add to work correctly. Otherwise, this
  worked fine.

ChangeSet@1.842.29.9, 2002-11-21 11:59:33-06:00, patmans@us.ibm.com
  [PATCH] current scsi-misc-2.5 include files
  
  On Thu, Nov 21, 2002 at 10:12:23AM -0600, J.E.J. Bottomley wrote:
  > The scsi-misc-2.5 resync should be done now.
  >
  > James
  
  Hi -
  
  hardirq.h and init.h were removed from scsi.h. So I had to make the
  following changes to compile. I also removed two extraneous includes.

ChangeSet@1.849, 2002-11-21 12:55:58-05:00, dledford@aladin.rdu.redhat.com
  Merge aladin.rdu.redhat.com:/usr/local/home/dledford/bk/linus-2.5
  into aladin.rdu.redhat.com:/usr/src/2.5

ChangeSet@1.842.29.8, 2002-11-21 10:06:03-06:00, hch@lst.de
  [PATCH] turn scsi_allocate_device into readable code

ChangeSet@1.842.29.7, 2002-11-21 10:05:24-06:00, hch@lst.de
  [PATCH] some HBA compile warning fixes

ChangeSet@1.842.29.6, 2002-11-21 10:04:47-06:00, hch@lst.de
  [PATCH] remove useless includes from scsi.h

ChangeSet@1.842.29.5, 2002-11-21 10:04:16-06:00, hch@lst.de
  [PATCH] get rid of scan_scsis
  
  this function was a multiplexer for two very different things, but
  with my last patch one of those faded away and only one callers
  is left.  Simplify it to what's still needed and rename to
  scsi_scan_host.

ChangeSet@1.842.29.4, 2002-11-21 10:03:50-06:00, hch@lst.de
  [PATCH] cleanup code for /proc add/remove single device
  
  two new helpers in scsi_scan.c: scsi_add_single_device and
  scsi_remove_single_device that do all hard work.  Thanks to
  beeing in scsi_scan.c and using proper helpers they're a lot
  smaller and cleaner.

ChangeSet@1.842.29.3, 2002-11-21 10:03:28-06:00, hch@lst.de
  [PATCH] make a few more routines private to scsi_lib.c

ChangeSet@1.842.29.2, 2002-11-21 10:03:10-06:00, hch@lst.de
  [PATCH] split busy check out of scsi_remove_host
  
  just a simple code cleanup

ChangeSet@1.842.29.1, 2002-11-21 10:02:57-06:00, hch@lst.de
  [PATCH] remove unused include in scsi_ioctl.c
  
  This time I explicitly checked for the in_atomic mess..

ChangeSet@1.848, 2002-11-20 19:05:37-05:00, dledford@aladin.rdu.redhat.com
  Merge aladin.rdu.redhat.com:/usr/local/home/dledford/bk/linus-2.5
  into aladin.rdu.redhat.com:/usr/src/2.5

ChangeSet@1.846.1.1, 2002-11-20 19:05:09-05:00, dledford@aladin.rdu.redhat.com
  scsi.c, scsi.h, scsi_syms.c, aic7xxx_old: add new function to track queue
  	full events at the mid layer instead of at the low level device
  	driver

ChangeSet@1.845, 2002-11-19 17:07:50-05:00, dledford@flossy.devel.redhat.com
  Merge bk://linux.bkbits.net/linux-2.5
  into flossy.devel.redhat.com:/usr/local/home/dledford/bk/linus-2.5

ChangeSet@1.825.7.2, 2002-11-17 22:09:07-05:00, dledford@aladin.rdu.redhat.com
  aic7xxx_old: update biosparam function with the (ugly) detail that
  	cylinder values > 65535 get truncated

ChangeSet@1.825.7.1, 2002-11-17 18:57:50-05:00, dledford@aladin.rdu.redhat.com
  aic7xxx_old: fix up the biosparam function to do 64bit math safely

Re: One more change pushed to bk tree

[Patrick Mansfield]

On Fri, Nov 22, 2002 at 01:36:03PM -0500, Doug Ledford wrote:
> Like last time, this has been through a full compile test, but not a boot 
> test.  I'm heading up to the office next so the boot test will come 
> shortly.  But, because it touches lots of files, again, I thought I would 
> go ahead and push it up for people to see.  If there are no objections to 
> this or my previous patches, then once the boot test is complete and I'm 
> satisfied things are working properly, I'll push the whole lot to Linus 
> (which will necessarily include the scsi-misc-2.5 changes because they are 
> in my tree as well).
> 
> bk pull bk://linux-scsi.bkbits.net/scsi-dledford
> 
> New Changeset added since last night:
> ChangeSet@1.852, 2002-11-22 12:59:10-05:00, dledford@dledford.theledfords.org
>   Convert from host->host_queue to host->my_devices list usage
>   Add in usage of new same_target_siblings list
>   Add scsi_release_commandblocks() call to scsi_free_sdev()
>   Make all scsi device freeing use scsi_free_sdev()

Hi Doug -

It booted and runs fine on a netfinity + aic. I haven't updated my 
modutils yet, so did not try the qla driver (ported to your new naming
scheme, I need the qla to test with a qlogics 2300 with a multi-lun target).

Was there a missing scsi_release_commandblocks call?

Why not remove the next/prev in scsi_device? Any code referencing
them is broken, usage should at least generate a compilation warning.

The slave_destroy should be called before the channel/id/lun is modified
during scanning, so slave_destroy if needed can index off of 
host/channel/id/lun and figure out properly what to destroy. Perhaps there
should be a common function for this that calls slave_destroy, modifies
sdevscan channel/id/lun, and then calls slave_alloc; such a function might
also release and build command blocks.

The report lun usage does not modify the channel/id/lun, so does not need
the slave_destroy/slave_alloc calls.

IMO the traversal of scsi_devices should be completely transparent
to the user, such as a wrapper or call back function, so we would not 
have to change anything if the underlying data structure changed (the
list of scsi_hosts contains a list of scsi_devices). Namely, for
multi-path  it is useful to have all the scsi_devices on one list, and
not have a host centric view of the scsi_devices.

-- Patrick Mansfield

Re: One more change pushed to bk tree

[Doug Ledford]

On Fri, Nov 22, 2002 at 03:56:15PM -0800, Patrick Mansfield wrote:
> Hi Doug -
> 
> It booted and runs fine on a netfinity + aic. I haven't updated my 
> modutils yet, so did not try the qla driver (ported to your new naming
> scheme, I need the qla to test with a qlogics 2300 with a multi-lun target).
> 
> Was there a missing scsi_release_commandblocks call?

Yep (at least it looked like it, so I decided better safe than sorry since 
scsi_release_commandblocks() is safe even when there are none to release).

> Why not remove the next/prev in scsi_device? Any code referencing
> them is broken, usage should at least generate a compilation warning.

I was going to in my next patch.  The first was to change it over, the 
next would have been various cleanups.

> The slave_destroy should be called before the channel/id/lun is modified
> during scanning, so slave_destroy if needed can index off of 
> host/channel/id/lun and figure out properly what to destroy.

Hmmm...I thought that's how I did it....

> Perhaps there
> should be a common function for this that calls slave_destroy, modifies
> sdevscan channel/id/lun, and then calls slave_alloc; such a function might
> also release and build command blocks.

Well, before we go too far down this road, I do want to pipe up and 
publically announce how much I thoroughly *DETEST* that whole section of 
code with its totally convoluted hoop jumping crap that it does to avoid 
calling blk_init_queue() on each scan target.  Personally, if it were up 
to me I would make a much simpler/faster queue init routine, quit this 
crap of passing around and reusing the same pointer, and clean up *tons* 
of crap in the scsi scan code.  That being said, anything that increases 
the complexity of that code is bad IMHO.

Furthermore, one of my intended near future patches, pending review of the
idea because this is a change that would touch *A LOT* of files, is to get
rid of scsi_{build,release}_commandblocks entirely and instead change the
scsi subsystem to use generic command blocks that are applicable to all
hosts, modify all low level drivers to use the cmd->device pointer to get
the host/bus/target id/lun values so that they don't need device specific
command blocks, then to allocate a small pool of these on startup and lazy
allocate/deallocate them as we run and as scsi_adjust_queue_depth() starts
getting called for various devices.  If that happens, then most of the 
discussion about command blocks is moot.  Then, since currently I'm the 
only user of slave_alloc(), it would also be easy enough for me to make a 
rule that no device driver is allowed to hard code device numbers in thier 
alloced struct so that we could do this scanning of devices, not need to 
rebuild command blocks, not need to realloc slave structs, and still have 
everything work.  Good enough?

> The report lun usage does not modify the channel/id/lun, so does not need
> the slave_destroy/slave_alloc calls.

Umm...doesn't report lun scan get called on successive targets during a 
host scan using a recycled sdevscan?

> IMO the traversal of scsi_devices should be completely transparent
> to the user, such as a wrapper or call back function, so we would not 
> have to change anything if the underlying data structure changed (the
> list of scsi_hosts contains a list of scsi_devices). Namely, for
> multi-path  it is useful to have all the scsi_devices on one list, and
> not have a host centric view of the scsi_devices.

I personally think it makes more sense to keep the host centric view of 
devices, and for multipath the most I would do is add another list struct 
for chaining together device structs that all point to the same device.

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

Re: One more change pushed to bk tree

[Luben Tuikov]

Doug Ledford wrote:
> 
> Well, before we go too far down this road, I do want to pipe up and
> publically announce how much I thoroughly *DETEST* that whole section of
> code with its totally convoluted hoop jumping crap that it does to avoid
> calling blk_init_queue() on each scan target.  Personally, if it were up
> to me I would make a much simpler/faster queue init routine, quit this
> crap of passing around and reusing the same pointer, and clean up *tons*
> of crap in the scsi scan code.  That being said, anything that increases
> the complexity of that code is bad IMHO.
> 
> Furthermore, one of my intended near future patches, pending review of the
> idea because this is a change that would touch *A LOT* of files, is to get
> rid of scsi_{build,release}_commandblocks entirely and instead change the
> scsi subsystem to use generic command blocks that are applicable to all
> hosts, modify all low level drivers to use the cmd->device pointer to get
> the host/bus/target id/lun values so that they don't need device specific
> command blocks, then to allocate a small pool of these on startup and lazy
> allocate/deallocate them as we run and as scsi_adjust_queue_depth() starts
> getting called for various devices.  If that happens, then most of the
> discussion about command blocks is moot.  Then, since currently I'm the
> only user of slave_alloc(), it would also be easy enough for me to make a
> rule that no device driver is allowed to hard code device numbers in thier
> alloced struct so that we could do this scanning of devices, not need to
> rebuild command blocks, not need to realloc slave structs, and still have
> everything work.  Good enough?

scsi_adjust_queue_depth() would ideally just change an integer,
say device->max_device_cmds or something like that (OSLT). This would play
nicely with int dev->device_cmds_held or int device_cmds_active OSLT, also
being an integer and it wouldn't matter if you _decrease_ the
max, but currently there are more active in the device. (you may
disallow this or set some policy if you wish, etc...)

As a spin off of this, if you set max_device_cmds to 0, you've
effectively turned off the device, but letting it complete what's
active -- one may not want to do this like that of course, but
mentioned here just as a consequence.

The point of all this is that this makes a lot more things a lot
more flexible and makes SCSI core shrink (long, long awaited).

BTW, you've mentioned this change many times before. I don't know
why you haven't implemented it yet, or how structural (non-cosmetic)
changes are voted for in linux-scsi for the SCSI layer, but
you have my vote on this change.

-- 
Luben

Re: One more change pushed to bk tree

[Patrick Mansfield]

Hi Doug -

On Fri, Nov 22, 2002 at 09:45:01PM -0500, Doug Ledford wrote:
> On Fri, Nov 22, 2002 at 03:56:15PM -0800, Patrick Mansfield wrote:
> > The slave_destroy should be called before the channel/id/lun is modified
> > during scanning, so slave_destroy if needed can index off of 
> > host/channel/id/lun and figure out properly what to destroy.
> 
> Hmmm...I thought that's how I did it....

I meant the ones done during scanning, like these back to back calls
in scsi_probe_and_add_lun, where we call both functions with the new
values of host/chan/id/lun:

	if (sdevscan->host->hostt->slave_destroy)
		sdevscan->host->hostt->slave_destroy(sdevscan);
	if (sdevscan->host->hostt->slave_alloc)
		sdevscan->host->hostt->slave_alloc(sdevscan);

> > Perhaps there
> > should be a common function for this that calls slave_destroy, modifies
> > sdevscan channel/id/lun, and then calls slave_alloc; such a function might
> > also release and build command blocks.
> 
> Well, before we go too far down this road, I do want to pipe up and 
> publically announce how much I thoroughly *DETEST* that whole section of 
> code with its totally convoluted hoop jumping crap that it does to avoid 
> calling blk_init_queue() on each scan target.  Personally, if it were up 
> to me I would make a much simpler/faster queue init routine, quit this 
> crap of passing around and reusing the same pointer, and clean up *tons* 
> of crap in the scsi scan code.  That being said, anything that increases 
> the complexity of that code is bad IMHO.

The really ugly loop stuff of 2.4 is gone. We still must pass
host/channel/id/lun, even if the sdevscan is removed, so we would
still have to pass around a bunch of args or a pointer to a 
host/chan/id/lun.

The worst part about the queue setup is that we allocate our own request,
as do the scsi character devices (st, osst and sg), never using the
make_request, so we need no struct request's. So during the scan we are
avoiding allocations/frees of data structures we do not even use. 

We need a two piece blk queue setup - one to setup the queue, and one
to allocate the number of struct request's needed. The latter can
be called at upper level driver attach time (maybe even in 
scsi_attach_device, needs another argument or use an sdev field).

> Furthermore, one of my intended near future patches, pending review of the
> idea because this is a change that would touch *A LOT* of files, is to get
> rid of scsi_{build,release}_commandblocks entirely and instead change the
> scsi subsystem to use generic command blocks that are applicable to all
> hosts, modify all low level drivers to use the cmd->device pointer to get
> the host/bus/target id/lun values so that they don't need device specific
> command blocks, then to allocate a small pool of these on startup and lazy
> allocate/deallocate them as we run and as scsi_adjust_queue_depth() starts
> getting called for various devices.  If that happens, then most of the 
> discussion about command blocks is moot.  Then, since currently I'm the 
> only user of slave_alloc(), it would also be easy enough for me to make a 
> rule that no device driver is allowed to hard code device numbers in thier 
> alloced struct so that we could do this scanning of devices, not need to 
> rebuild command blocks, not need to realloc slave structs, and still have 
> everything work.  Good enough?

Sounds good, though lazy allocation (even like we have now) can have
cases where we are trying to swap out, and suddenly load up the swap
device, and the swap device must then allocate new command blocks while
low on memory - IO will happen but could be slower than optimal.

I would like abstract interfaces so the device drivers don't have to be
modified if the data structures ever change. i.e. something like:

	scsi_get_cmd_host(scmd)
	scsi_get_cmd_channel(scmd)
	scsi_get_cmd_id(scdm)
	scsi_get_cmd_lun(scmd)

If the above returned handles (or structs) the interface could be
abstracted even further, making future changes easier to deal with
(like changing the the lun size; you can grep for the names everywhere
but that's not easy). The above helps me with the scsi multi-path (even
if you don't agree with the approach).

> > The report lun usage does not modify the channel/id/lun, so does not need
> > the slave_destroy/slave_alloc calls.
> 
> Umm...doesn't report lun scan get called on successive targets during a 
> host scan using a recycled sdevscan?

Yes, but the host/chan/id/lun has is already setup for scanning lun 0.
It does not hurt and is cleaner to do the same setup/teardown in all
cases.

> > IMO the traversal of scsi_devices should be completely transparent
> > to the user, such as a wrapper or call back function, so we would not 
> > have to change anything if the underlying data structure changed (the
> > list of scsi_hosts contains a list of scsi_devices). Namely, for
> > multi-path  it is useful to have all the scsi_devices on one list, and
> > not have a host centric view of the scsi_devices.
> 
> I personally think it makes more sense to keep the host centric view of 
> devices, and for multipath the most I would do is add another list struct 
> for chaining together device structs that all point to the same device.

But no matter which way it goes, having an abastracted interface (callback
or whatever) makes it possible to use a host or non-host centric list of
scsi_devices.

The current scsi multi-path patch I have uses a list to chain together
paths to one scsi_device, not a scsi_device per path. If we have a
scsi_device per path, we have the overhead of duplicate block queues
(and lack of sorting/merging across queues), duplicated data in
a scsi_device, we have to figure out what scsi_device to use or expose
to user land, and passing errors into the block layer is difficult
(generally, externalizing the code/data contained in
scsi_decide_disposition).

-- Patrick Mansfield