Skip to contentSkip to conversation

gdth: clean up host private data

PATCH In review Christoph Hellwig 2 participants 2 messages Jul 21, 2007 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 6/7 gdth: clean up host private data

Get rid of all the indirection in the Scsi_Host private data and always but the gdth_ha_str directly into it.

drivers/scsi/gdth.h +3 -13
Index: linux-2.6/drivers/scsi/gdth.h
===================================================================
@@ -853,6 +853,8 @@ typedef struct {
 
 /* controller information structure */
 typedef struct {
+    struct Scsi_Host    *shost;
+    ushort      	hanum;
     ushort              oem_id;                 /* OEM */
     ushort              type;                   /* controller class */
     ulong32             stype;                  /* subtype (PCI: device ID) */
@@ -864,6 +866,7 @@ typedef struct {
     void __iomem        *brd;                   /* DPRAM address */
     ulong32             brd_phys;               /* slot number/BIOS address */
     gdt6c_plx_regs      *plx;                   /* PLX regs (new PCI contr.) */
+    gdth_cmd_str        cmdext;
     gdth_cmd_str        *pccb;                  /* address command structure */
     ulong32             ccb_phys;               /* phys. address */
 #ifdef INT_COAL
@@ -937,19 +940,6 @@ typedef struct {
     struct scsi_device         *sdev;
 } gdth_ha_str;
 
-/* structure for scsi_register(), SCSI bus != 0 */
-typedef struct {
-    ushort      hanum;
-} gdth_num_str;
-
-/* structure for scsi_register() */
-typedef struct {
-    gdth_num_str        numext;                 /* must be the first element */
-    gdth_ha_str         haext;
-    gdth_cmd_str        cmdext;
-} gdth_ext_str;
-
-
 /* INQUIRY data format */
 typedef struct {
     unchar      type_qual;
drivers/scsi/gdth_proc.c +11 -13
Index: linux-2.6/drivers/scsi/gdth_proc.c
===================================================================
@@ -7,17 +7,15 @@
 int gdth_proc_info(struct Scsi_Host *host, char *buffer,char **start,off_t offset,int length,   
                    int inout)
 {
-    int hanum;
+    gdth_ha_str *ha = shost_priv(host);
 
     TRACE2(("gdth_proc_info() length %d offs %d inout %d\n",
             length,(int)offset,inout));
 
-    hanum = NUMDATA(host)->hanum;
-
     if (inout)
-        return(gdth_set_info(buffer,length,host,hanum));
+        return(gdth_set_info(buffer,length,host,ha->hanum));
     else
-        return(gdth_get_info(buffer,start,offset,length,host,hanum));
+        return(gdth_get_info(buffer,start,offset,length,host,ha->hanum));
 }
 
 static int gdth_set_info(char *buffer,int length,struct Scsi_Host *host,
@@ -53,7 +51,7 @@ static int gdth_set_asc_info(struct Scsi
     memset(&gdtcmd, 0, sizeof(gdth_cmd_str));
 
     TRACE2(("gdth_set_asc_info() ha %d\n",hanum));
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     orig_length = length + 5;
     drive = -1;
     wb_mode = 0;
@@ -187,7 +185,7 @@ static int gdth_get_info(char *buffer,ch
     memset(gdtcmd, 0, sizeof(gdth_cmd_str));
 
     TRACE2(("gdth_get_info() ha %d\n",hanum));
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
 
     
     /* request is i.e. "cat /proc/scsi/gdth/0" */ 
@@ -678,7 +676,7 @@ static char *gdth_ioctl_alloc(int hanum,
     if (size == 0)
         return NULL;
 
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
     if (!ha->scratch_busy && size <= GDTH_SCRATCH) {
@@ -703,7 +701,7 @@ static void gdth_ioctl_free(int hanum, i
     gdth_ha_str *ha;
     ulong flags;
 
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
     if (buf == ha->pscratch) {
@@ -722,7 +720,7 @@ static int gdth_ioctl_check_bin(int hanu
     ulong flags;
     int ret_val;
 
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
     ret_val = FALSE;
@@ -743,7 +741,7 @@ static void gdth_wait_completion(int han
     Scsi_Cmnd *scp;
     unchar b, t;
 
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
     for (i = 0; i < GDTH_MAXCMDS; ++i) {
@@ -770,7 +768,7 @@ static void gdth_stop_timeout(int hanum,
     Scsi_Cmnd *scp;
     unchar b, t;
 
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
     for (scp = ha->req_first; scp; scp = (Scsi_Cmnd *)scp->SCp.ptr) {
@@ -793,7 +791,7 @@ static void gdth_start_timeout(int hanum
     Scsi_Cmnd *scp;
     unchar b, t;
 
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
     for (scp = ha->req_first; scp; scp = (Scsi_Cmnd *)scp->SCp.ptr) {
drivers/scsi/gdth.c +55 -69

435 lines not on this page · raw message

#1 lore raw
Quoted 16 lines · Christoph Hellwig

Get rid of all the indirection in the Scsi_Host private data and always but the gdth_ha_str directly into it.

drivers/scsi/gdth_proc.c +1 -1
@@ -678,7 +676,7 @@ static char *gdth_ioctl_alloc(int hanum,
     if (size == 0)
         return NULL;
 
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
     if (!ha->scratch_busy && size <= GDTH_SCRATCH) {

drivers/scsi/gdth_proc.c static void gdth_ioctl_free(int hanum, i · line 707

Quoted 8 lines
drivers/scsi/gdth_proc.c +1 -1
@@ -703,7 +701,7 @@ static void gdth_ioctl_free(int hanum, i
     gdth_ha_str *ha;
     ulong flags;
 
-    ha = HADATA(gdth_ctr_tab[hanum]);
+    ha = shost_priv(gdth_ctr_tab[hanum]);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
     if (buf == ha->pscratch) {

Major problem with this patch: it still directly references statically sized gdth_ctr_tab[].

This patch should be rewritten to apply on top of "gdth: Isolate driver-global variables using helpers". That patch has the clear advantage of isolating the direct references to gdth_ctr_tab[], which means that it can be trivially replaced with a dynamically-sized implementation in a later cleanup, without affecting the code or procfs output at all.

IOW, the "ha from hanum" common operation should clearly have a wrapper.

Once that is complete, your patch here shrinks, since you merely have to update gdth_find_ha() once rather than each codesite.

Jeff

P.S. Are you sure you got achim's email correct? I was using [email protected] since that it what is in gdth.c and his most recent Acked-by entry in the kernel changelog.