Advanced TCA SCSI/FC disk hotswap driver for kernel 2.5.44

lkml,

Attached is an update of the Advanced TCA disk hotswap driver to provide disk hotswap support for the Linux Kernel 2.5.43. Hotswap targets include both SCSI and FibreChannel. Note this support is generic in that it will work in a typical PC system with SCSI or FibreChannel disks.

I've not yet ported or made suggested changes to the ga mapping driver yet. I'll work on that next.

As always, the latest Advanced TCA hotswap patches are available for Linux 2.4 and Linux 2.5 at the sourceforge site: (please note ga mapper hasn't yet been ported to 2.5)
http://sourceforge.net/project/showfiles.php?group_id=64580

Changes from kernel 2.4.19 to 2.5.44 release:
* Complete port to linux kernel 2.5.44
* Locking of scsi_host->host_queue structure when traversing scsi device list (suggested by Alan Cox)
- This keeps multiple hotswap commands from corrupting the SCSI
device list and
causing system failure
* ioctls deleted and replaced by ramfs scsi_hotswap_fs (suggested by Greg KH)
- removes need for character device node
- usage information available by read() call to files
- available over nfs exported directories
* local file-scope functions declared as static instead of global (suggested by Greg KH)
* code reformatted to fit on 80 character wide screen and to follow linux general kernel instead of scsi subsystem format
* KERNEL_SYSCALLS define removed from header (suggested by Greg KH)
* list of include files optimized to only include files as needed
* Removed direct call from scsi.c into scsi_hotswap_init and used module_init instead
* Made modular so can be compiled as a module or in the kernel directly
* Fixed bug where scsi_hotswap_insert_by_scsi_id didn't properly add a device in some cases

******************************************************

diff -uNr linux-2.5.44-qla/MAINTAINERS linux-2.5.44-scsi-hotswap/MAINTAINERS
@@ -1418,6 +1418,12 @@

W:    http://www.kernel.dk
S:    Maintained

+SCSI HOTSWAP DRIVER

+P:    Steven Dake
+M:    [email protected]
+L:    [email protected]
+S:    Maintained
+
SCSI SG DRIVER
P:    Doug Gilbert
M:    [email protected]

diff -uNr linux-2.5.44-qla/drivers/scsi/Config.help linux-2.5.44-scsi-hotswap/drivers/scsi/Config.help
@@ -34,6 +34,17 @@
 is located on a SCSI disk. In this case, do not compile the driver
 for your SCSI host adapter (below) as a module either.
 
+CONFIG_SCSIFCHOTSWAP
+ If you want to support the ability to include the hotswap FibreChannel
+ and SCSI driver, please say yes here.  Hotswap can then be executed
+ through a ramfs interface which provides better error reporting.
+
+ Further, this interface supports insertion and removal by WWN for
+ FibreChannel drivers which support this feature.
+
+ The only FibreChannel driver that supports this feature is Qlogic V6
+ with a specific support patch.
+

CONFIG_SD_EXTRA_DEVS
This controls the amount of additional space allocated in tables for drivers that are loaded as modules after the kernel is booted. In

diff -uNr linux-2.5.44-qla/drivers/scsi/Config.in linux-2.5.44-scsi-hotswap/drivers/scsi/Config.in
@@ -1,5 +1,6 @@

comment 'SCSI support type (disk, tape, CD-ROM)'

+dep_tristate ' SCSI hotswap support' CONFIG_SCSIFCHOTSWAP $CONFIG_SCSI dep_tristate ' SCSI disk support' CONFIG_BLK_DEV_SD $CONFIG_SCSI

if [ "$CONFIG_BLK_DEV_SD" != "n" ]; then

diff -uNr linux-2.5.44-qla/drivers/scsi/Makefile linux-2.5.44-scsi-hotswap/drivers/scsi/Makefile
@@ -117,6 +117,7 @@
 

obj-$(CONFIG_ARCH_ACORN) += ../acorn/scsi/

+obj-$(CONFIG_SCSIFCHOTSWAP) += hotswap.o
obj-$(CONFIG_CHR_DEV_ST) += st.o
obj-$(CONFIG_CHR_DEV_OSST) += osst.o
obj-$(CONFIG_BLK_DEV_SD) += sd_mod.o

diff -uNr linux-2.5.44-qla/drivers/scsi/hosts.c linux-2.5.44-scsi-hotswap/drivers/scsi/hosts.c
@@ -371,6 +371,7 @@
   scsi_hosts_registered++;
 
   spin_lock_init(&shost->default_lock);
+    spin_lock_init(&shost->host_queue_lock);
   scsi_assign_lock(shost, &shost->default_lock);
   atomic_set(&shost->host_active,0);
 

diff -uNr linux-2.5.44-qla/drivers/scsi/hosts.h linux-2.5.44-scsi-hotswap/drivers/scsi/hosts.h
@@ -264,6 +264,14 @@
    */
   int (* bios_param)(Disk *, struct block_device *, int []);
 
+#ifdef CONFIG_SCSIFCHOTSWAP
+    /*
+     * Used to determine the id to send the inquiry command to during
+     * hot additions of FibreChannel targets
+     */
+    int (*get_scsi_info_from_wwn)(int mode, unsigned long long wwn, int *host, int *channel, int *lun, int *id);
+#endif
+
   /*
    * This determines if we will use a non-interrupt driven
    * or an interrupt driven scheme,  It is set to the maximum number

@@ -374,6 +382,7 @@
    */
   struct list_head      sh_list;
   Scsi_Device           * host_queue;
+    spinlock_t          host_queue_lock;
   struct list_head      all_scsi_hosts;
   struct list_head      my_devices;
 

diff -uNr linux-2.5.44-qla/drivers/scsi/qla2xxx/qla2x00.c linux-2.5.44-scsi-hotswap/drivers/scsi/qla2xxx/qla2x00.c
@@ -244,6 +244,7 @@

STATIC uint8_t qla2x00_register_with_Linux(scsi_qla_host_t *ha, uint8_t maxchannels);
STATIC int qla2x00_done(scsi_qla_host_t *);
//STATIC void qla2x00_select_queue_depth(struct Scsi_Host *, Scsi_Device *);
+int qla2x00_get_scsi_info_from_wwn (int mode, unsigned long long wwn, int *host, int *channel, int *lun, int *id);

STATIC void
qla2x00_timer(scsi_qla_host_t *);
@@ -2037,6 +2038,7 @@
host->can_queue = max_srbs; /* default value:-MAX_SRBS(4096) */ host->cmd_per_lun = 1;
// host->select_queue_depths = qla2x00_select_queue_depth;
+ host->hostt->get_scsi_info_from_wwn = qla2x00_get_scsi_info_from_wwn;

   host->n_io_port = 0xFF;

@@ -3989,6 +3991,80 @@
}
#endif

