Re: [PATCH 6/7] gdth: clean up host private data

[Jeff Garzik <jeff@garzik.org>]

Christoph Hellwig wrote:
> Get rid of all the indirection in the Scsi_Host private data and always
> but the gdth_ha_str directly into it.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

> @@ -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) {

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 
achim_leubner@adaptec.com since that it what is in gdth.c and his most 
recent Acked-by entry in the kernel changelog.