PATCH [1/1] cciss: auto engage scsi subsystem for tape support

PATCH [1/1] cciss: auto engage scsi subsystem for tape support

[Mike Miller]

commit 608b0262ce818901b931d7e6534aad375c698924
Author: Stephen M. Cameron <scameron@beardog.cce.hp.com>
Date:   Mon Aug 29 13:27:46 2011 -0500

    cciss: auto engage scsi mid-layer

Acked-by: Mike Miller <mike.miller@hp.com>

diff --git a/Documentation/blockdev/cciss.txt b/Documentation/blockdev/cciss.txt
index c00c6a5..7c81283 100644
--- a/Documentation/blockdev/cciss.txt
+++ b/Documentation/blockdev/cciss.txt
@@ -88,14 +88,12 @@ You must enable "SCSI tape drive support for Smart Array 5xxx" and
 "SCSI support" in your kernel configuration to be able to use SCSI
 tape drives with your Smart Array 5xxx controller.
 
-Additionally, note that the driver will not engage the SCSI core at init 
-time.  The driver must be directed to dynamically engage the SCSI core via 
-the /proc filesystem entry which the "block" side of the driver creates as 
-/proc/driver/cciss/cciss* at runtime.  This is because at driver init time, 
-the SCSI core may not yet be initialized (because the driver is a block 
-driver) and attempting to register it with the SCSI core in such a case 
-would cause a hang.  This is best done via an initialization script 
-(typically in /etc/init.d, but could vary depending on distribution). 
+Additionally, note that the driver will engage the SCSI core at init 
+time if any tape drives or medium changers are detected.  The driver may
+also be directed to dynamically engage the SCSI core via the /proc filesystem
+entry which the "block" side of the driver creates as
+/proc/driver/cciss/cciss* at runtime.  This is best done via a script.
+
 For example:
 
 	for x in /proc/driver/cciss/cciss[0-9]*
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 8f4ef65..c945024 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -5126,6 +5126,7 @@ reinit_after_soft_reset:
 	h->cciss_max_sectors = 8192;
 
 	rebuild_lun_table(h, 1, 0);
+	cciss_engage_scsi(h);
 	h->busy_initializing = 0;
 	return 1;
 
diff --git a/drivers/block/cciss_scsi.c b/drivers/block/cciss_scsi.c
index 951a4e3..e820b68 100644
--- a/drivers/block/cciss_scsi.c
+++ b/drivers/block/cciss_scsi.c
@@ -1720,5 +1720,6 @@ static int  cciss_eh_abort_handler(struct scsi_cmnd *scsicmd)
 /* If no tape support, then these become defined out of existence */
 
 #define cciss_scsi_setup(cntl_num)
+#define cciss_engage_scsi(h)
 
 #endif /* CONFIG_CISS_SCSI_TAPE */

Re: PATCH [1/1] cciss: auto engage scsi subsystem for tape support

[Andrew Morton]

On Fri, 16 Sep 2011 16:32:00 -0500
Mike Miller <mike.miller@hp.com> wrote:

> 
> commit 608b0262ce818901b931d7e6534aad375c698924
> Author: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> Date:   Mon Aug 29 13:27:46 2011 -0500
> 
>     cciss: auto engage scsi mid-layer
> 
> Acked-by: Mike Miller <mike.miller@hp.com>

Nobody seems to have applied this to anything.

The patch has no signoffs.

The changelog is awful.  What are the end-user-visible effects of this
change?  How is anyone supposed to tell?

> --- a/drivers/block/cciss_scsi.c
> +++ b/drivers/block/cciss_scsi.c
> @@ -1720,5 +1720,6 @@ static int  cciss_eh_abort_handler(struct scsi_cmnd *scsicmd)
>  /* If no tape support, then these become defined out of existence */
>  
>  #define cciss_scsi_setup(cntl_num)
> +#define cciss_engage_scsi(h)
>  
>  #endif /* CONFIG_CISS_SCSI_TAPE */

cciss #includes .c files?  I can't believe you did that :(

Re: PATCH [1/1] cciss: auto engage scsi subsystem for tape support

[scameron]

On Thu, Sep 29, 2011 at 04:54:17PM -0700, Andrew Morton wrote:
> On Fri, 16 Sep 2011 16:32:00 -0500
> Mike Miller <mike.miller@hp.com> wrote:
> 
> > 
> > commit 608b0262ce818901b931d7e6534aad375c698924
> > Author: Stephen M. Cameron <scameron@beardog.cce.hp.com>
> > Date:   Mon Aug 29 13:27:46 2011 -0500
> > 
> >     cciss: auto engage scsi mid-layer
> > 
> > Acked-by: Mike Miller <mike.miller@hp.com>
> 
> Nobody seems to have applied this to anything.
> 
> The patch has no signoffs.
> 
> The changelog is awful.  What are the end-user-visible effects of this
> change?  How is anyone supposed to tell?

Eh, Mike sent that one up for me and I guess I hadn't updated
the change log (actually, looking around, it seems like I never
made a patch for kernel.org, so I guess he pulled the patch out of
our local HP repository here.)

I can resend it with a changelog that makes more sense.

The history of this thing goes like this:

Back in 2000 or so, when we were first adding tape drive support
to cciss, one of the distros, or maybe more than one, would load
the block drivers before the scsi mid layer was loaded.  Consequently,
if the block driver tried to call scsi_add_host (or anything from the
scsi mid layer) it would panic.  So we delayed this until after
the scsi mid layer was up, and by poking at the driver from userland,
after the scsi mid layer was up and running we would get it to engage
the scsi mid layer .  That's what this whole

echo engage scsi > /proc/driver/cciss/cciss0

nonsense was about.

At some point along the way (probably a long time ago now) this
ceased to be a problem, and the cciss driver could safely rely
on the scsi mid layer being present straight away, thus removing
the need for this "echo engage scsi > /proc/driver/cciss/cciss0"
nonsense.  So we want to take it out.

The user visible effect is that tape drives and medium changers
show up at driver load time without user intervention or the need
for any scripts in /etc/rc.d to poke at the driver.

> 
> > --- a/drivers/block/cciss_scsi.c
> > +++ b/drivers/block/cciss_scsi.c
> > @@ -1720,5 +1720,6 @@ static int  cciss_eh_abort_handler(struct scsi_cmnd *scsicmd)
> >  /* If no tape support, then these become defined out of existence */
> >  
> >  #define cciss_scsi_setup(cntl_num)
> > +#define cciss_engage_scsi(h)
> >  
> >  #endif /* CONFIG_CISS_SCSI_TAPE */
> 
> cciss #includes .c files?  I can't believe you did that :(

That's been that way for 10 years or so, from the very
beginning.  The rationale, for what it's worth, was that
cciss was originally a block driver, but then a scsi
driver was grafted into it for tape drive support.  It
wasn't at all clear to me at the time that grafting a
scsi driver onto the side of a block driver would be acceptable,
so I wanted to keep them more or less separate, so I could
throw it all away easily if need be.  You might also notice
that cciss_scsi.c and cciss_scsi.h are ancestors of
drivers/scsi/hpsa.c and drivers/scsi/hpsa.h (though enough
changes have occurred by now that they are quite different now,
though still recognizably related, and quite often patches still
are almost compatible with both.)  I probably have an ancient
(2007) patch set lying around somewhere that shows the beginning part
of that evolution, transforming cciss_scsi.[ch] into a working
scsi Smart Array driver.  So keeping those parts separated was
ultimately not without its benefits, despite being a bit
unconventionally ugly.

If it makes you feel any better cciss should be more or less
fading off into irrelevancy in a few years as that hardware ages.

-- steve