+union wwnmap {
+ unsigned long long wwn;
+ unsigned char wwn_u8[8];
+};
+
+int qla2x00_get_scsi_info_from_wwn (int mode,

+    unsigned long long wwn,
+    int *host,
+    int *channel,
+    int *lun,
+    int *id) {

+
+scsi_qla_host_t *list;
+Scsi_Device *scsi_device;
+union wwnmap wwncompare;
+union wwnmap wwncompare2;
+int i, j, k;
+
+ /*
+ * Retrieve big endian version of world wide name
+ */
+ wwncompare2.wwn = wwn;
+ for (j = 0, k=7; j < 8; j++, k--) {
+ wwncompare.wwn_u8[j] = wwncompare2.wwn_u8[k];
+ }
+
+ /*
+ * query all hosts searching for WWN
+ */
+ for (list = qla2x00_hostlist; list; list = list->next) {
+ for (i = 0; i < MAX_FIBRE_DEVICES; i++) {
+ /*

+             * Scan all devices in FibreChannel database
+             * if WWN match found, return SCSI device information
+             */

+ if (memcmp (wwncompare.wwn_u8, list->fc_db[i].wwn, 8) == 0) {
+ /*
+ * If inserting, avoid scan for channel and lun information
+ */
+ if (mode == 0) {

+                    *channel = 0;
+                    *lun = 0;
+                    *host = list->host->host_no;
+                    *id = i;
+                    return (0);
+                }
+           +
+                /*
+                 * WWN matches, find channel and lun information from scsi
+                 * device
+                 */
+                for (scsi_device = list->host->host_queue; scsi_device; scsi_device = scsi_device->next) {
+                    if (scsi_device->id == i) {
+                        *channel = scsi_device->channel;
+                        *lun = scsi_device->lun;
+                        break;

+ }
+ }
+ if (scsi_device == 0) {
+ return (-ENOENT);
+ }
+ /*
+ * Device found, return all data
+ */

+                *host = list->host->host_no;
+                *id = i;
+                return (0);

+ } /* memcmp */
+ } /* i < MAXFIBREDEVICES */
+ }
+ return (-ENOENT);
+}
+ /**************************************************************************
* qla2x00_select_queue_depth
*

diff -uNr linux-2.5.44-qla/drivers/scsi/scsi.c linux-2.5.44-scsi-hotswap/drivers/scsi/scsi.c
@@ -413,6 +413,9 @@
                * allow us to more easily figure out whether we should
                * do anything here or not.
                */
+
+                spin_lock (&host->host_queue_lock);
+
               for (SDpnt = host->host_queue;
                    SDpnt;
                    SDpnt = SDpnt->next) {

@@ -430,6 +433,9 @@
                                               break;
                                       }
               }
+
+                spin_unlock (&host->host_queue_lock);
+
               if (SDpnt) {
                   /*
                    * Some other device in this cluster is busy.

@@ -1694,6 +1700,8 @@
       len += size;
       pos = begin + len;

#endif
+ spin_lock (&HBA_ptr->host_queue_lock);
+

       for (scd = HBA_ptr->host_queue; scd; scd = scd->next) {
           proc_print_scsidevice(scd, buffer, &size, len);
           len += size;
@@ -1706,6 +1714,8 @@
           if (pos > offset + length)
               goto stop_output;
       }

+
+ spin_unlock (&HBA_ptr->host_queue_lock);
}

stop_output:
@@ -1864,6 +1874,8 @@

       if (!HBA_ptr)
           goto out;

+ spin_lock (&HBA_ptr->host_queue_lock);
+

       for (scd = HBA_ptr->host_queue; scd; scd = scd->next) {
           if ((scd->channel == channel
                && scd->id == id
@@ -1872,6 +1884,8 @@
           }
       }

+ spin_unlock (&HBA_ptr->host_queue_lock);
+
err = -ENOSYS;
if (scd)
goto out; /* We do not yet support unplugging */
@@ -1910,6 +1924,8 @@

       if (!HBA_ptr)
           goto out;

+ spin_lock (&HBA_ptr->host_queue_lock);
+

       for (scd = HBA_ptr->host_queue; scd; scd = scd->next) {
           if ((scd->channel == channel
                && scd->id == id
@@ -1918,6 +1934,8 @@
           }
       }

+ spin_unlock (&HBA_ptr->host_queue_lock);
+
if (scd == NULL)
goto out; /* there is no such device attached */

@@ -1951,9 +1969,14 @@

           if (scd->prev != NULL)
               scd->prev->next = scd->next;

+ spin_lock (&HBA_ptr->host_queue_lock);
+

           if (HBA_ptr->host_queue == scd) {
               HBA_ptr->host_queue = scd->next;
           }
+
+            spin_unlock (&HBA_ptr->host_queue_lock);
+
           blk_cleanup_queue(&scd->request_queue);
           if (scd->inquiry)
               kfree(scd->inquiry);

@@ -1997,11 +2020,15 @@

   for (shpnt = scsi_host_get_next(NULL); shpnt;
        shpnt = scsi_host_get_next(shpnt)) {
+        spin_lock (&shpnt->host_queue_lock);
+
       for (SDpnt = shpnt->host_queue; SDpnt;
            SDpnt = SDpnt->next) {
           if (tpnt->detect)
               SDpnt->attached += (*tpnt->detect) (SDpnt);
       }

+
+ spin_unlock (&shpnt->host_queue_lock);
}

/*
@@ -2017,6 +2044,8 @@

    */
   for (shpnt = scsi_host_get_next(NULL); shpnt;
        shpnt = scsi_host_get_next(shpnt)) {
+        spin_lock (&shpnt->host_queue_lock);
+
       for (SDpnt = shpnt->host_queue; SDpnt;
            SDpnt = SDpnt->next) {
           if (tpnt->attach)
@@ -2032,6 +2061,8 @@
                   out_of_space = 1;
           }
       }

+
+ spin_unlock (&shpnt->host_queue_lock);
}

/*
@@ -2068,6 +2099,8 @@

   for (shpnt = scsi_host_get_next(NULL); shpnt;
        shpnt = scsi_host_get_next(shpnt)) {
+        spin_lock (&shpnt->host_queue_lock);
+
       for (SDpnt = shpnt->host_queue; SDpnt;
            SDpnt = SDpnt->next) {
           if (tpnt->detach)
@@ -2084,6 +2117,8 @@
               scsi_release_commandblocks(SDpnt);
           }
       }

+
+ spin_unlock (&shpnt->host_queue_lock);
}
/*
* Extract the template from the linked list.
@@ -2154,6 +2189,8 @@
for (shpnt = scsi_host_get_next(NULL); shpnt;
shpnt = scsi_host_get_next(shpnt)) {
printk(KERN_INFO "h:c:t:l (dev sect nsect cnumsec sg) (ret all
flg) (to/cmd to ito) cmd snse result\n");
+ spin_lock (&shpnt->host_queue_lock);
+

       for (SDpnt = shpnt->host_queue; SDpnt; SDpnt = SDpnt->next) {
           for (SCpnt = SDpnt->device_queue; SCpnt; SCpnt = SCpnt->next) {
               /*  (0) h:c:t:l (dev sect nsect cnumsec sg) (ret all 
flg) (to/cmd to ito) cmd snse result %d %x      */
@@ -2185,6 +2222,8 @@
                      SCpnt->result);
           }
       }

+
+ spin_unlock (&shpnt->host_queue_lock);
}
#endif /* CONFIG_SCSI_LOGGING */ /* } */
}

diff -uNr linux-2.5.44-qla/drivers/scsi/scsi_error.c linux-2.5.44-scsi-hotswap/drivers/scsi/scsi_error.c
@@ -202,6 +202,8 @@
   int devices_failed = 0;
 
 
+    spin_lock (&shost->host_queue_lock);
+
   for (sdev = shost->host_queue; sdev; sdev = sdev->next) {
       for (scmd = sc_list; scmd; scmd = scmd->bh_next) {
           if (scmd->device == sdev) {

@@ -227,6 +229,8 @@
       }
   }
 
+    spin_unlock (&shost->host_queue_lock);
+
   SCSI_LOG_ERROR_RECOVERY(2, printk("Total of %d commands on %d"
                     " devices require eh work\n",
                 total_failures, devices_failed));

@@ -247,6 +251,8 @@
   Scsi_Device *sdev;
   Scsi_Cmnd *scmd;
 
+    spin_lock (&shost->host_queue_lock);
+
   for (found = 0, sdev = shost->host_queue; sdev; sdev = sdev->next) {
       for (scmd = sdev->device_queue; scmd; scmd = scmd->next) {
           if (scsi_eh_eflags_chk(scmd, SCSI_EH_CMD_ERR)) {

@@ -283,6 +289,8 @@
       }
   }
 
+    spin_unlock (&shost->host_queue_lock);
+
   SCSI_LOG_ERROR_RECOVERY(1, scsi_eh_prt_fail_stats(*sc_list, shost));
 
   if (shost->host_failed != found)

@@ -962,6 +970,8 @@
 
   SCSI_LOG_ERROR_RECOVERY(3, printk("%s: Trying BDR\n", __FUNCTION__));
 
+    spin_lock (&shost->host_queue_lock);
+
   for (sdev = shost->host_queue; sdev; sdev = sdev->next) {
       for (scmd = sc_todo; scmd; scmd = scmd->bh_next)
           if ((scmd->device == sdev) &&

@@ -985,6 +995,7 @@
                           scsi_eh_finish_cmd(scmd, shost);
                   }
   }
+    spin_unlock (&shost->host_queue_lock);
 
   return shost->host_failed;

}
@@ -1016,11 +1027,15 @@

       /*
        * Mark all affected devices to expect a unit attention.
        */
+        spin_lock (&scmd->host->host_queue_lock);
+
       for (sdev = scmd->host->host_queue; sdev; sdev = sdev->next)
           if (scmd->channel == sdev->channel) {
               sdev->was_reset = 1;
               sdev->expecting_cc_ua = 1;
           }

+
+ spin_unlock (&scmd->host->host_queue_lock);
}
return rtn;
}
@@ -1052,11 +1067,13 @@

       /*
        * Mark all affected devices to expect a unit attention.
        */
+        spin_lock (&scmd->host->host_queue_lock);
       for (sdev = scmd->host->host_queue; sdev; sdev = sdev->next)
           if (scmd->channel == sdev->channel) {
               sdev->was_reset = 1;
               sdev->expecting_cc_ua = 1;
           }

+ spin_unlock (&scmd->host->host_queue_lock);
}
return rtn;
}
@@ -1475,6 +1492,8 @@
* requests are started.
*/
spin_lock_irqsave(shost->host_lock, flags);
+ spin_lock (&shost->host_queue_lock);
+

   for (sdev = shost->host_queue; sdev; sdev = sdev->next) {
       request_queue_t *q = &sdev->request_queue;

@@ -1487,6 +1506,8 @@

       q->request_fn(q);

}
+
+ spin_unlock (&shost->host_queue_lock);
spin_unlock_irqrestore(shost->host_lock, flags);
}

diff -uNr linux-2.5.44-qla/drivers/scsi/scsi_lib.c linux-2.5.44-scsi-hotswap/drivers/scsi/scsi_lib.c
@@ -261,6 +261,8 @@
   if (SDpnt->single_lun && blk_queue_empty(q) && SDpnt->device_busy ==0) {
       request_queue_t *q;
 
+        spin_lock (&SHpnt->host_queue_lock);
+
       for (SDpnt = SHpnt->host_queue; SDpnt; SDpnt = SDpnt->next) {
           if (((SHpnt->can_queue > 0)
                && (SHpnt->host_busy >= SHpnt->can_queue))

@@ -273,6 +275,8 @@
           q = &SDpnt->request_queue;
           q->request_fn(q);
       }
+
+        spin_unlock (&SHpnt->host_queue_lock);
   }
 
   /*

@@ -285,6 +289,7 @@
    */
   all_clear = 1;
   if (SHpnt->some_device_starved) {
+        spin_lock (&SHpnt->host_queue_lock);
       for (SDpnt = SHpnt->host_queue; SDpnt; SDpnt = SDpnt->next) {
           request_queue_t *q;
           if ((SHpnt->can_queue > 0 && (SHpnt->host_busy >= 

SHpnt->can_queue))
@@ -299,6 +304,7 @@

           q->request_fn(q);
           all_clear = 0;
       }
+        spin_unlock (&SHpnt->host_queue_lock);
       if (SDpnt == NULL && all_clear) {
           SHpnt->some_device_starved = 0;
       }

@@ -1038,8 +1044,13 @@

SHpnt->host_self_blocked = FALSE;
/* Now that we are unblocked, try to start the queues. */
+
+ spin_lock (&SHpnt->host_queue_lock);
+
for (SDloop = SHpnt->host_queue; SDloop; SDloop = SDloop->next)

       scsi_queue_next_request(&SDloop->request_queue, NULL);

+
+ spin_unlock (&SHpnt->host_queue_lock);
}

/*
@@ -1066,12 +1077,17 @@
void scsi_report_bus_reset(struct Scsi_Host * SHpnt, int channel) {
Scsi_Device *SDloop;
+
+ spin_lock (&SHpnt->host_queue_lock);
+

   for (SDloop = SHpnt->host_queue; SDloop; SDloop = SDloop->next) {
       if (channel == SDloop->channel) {
           SDloop->was_reset = 1;
           SDloop->expecting_cc_ua = 1;
       }

}
+
+ spin_unlock (&SHpnt->host_queue_lock);
}

/*

diff -uNr linux-2.5.44-qla/drivers/scsi/scsi_scan.c linux-2.5.44-scsi-hotswap/drivers/scsi/scsi_scan.c
@@ -530,6 +530,8 @@
       /*
        * Add it to the end of the shost->host_queue list.
        */
+        spin_lock (&shost->host_queue_lock);
+
       if (shost->host_queue != NULL) {
           sdev->prev = shost->host_queue;
           while (sdev->prev->next != NULL)

@@ -538,6 +540,8 @@
       } else
           shost->host_queue = sdev;
 
+        spin_unlock (&shost->host_queue_lock);
+
   }
   return (sdev);

}
@@ -554,8 +558,12 @@
{
if (sdev->prev != NULL)

       sdev->prev->next = sdev->next;
-    else
+    else {
+        spin_lock (&sdev->host->host_queue_lock);
       sdev->host->host_queue = sdev->next;

+ spin_unlock (&sdev->host->host_queue_lock);
+ }
+
if (sdev->next != NULL)

       sdev->next->prev = sdev->prev;

First off, your patch is line wrapped, please fix your email client.

Also, please read Documentation/CodingStyle and use tabs. Unless this is a side effect of your email client munging the patch.

Any reason you can't use driverfs for the 2.5 code?

Greg,

Thanks yes, it is definately this brain dead mozilla mailer. I did use tabs of course.

I'm not sure how driverfs would be used by this particular patch. Could you be more specific in stating how this could be used?

Think this looks good for inclusion in 2.5.45?

Thanks
-steve

Your "driver" needs only 6 or so files to work properly, right? Why not just create those files under /sys/bus/scsi/ somewhere? That way you don't have to create your own fs, and only have to provide read and write functions for those files, which would be much smaller.

pcihpfs is moving under driverfs soon, once I get some spare cycles.

I'll let the scsi people make that call, I don't know the scsi layer at all (and don't really want to...)

thanks,

greg k-h

I haven't looked extensively at the code, but I'll give you these hints:
Each scsi device gets a directory in the driverfes hierarchy. If you can obtain a pointer to that device during initialization, you can create attribute files to handle read/write in that directory.

You need to declare a struct device attribute, which looks like:

(from include/linux/device.h)

struct device_attribute {
        struct attribute        attr;
        ssize_t (*show)(struct device * dev, char * buf, size_t count, loff_t off);
        ssize_t (*store)(struct device * dev, const char * buf, size_t count, loff_t off);
};

for each file you want to export.

There is a macro to help, which will fill in the embedded attr structure for you:

DEVICE_ATTR(name,mode,show,store);

which is equivalent to declaring.

struct device_attribute dev_attr_<name> = { ... };

(You can do it manually, too, if you feel really manly).

I assume that the ability to hotswap drives in a feature supported by a subset of SCSI drives. I don't know how you handle checking each device, and it's not really relevant. Whenever you find a device that supports hotswap, then do something like:

{
	Scsi_Device * scsi_dev;
	...
	device_create_file(&scsi_dev->sdev_driverfs_dev,&hotswap_attr); 
	...
}

The attribute descriptor will be reused for each device that exports it. On read() and write(), your show() and store() callbacks will be called, with a pointer to the device for which it was called as the first argument.

I hope this all makes sense... If you have any questions, please feel free to ask.

-pat

Thanks a lot for doing the work. There are still some issues to sort out, but I guess we'll be able to finish getting it integrated into the tree.

Btw, please Cc [email protected] for scsi-related patches.

Please make this method unconditional - even if we don't call it without your option set, the drivers are cluttered up much less this way.

	return scsi_hotswap_mknod(dir, dentry, mode | S_IFDIR, 0);

Looks like this wants a bit nicer line-wrapping (or shorter function names :))

<lots of file_operations snipped)

Mayb you can put a switch into ->read/->write instead of having many different file operations?

Doesn't seem to exist in my tree..

I think this one should go to include/scsi/ instead, that's where the other scsi headers live.

Christoph

Thanks for the comments some responses below.

I used tabs. My mozilla mailer won't let me include patches inline so I guess I can just attach them in the future.

I see your point. Will do.

must be a cut and paste error

I'm going to move the driver to driverfs for the 2.5 patch so most of the comments should fall out. Thanks for looking at the code though!

[SNIP]

Thanks will do.

Good idea.

I've gotten mixed feedback on this one. One thing that annoys me about some of the proc read/write routines to control the kernel is without looking at the code, there is no way to understand how to use them.

The user should be able to read the file and see "how to use" the method exported by the filesystem without looking at the code (imho). This is a big advantage of having a read() method.

I'll replace proc_scsi_gen_writes() routine with this routine instead. The point of this routine is to be a generic call for other kernel functions to call like proc_scsi_gen_write.

I do think hotswap functionality should be removed from proc_scsi_gen_write but that might confuse some folks that rely on that functionality.

This is in my private tree only and is the QLogic 6.1 driver integrated into 2.5.44. This is needed to support FibreChannel hotswap only. The generic kernel can live without this patch until the QLogic driver is integrated...

This is in include/linux so that other kernel functions may call the hotswap operations and have the function prototypes defined. An example here is 1394. We don't want 1394 having to include "../../drivers/scsi/scsi_hotswap.h" I don't think :)

I'll make some of the changes you have suggested and post a new patch.

Thanks for the comments!
-steve

Erik,

I wanted to seperate the hotswap functionality from the main scsi code. I also feel that hotswap shouldn't be a function of proc. Also, the patch I supplied supports fibrechannel hotswap as well. Your patch looks pretty good and I plan to change the proc_scsi_gen_write() to use the scsi hotswap commands in hotswap.c. The advantage of this technique is that the HBA_ptr doesn't have to be known (and this generic code can go in the actual routine instead of the proc parsing routine). The final advantage of having seperate comands is less time is spent passing and parsing text data. Plus my patch provides usage information for those that don't or can't read the source code to understand the interface :)

As far as I'm concerned, I'd be happy to entirely remove the proc_scsi_gen_write code, but I think that might confuse some people.

As for the interfaces stomping each other, I've added correct locking to proc_scsi_gen_write() to ensure that the host_queue is locked during access so changes to it cannot occur during hotswap operations. This keeps a scsi_device entry from being deleted while the list is being parsed, resulting in bad magic.