gdth new set of patches for 2.6.24 stable

gdth new set of patches for 2.6.24 stable

[Boaz Harrosh]

Submitted are a new set of patches, that fix lots of problems
with the gdth driver.

It fixes the following problems:
- scan for drives on hosts. (Already in mainline)
- truly fixes the exit/reboot problems but does call flush() before
  reboot.
- fix crash when accessing array with icpcon management application.
- fix crash when doing $ cat /proc/sys/gdth/0.
  This one still has the below WARN_ON in messages (see <gdth_info> below)
  So there is one more thing hiding in there.
- use pci_get_device
  One of the testers requested if we can also put the move to pci_get_device 
  patch with removal of dependency on PCI_LEGACY, to the stable release.

The patches are for and based on Linux-2.6.24. here is the list of patches:
  [PATCH 1/5] gdth: update deprecated pci_find_device
  [PATCH 2/5] gdth: scan for scsi devices
  [PATCH 3/5] gdth: bugfix for the at-exit problems
  [PATCH 4/5] gdth: fix to internal commands execution
  [PATCH 5/5] gdth: remove gdth cooked up command accessors

Please all test and report your findings.

Thanks in advance
Boaz

---
<gdth_info>
  WARNING: at arch/x86/kernel/pci-dma_32.c:66 dma_free_coherent()
  Pid: 5501, comm: cat Not tainted 2.6.24 #43
   [<c0107137>] dma_free_coherent+0x93/0x95
   [<c025ef73>] gdth_ioctl_free+0x4c/0x69
   [<c0264a36>] gdth_proc_info+0x165f/0x182c
   [<c0111f7a>] update_curr+0xeb/0xf2
   [<c01132aa>] task_rq_lock+0x29/0x50
   [<c0113706>] try_to_wake_up+0x42/0x342
   [<c0113706>] try_to_wake_up+0x42/0x342
   [<c0111a9f>] __wake_up_common+0x46/0x6d
   [<c0113569>] __wake_up+0x32/0x42
   [<c022dad9>] n_tty_receive_buf+0x2e8/0xe97
   [<c022dad9>] n_tty_receive_buf+0x2e8/0xe97
   [<c0111f0a>] update_curr+0x7b/0xf2
   [<c0112625>] enqueue_task_fair+0x27/0x30
   [<c0111783>] enqueue_task+0xa/0x14
   [<c025e351>] proc_scsi_read+0x29/0x3d
   [<c025e328>] proc_scsi_read+0x0/0x3d
   [<c0189704>] proc_file_read+0x1c6/0x279
   [<c018953e>] proc_file_read+0x0/0x279
   [<c0185eca>] proc_reg_read+0x53/0x71
   [<c0185e77>] proc_reg_read+0x0/0x71
   [<c0159968>] vfs_read+0x85/0x11b
   [<c0159d9d>] sys_read+0x41/0x6a
   [<c0102822>] sysenter_past_esp+0x5f/0x85
 </gdth_info>

[PATCH 1/5] gdth: update deprecated pci_find_device

[Boaz Harrosh]

From: Sergio Luis <sergio@larces.uece.br>

Fix compilation warning in gdth.c, which was using the deprecated
pci_find_device.

drivers/scsi/gdth.c:645: warning: 'pci_find_device' is deprecated (declared at include/linux/pci.h:495)

Changing it to use pci_get_device, instead.

Signed-off-by: Sergio Luis <sergio@larces.uece.br>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/Kconfig |    2 +-
 drivers/scsi/gdth.c  |    7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 184c7ae..46fcb82 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -725,7 +725,7 @@ config SCSI_FD_MCS
 
 config SCSI_GDTH
 	tristate "Intel/ICP (former GDT SCSI Disk Array) RAID Controller support"
-	depends on (ISA || EISA || PCI) && SCSI && ISA_DMA_API && PCI_LEGACY
+	depends on (ISA || EISA || PCI) && SCSI && ISA_DMA_API
 	---help---
 	  Formerly called GDT SCSI Disk Array Controller Support.
 
diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index b253b8c..5f0e054 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -643,12 +643,15 @@ static void __init gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
           *cnt, vendor, device));
 
     pdev = NULL;
-    while ((pdev = pci_find_device(vendor, device, pdev)) 
+    while ((pdev = pci_get_device(vendor, device, pdev))
            != NULL) {
         if (pci_enable_device(pdev))
             continue;
-        if (*cnt >= MAXHA)
+        if (*cnt >= MAXHA) {
+            pci_dev_put(pdev);
             return;
+        }
+
         /* GDT PCI controller found, resources are already in pdev */
         pcistr[*cnt].pdev = pdev;
         pcistr[*cnt].irq = pdev->irq;
-- 
1.5.3.3

Re: [PATCH 1/5] gdth: update deprecated pci_find_device

[Jeff Garzik]

Boaz Harrosh wrote:
> From: Sergio Luis <sergio@larces.uece.br>
> 
> Fix compilation warning in gdth.c, which was using the deprecated
> pci_find_device.
> 
> drivers/scsi/gdth.c:645: warning: 'pci_find_device' is deprecated (declared at include/linux/pci.h:495)
> 
> Changing it to use pci_get_device, instead.
> 
> Signed-off-by: Sergio Luis <sergio@larces.uece.br>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  drivers/scsi/Kconfig |    2 +-
>  drivers/scsi/gdth.c  |    7 +++++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> index 184c7ae..46fcb82 100644
> --- a/drivers/scsi/Kconfig
> +++ b/drivers/scsi/Kconfig
> @@ -725,7 +725,7 @@ config SCSI_FD_MCS
>  
>  config SCSI_GDTH
>  	tristate "Intel/ICP (former GDT SCSI Disk Array) RAID Controller support"
> -	depends on (ISA || EISA || PCI) && SCSI && ISA_DMA_API && PCI_LEGACY
> +	depends on (ISA || EISA || PCI) && SCSI && ISA_DMA_API
>  	---help---
>  	  Formerly called GDT SCSI Disk Array Controller Support.
>  
> diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
> index b253b8c..5f0e054 100644
> --- a/drivers/scsi/gdth.c
> +++ b/drivers/scsi/gdth.c
> @@ -643,12 +643,15 @@ static void __init gdth_search_dev(gdth_pci_str *pcistr, ushort *cnt,
>            *cnt, vendor, device));
>  
>      pdev = NULL;
> -    while ((pdev = pci_find_device(vendor, device, pdev)) 
> +    while ((pdev = pci_get_device(vendor, device, pdev))
>             != NULL) {
>          if (pci_enable_device(pdev))
>              continue;
> -        if (*cnt >= MAXHA)
> +        if (*cnt >= MAXHA) {
> +            pci_dev_put(pdev);
>              return;
> +        }

This patch is already upstream...  (unfortunately)

	Jeff

Re: [PATCH 1/5] gdth: update deprecated pci_find_device

[James Bottomley]

On Thu, 2008-02-14 at 15:13 -0500, Jeff Garzik wrote:
> Boaz Harrosh wrote:
> > From: Sergio Luis <sergio@larces.uece.br>
> > 
> > Fix compilation warning in gdth.c, which was using the deprecated
> > pci_find_device.
[...]
> This patch is already upstream...  (unfortunately)

I think, in spite of the cover name (patches for stable), these are for
the set of testers who're based on 2.6.24.

James

Re: [PATCH 1/5] gdth: update deprecated pci_find_device

[Jeff Garzik]

James Bottomley wrote:
> On Thu, 2008-02-14 at 15:13 -0500, Jeff Garzik wrote:
>> Boaz Harrosh wrote:
>>> From: Sergio Luis <sergio@larces.uece.br>
>>>
>>> Fix compilation warning in gdth.c, which was using the deprecated
>>> pci_find_device.
> [...]
>> This patch is already upstream...  (unfortunately)
> 
> I think, in spite of the cover name (patches for stable), these are for
> the set of testers who're based on 2.6.24.

Oops!  My mistake, then.

	Jeff

[PATCH 2/5] gdth: scan for scsi devices

[Boaz Harrosh]

The patch: "gdth: switch to modern scsi host registration"

missed one simple fact when moving a way from scsi_module.c.
That is to call scsi_scan_host() on the probed host.
With this the gdth driver from 2.6.24 is again able to
see drives and boot.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
Tested-by: Joerg Dorchain: <joerg@dorchain.net>
Tested-by: Stefan Priebe <s.priebe@allied-internet.ag>
Tested-by: Jon Chelton <jchelton@ffpglobal.com>
---
 drivers/scsi/gdth.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 5f0e054..4959037 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -4841,6 +4841,9 @@ static int __init gdth_isa_probe_one(ulong32 isa_bios)
 	if (error)
 		goto out_free_coal_stat;
 	list_add_tail(&ha->list, &gdth_instances);
+
+	scsi_scan_host(shp);
+
 	return 0;
 
  out_free_coal_stat:
@@ -4968,6 +4971,9 @@ static int __init gdth_eisa_probe_one(ushort eisa_slot)
 	if (error)
 		goto out_free_coal_stat;
 	list_add_tail(&ha->list, &gdth_instances);
+
+	scsi_scan_host(shp);
+
 	return 0;
 
  out_free_ccb_phys:
@@ -5105,6 +5111,9 @@ static int __init gdth_pci_probe_one(gdth_pci_str *pcistr, int ctr)
 	if (error)
 		goto out_free_coal_stat;
 	list_add_tail(&ha->list, &gdth_instances);
+
+	scsi_scan_host(shp);
+
 	return 0;
 
  out_free_coal_stat:
-- 
1.5.3.3

[PATCH 3/5] gdth: bugfix for the at-exit problems

[Boaz Harrosh]

This is a bugfix for the 2.6.24.x stable releases.

gdth_exit would first remove all cards then stop the timer
and would not sync with the timer function. This caused a crash
in gdth_timer() when module was unloaded.
So del_timer_sync the timer before we delete the cards.

also the reboot notifier function would crash. So clean
that up and fix the crashes.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/gdth.c |   82 +++++++++++++++++---------------------------------
 1 files changed, 28 insertions(+), 54 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 4959037..0979553 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -183,7 +183,6 @@ static int gdth_ioctl(struct inode *inode, struct file *filep,
                       unsigned int cmd, unsigned long arg);
 
 static void gdth_flush(gdth_ha_str *ha);
-static int gdth_halt(struct notifier_block *nb, ulong event, void *buf);
 static int gdth_queuecommand(Scsi_Cmnd *scp,void (*done)(Scsi_Cmnd *));
 static int __gdth_queuecommand(gdth_ha_str *ha, struct scsi_cmnd *scp,
 				struct gdth_cmndinfo *cmndinfo);
@@ -418,12 +417,6 @@ static inline void gdth_set_sglist(struct scsi_cmnd *cmd,
 #include "gdth_proc.h"
 #include "gdth_proc.c"
 
-/* notifier block to get a notify on system shutdown/halt/reboot */
-static struct notifier_block gdth_notifier = {
-    gdth_halt, NULL, 0
-};
-static int notifier_disabled = 0;
-
 static gdth_ha_str *gdth_find_ha(int hanum)
 {
 	gdth_ha_str *ha;
@@ -3796,6 +3789,8 @@ static void gdth_timeout(ulong data)
     gdth_ha_str *ha;
     ulong flags;
 
+    BUG_ON(list_empty(&gdth_instances));
+
     ha = list_first_entry(&gdth_instances, gdth_ha_str, list);
     spin_lock_irqsave(&ha->smp_lock, flags);
 
@@ -4671,45 +4666,6 @@ static void gdth_flush(gdth_ha_str *ha)
     }
 }
 
-/* shutdown routine */
-static int gdth_halt(struct notifier_block *nb, ulong event, void *buf)
-{
-    gdth_ha_str *ha;
-#ifndef __alpha__
-    gdth_cmd_str    gdtcmd;
-    char            cmnd[MAX_COMMAND_SIZE];   
-#endif
-
-    if (notifier_disabled)
-        return NOTIFY_OK;
-
-    TRACE2(("gdth_halt() event %d\n",(int)event));
-    if (event != SYS_RESTART && event != SYS_HALT && event != SYS_POWER_OFF)
-        return NOTIFY_DONE;
-
-    notifier_disabled = 1;
-    printk("GDT-HA: Flushing all host drives .. ");
-    list_for_each_entry(ha, &gdth_instances, list) {
-        gdth_flush(ha);
-
-#ifndef __alpha__
-        /* controller reset */
-        memset(cmnd, 0xff, MAX_COMMAND_SIZE);
-        gdtcmd.BoardNode = LOCALBOARD;
-        gdtcmd.Service = CACHESERVICE;
-        gdtcmd.OpCode = GDT_RESET;
-        TRACE2(("gdth_halt(): reset controller %d\n", ha->hanum));
-        gdth_execute(ha->shost, &gdtcmd, cmnd, 10, NULL);
-#endif
-    }
-    printk("Done.\n");
-
-#ifdef GDTH_STATISTICS
-    del_timer(&gdth_timer);
-#endif
-    return NOTIFY_OK;
-}
-
 /* configure lun */
 static int gdth_slave_configure(struct scsi_device *sdev)
 {
@@ -5144,13 +5100,13 @@ static void gdth_remove_one(gdth_ha_str *ha)
 
 	scsi_remove_host(shp);
 
+	gdth_flush(ha);
+
 	if (ha->sdev) {
 		scsi_free_host_dev(ha->sdev);
 		ha->sdev = NULL;
 	}
 
-	gdth_flush(ha);
-
 	if (shp->irq)
 		free_irq(shp->irq,ha);
 
@@ -5176,6 +5132,24 @@ static void gdth_remove_one(gdth_ha_str *ha)
 	scsi_host_put(shp);
 }
 
+static int gdth_halt(struct notifier_block *nb, ulong event, void *buf)
+{
+	gdth_ha_str *ha;
+
+	TRACE2(("gdth_halt() event %d\n", (int)event));
+	if (event != SYS_RESTART && event != SYS_HALT && event != SYS_POWER_OFF)
+		return NOTIFY_DONE;
+
+	list_for_each_entry(ha, &gdth_instances, list)
+		gdth_flush(ha);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block gdth_notifier = {
+    gdth_halt, NULL, 0
+};
+
 static int __init gdth_init(void)
 {
 	if (disable) {
@@ -5238,7 +5212,6 @@ static int __init gdth_init(void)
 	add_timer(&gdth_timer);
 #endif
 	major = register_chrdev(0,"gdth", &gdth_fops);
-	notifier_disabled = 0;
 	register_reboot_notifier(&gdth_notifier);
 	gdth_polling = FALSE;
 	return 0;
@@ -5248,14 +5221,15 @@ static void __exit gdth_exit(void)
 {
 	gdth_ha_str *ha;
 
-	list_for_each_entry(ha, &gdth_instances, list)
-		gdth_remove_one(ha);
+	unregister_chrdev(major, "gdth");
+	unregister_reboot_notifier(&gdth_notifier);
 
 #ifdef GDTH_STATISTICS
-	del_timer(&gdth_timer);
+	del_timer_sync(&gdth_timer);
 #endif
-	unregister_chrdev(major,"gdth");
-	unregister_reboot_notifier(&gdth_notifier);
+
+	list_for_each_entry(ha, &gdth_instances, list)
+		gdth_remove_one(ha);
 }
 
 module_init(gdth_init);
-- 
1.5.3.3

[PATCH 4/5] gdth: fix to internal commands execution

[Boaz Harrosh]

The recent patch named:
  [SCSI] gdth: !use_sg cleanup and use of scsi accessors

has done a bad job in handling internal commands issued by gdth_execute().

Internal commands are issued with device gdth_cmd_str ready made directly
to the card, without any mapping or translations of scsi commands. So here
I added a gdth_cmd_str pointer to the gdth_cmndinfo private structure which
is then copied directly to host.

following this patch is a cleanup that removes the home cooked accessors
and reverts them to regular scsi_cmnd accessors. Since they are not used
anymore. After review maybe the 2 patches should be squashed together.

FIXME: There is still a problem with gdth_get_info(). as reported there
   is a WARN_ON trigerd in dma_free_coherent() when doing:
   $ cat /proc/sys/gdth/0

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/gdth.c |   30 ++++++++++++------------------
 drivers/scsi/gdth.h |    1 +
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 0979553..077f972 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -160,7 +160,7 @@ static void gdth_readapp_event(gdth_ha_str *ha, unchar application,
 static void gdth_clear_events(void);
 
 static void gdth_copy_internal_data(gdth_ha_str *ha, Scsi_Cmnd *scp,
-                                    char *buffer, ushort count, int to_buffer);
+                                    char *buffer, ushort count);
 static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp);
 static int gdth_fill_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp, ushort hdrive);
 
@@ -439,8 +439,8 @@ static struct gdth_cmndinfo *gdth_get_cmndinfo(gdth_ha_str *ha)
 	for (i=0; i<GDTH_MAXCMDS; ++i) {
 		if (ha->cmndinfo[i].index == 0) {
 			priv = &ha->cmndinfo[i];
-			priv->index = i+1;
 			memset(priv, 0, sizeof(*priv));
+			priv->index = i+1;
 			break;
 		}
 	}
@@ -487,7 +487,6 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,
     gdth_ha_str *ha = shost_priv(sdev->host);
     Scsi_Cmnd *scp;
     struct gdth_cmndinfo cmndinfo;
-    struct scatterlist one_sg;
     DECLARE_COMPLETION_ONSTACK(wait);
     int rval;
 
@@ -501,13 +500,10 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,
     /* use request field to save the ptr. to completion struct. */
     scp->request = (struct request *)&wait;
     scp->timeout_per_command = timeout*HZ;
-    sg_init_one(&one_sg, gdtcmd, sizeof(*gdtcmd));
-    gdth_set_sglist(scp, &one_sg);
-    gdth_set_sg_count(scp, 1);
-    gdth_set_bufflen(scp, sizeof(*gdtcmd));
     scp->cmd_len = 12;
     memcpy(scp->cmnd, cmnd, 12);
     cmndinfo.priority = IOCTL_PRI;
+    cmndinfo.internal_cmd_str = gdtcmd;
     cmndinfo.internal_command = 1;
 
     TRACE(("__gdth_execute() cmd 0x%x\n", scp->cmnd[0]));
@@ -2351,7 +2347,7 @@ static void gdth_next(gdth_ha_str *ha)
  * buffers, kmap_atomic() as needed.
  */
 static void gdth_copy_internal_data(gdth_ha_str *ha, Scsi_Cmnd *scp,
-                                    char *buffer, ushort count, int to_buffer)
+                                    char *buffer, ushort count)
 {
     ushort cpcount,i, max_sg = gdth_sg_count(scp);
     ushort cpsum,cpnow;
@@ -2377,10 +2373,7 @@ static void gdth_copy_internal_data(gdth_ha_str *ha, Scsi_Cmnd *scp,
             }
             local_irq_save(flags);
             address = kmap_atomic(sg_page(sl), KM_BIO_SRC_IRQ) + sl->offset;
-            if (to_buffer)
-                memcpy(buffer, address, cpnow);
-            else
-                memcpy(address, buffer, cpnow);
+            memcpy(address, buffer, cpnow);
             flush_dcache_page(sg_page(sl));
             kunmap_atomic(address, KM_BIO_SRC_IRQ);
             local_irq_restore(flags);
@@ -2434,7 +2427,7 @@ static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
         strcpy(inq.vendor,ha->oem_name);
         sprintf(inq.product,"Host Drive  #%02d",t);
         strcpy(inq.revision,"   ");
-        gdth_copy_internal_data(ha, scp, (char*)&inq, sizeof(gdth_inq_data), 0);
+        gdth_copy_internal_data(ha, scp, (char*)&inq, sizeof(gdth_inq_data));
         break;
 
       case REQUEST_SENSE:
@@ -2444,7 +2437,7 @@ static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
         sd.key       = NO_SENSE;
         sd.info      = 0;
         sd.add_length= 0;
-        gdth_copy_internal_data(ha, scp, (char*)&sd, sizeof(gdth_sense_data), 0);
+        gdth_copy_internal_data(ha, scp, (char*)&sd, sizeof(gdth_sense_data));
         break;
 
       case MODE_SENSE:
@@ -2456,7 +2449,7 @@ static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
         mpd.bd.block_length[0] = (SECTOR_SIZE & 0x00ff0000) >> 16;
         mpd.bd.block_length[1] = (SECTOR_SIZE & 0x0000ff00) >> 8;
         mpd.bd.block_length[2] = (SECTOR_SIZE & 0x000000ff);
-        gdth_copy_internal_data(ha, scp, (char*)&mpd, sizeof(gdth_modep_data), 0);
+        gdth_copy_internal_data(ha, scp, (char*)&mpd, sizeof(gdth_modep_data));
         break;
 
       case READ_CAPACITY:
@@ -2466,7 +2459,7 @@ static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
         else
             rdc.last_block_no = cpu_to_be32(ha->hdr[t].size-1);
         rdc.block_length  = cpu_to_be32(SECTOR_SIZE);
-        gdth_copy_internal_data(ha, scp, (char*)&rdc, sizeof(gdth_rdcap_data), 0);
+        gdth_copy_internal_data(ha, scp, (char*)&rdc, sizeof(gdth_rdcap_data));
         break;
 
       case SERVICE_ACTION_IN:
@@ -2478,7 +2471,7 @@ static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
             rdc16.last_block_no = cpu_to_be64(ha->hdr[t].size-1);
             rdc16.block_length  = cpu_to_be32(SECTOR_SIZE);
             gdth_copy_internal_data(ha, scp, (char*)&rdc16,
-                                                 sizeof(gdth_rdcap16_data), 0);
+                                                 sizeof(gdth_rdcap16_data));
         } else { 
             scp->result = DID_ABORT << 16;
         }
@@ -2848,6 +2841,7 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar b)
 static int gdth_special_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
 {
     register gdth_cmd_str *cmdp;
+    struct gdth_cmndinfo *cmndinfo = gdth_cmnd_priv(scp);
     int cmd_index;
 
     cmdp= ha->pccb;
@@ -2856,7 +2850,7 @@ static int gdth_special_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
     if (ha->type==GDT_EISA && ha->cmd_cnt>0) 
         return 0;
 
-    gdth_copy_internal_data(ha, scp, (char *)cmdp, sizeof(gdth_cmd_str), 1);
+    *cmdp = *cmndinfo->internal_cmd_str;
     cmdp->RequestBuffer = scp;
 
     /* search free command index */
diff --git a/drivers/scsi/gdth.h b/drivers/scsi/gdth.h
index 1434c6b..26e4e92 100644
--- a/drivers/scsi/gdth.h
+++ b/drivers/scsi/gdth.h
@@ -915,6 +915,7 @@ typedef struct {
     struct gdth_cmndinfo {                      /* per-command private info */
         int index;
         int internal_command;                   /* don't call scsi_done */
+        gdth_cmd_str *internal_cmd_str;         /* crier for internal messages*/
         dma_addr_t sense_paddr;                 /* sense dma-addr */
         unchar priority;
         int timeout;
-- 
1.5.3.3

[PATCH 5/5] gdth: remove gdth cooked up command accessors

[Boaz Harrosh]

Last patch rendered these, to stupid status. So bye bye

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/scsi/gdth.c |   73 ++++++++++-----------------------------------------
 1 files changed, 14 insertions(+), 59 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 077f972..cfce0fe 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -85,10 +85,10 @@
 
 /* The meaning of the Scsi_Pointer members in this driver is as follows:
  * ptr:                     Chaining
- * this_residual:           gdth_bufflen
- * buffer:                  gdth_sglist
+ * this_residual:           unused
+ * buffer:                  unused
  * dma_handle:              unused
- * buffers_residual:        gdth_sg_count
+ * buffers_residual:        unused
  * Status:                  unused
  * Message:                 unused
  * have_data_in:            unused
@@ -373,47 +373,6 @@ static const struct file_operations gdth_fops = {
     .release = gdth_close,
 };
 
-/*
- * gdth scsi_command access wrappers.
- *   below 6 functions are used throughout the driver to access scsi_command's
- *   io parameters. The reason we do not use the regular accessors from
- *   scsi_cmnd.h is because of gdth_execute(). Since it is unrecommended for
- *   llds to directly set scsi_cmnd's IO members. This driver will use SCp
- *   members for IO parameters, and will copy scsi_cmnd's members to Scp
- *   members in queuecommand. For internal commands through gdth_execute()
- *   SCp's members will be set directly.
- */
-static inline unsigned gdth_bufflen(struct scsi_cmnd *cmd)
-{
-	return (unsigned)cmd->SCp.this_residual;
-}
-
-static inline void gdth_set_bufflen(struct scsi_cmnd *cmd, unsigned bufflen)
-{
-	cmd->SCp.this_residual = bufflen;
-}
-
-static inline unsigned gdth_sg_count(struct scsi_cmnd *cmd)
-{
-	return (unsigned)cmd->SCp.buffers_residual;
-}
-
-static inline void gdth_set_sg_count(struct scsi_cmnd *cmd, unsigned sg_count)
-{
-	cmd->SCp.buffers_residual = sg_count;
-}
-
-static inline struct scatterlist *gdth_sglist(struct scsi_cmnd *cmd)
-{
-	return cmd->SCp.buffer;
-}
-
-static inline void gdth_set_sglist(struct scsi_cmnd *cmd,
-                                   struct scatterlist *sglist)
-{
-	cmd->SCp.buffer = sglist;
-}
-
 #include "gdth_proc.h"
 #include "gdth_proc.c"
 
@@ -2349,12 +2308,12 @@ static void gdth_next(gdth_ha_str *ha)
 static void gdth_copy_internal_data(gdth_ha_str *ha, Scsi_Cmnd *scp,
                                     char *buffer, ushort count)
 {
-    ushort cpcount,i, max_sg = gdth_sg_count(scp);
+    ushort cpcount,i, max_sg = scsi_sg_count(scp);
     ushort cpsum,cpnow;
     struct scatterlist *sl;
     char *address;
 
-    cpcount = min_t(ushort, count, gdth_bufflen(scp));
+    cpcount = min_t(ushort, count, scsi_bufflen(scp));
 
     if (cpcount) {
         cpsum=0;
@@ -2362,7 +2321,7 @@ static void gdth_copy_internal_data(gdth_ha_str *ha, Scsi_Cmnd *scp,
             unsigned long flags;
             cpnow = (ushort)sl->length;
             TRACE(("copy_internal() now %d sum %d count %d %d\n",
-                          cpnow, cpsum, cpcount, gdth_bufflen(scp)));
+                          cpnow, cpsum, cpcount, scsi_bufflen(scp)));
             if (cpsum+cpnow > cpcount) 
                 cpnow = cpcount - cpsum;
             cpsum += cpnow;
@@ -2585,10 +2544,10 @@ static int gdth_fill_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp, ushort hdrive)
             cmdp->u.cache.BlockCnt = blockcnt;
         }
 
-        if (gdth_bufflen(scp)) {
+        if (scsi_bufflen(scp)) {
             cmndinfo->dma_dir = (read_write == 1 ?
                 PCI_DMA_TODEVICE : PCI_DMA_FROMDEVICE);   
-            sgcnt = pci_map_sg(ha->pdev, gdth_sglist(scp), gdth_sg_count(scp),
+            sgcnt = pci_map_sg(ha->pdev, scsi_sglist(scp), scsi_sg_count(scp),
                                cmndinfo->dma_dir);
             if (mode64) {
                 struct scatterlist *sl;
@@ -2735,7 +2694,7 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar b)
             cmdp->u.raw64.lun        = l;
             cmdp->u.raw64.bus        = b;
             cmdp->u.raw64.priority   = 0;
-            cmdp->u.raw64.sdlen      = gdth_bufflen(scp);
+            cmdp->u.raw64.sdlen      = scsi_bufflen(scp);
             cmdp->u.raw64.sense_len  = 16;
             cmdp->u.raw64.sense_data = sense_paddr;
             cmdp->u.raw64.direction  = 
@@ -2752,7 +2711,7 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar b)
             cmdp->u.raw.bus        = b;
             cmdp->u.raw.priority   = 0;
             cmdp->u.raw.link_p     = 0;
-            cmdp->u.raw.sdlen      = gdth_bufflen(scp);
+            cmdp->u.raw.sdlen      = scsi_bufflen(scp);
             cmdp->u.raw.sense_len  = 16;
             cmdp->u.raw.sense_data = sense_paddr;
             cmdp->u.raw.direction  = 
@@ -2761,9 +2720,9 @@ static int gdth_fill_raw_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp, unchar b)
             cmdp->u.raw.sg_ranz    = 0;
         }
 
-        if (gdth_bufflen(scp)) {
+        if (scsi_bufflen(scp)) {
             cmndinfo->dma_dir = PCI_DMA_BIDIRECTIONAL;
-            sgcnt = pci_map_sg(ha->pdev, gdth_sglist(scp), gdth_sg_count(scp),
+            sgcnt = pci_map_sg(ha->pdev, scsi_sglist(scp), scsi_sg_count(scp),
                                cmndinfo->dma_dir);
             if (mode64) {
                 struct scatterlist *sl;
@@ -3383,8 +3342,8 @@ static int gdth_sync_event(gdth_ha_str *ha, int service, unchar index,
             /* retry */
             return 2;
         }
-        if (gdth_bufflen(scp))
-            pci_unmap_sg(ha->pdev, gdth_sglist(scp), gdth_sg_count(scp),
+        if (scsi_bufflen(scp))
+            pci_unmap_sg(ha->pdev, scsi_sglist(scp), scsi_sg_count(scp),
                          cmndinfo->dma_dir);
 
         if (cmndinfo->sense_paddr)
@@ -4026,10 +3985,6 @@ static int gdth_queuecommand(struct scsi_cmnd *scp,
     gdth_update_timeout(scp, scp->timeout_per_command * 6);
     cmndinfo->priority = DEFAULT_PRI;
 
-    gdth_set_bufflen(scp, scsi_bufflen(scp));
-    gdth_set_sg_count(scp, scsi_sg_count(scp));
-    gdth_set_sglist(scp, scsi_sglist(scp));
-
     return __gdth_queuecommand(ha, scp, cmndinfo);
 }
 
-- 
1.5.3.3

Re: gdth new set of patches for 2.6.24 stable

[Boaz Harrosh]

On Thu, Feb 14 2008 at 20:47 +0200, Boaz Harrosh <bharrosh@panasas.com> wrote:
> Submitted are a new set of patches, that fix lots of problems
> with the gdth driver.
> 
> It fixes the following problems:
> - scan for drives on hosts. (Already in mainline)
> - truly fixes the exit/reboot problems but does call flush() before
>   reboot.
> - fix crash when accessing array with icpcon management application.
> - fix crash when doing $ cat /proc/sys/gdth/0.
>   This one still has the below WARN_ON in messages (see <gdth_info> below)
>   So there is one more thing hiding in there.
> - use pci_get_device
>   One of the testers requested if we can also put the move to pci_get_device 
>   patch with removal of dependency on PCI_LEGACY, to the stable release.
> 
> The patches are for and based on Linux-2.6.24. here is the list of patches:
>   [PATCH 1/5] gdth: update deprecated pci_find_device
>   [PATCH 2/5] gdth: scan for scsi devices
>   [PATCH 3/5] gdth: bugfix for the at-exit problems
>   [PATCH 4/5] gdth: fix to internal commands execution
>   [PATCH 5/5] gdth: remove gdth cooked up command accessors
> 
> Please all test and report your findings.
> 
> Thanks in advance
> Boaz
> 
> ---
> <gdth_info>
>   WARNING: at arch/x86/kernel/pci-dma_32.c:66 dma_free_coherent()
>   Pid: 5501, comm: cat Not tainted 2.6.24 #43
>    [<c0107137>] dma_free_coherent+0x93/0x95
>    [<c025ef73>] gdth_ioctl_free+0x4c/0x69
>    [<c0264a36>] gdth_proc_info+0x165f/0x182c
>    [<c0111f7a>] update_curr+0xeb/0xf2
>    [<c01132aa>] task_rq_lock+0x29/0x50
>    [<c0113706>] try_to_wake_up+0x42/0x342
>    [<c0113706>] try_to_wake_up+0x42/0x342
>    [<c0111a9f>] __wake_up_common+0x46/0x6d
>    [<c0113569>] __wake_up+0x32/0x42
>    [<c022dad9>] n_tty_receive_buf+0x2e8/0xe97
>    [<c022dad9>] n_tty_receive_buf+0x2e8/0xe97
>    [<c0111f0a>] update_curr+0x7b/0xf2
>    [<c0112625>] enqueue_task_fair+0x27/0x30
>    [<c0111783>] enqueue_task+0xa/0x14
>    [<c025e351>] proc_scsi_read+0x29/0x3d
>    [<c025e328>] proc_scsi_read+0x0/0x3d
>    [<c0189704>] proc_file_read+0x1c6/0x279
>    [<c018953e>] proc_file_read+0x0/0x279
>    [<c0185eca>] proc_reg_read+0x53/0x71
>    [<c0185e77>] proc_reg_read+0x0/0x71
>    [<c0159968>] vfs_read+0x85/0x11b
>    [<c0159d9d>] sys_read+0x41/0x6a
>    [<c0102822>] sysenter_past_esp+0x5f/0x85
>  </gdth_info>
> -
James hi.

All my testers have reported back that with these 5 patches applied they can
now run with a 2.6.24 kernel the same way they ran before. However there is
that reported issue, with the dma_free_coherent WARN_ON (above). The code was 
like that from day one and it is a very old issue, however it is a regression 
because 2.6.24 introduced that new WARN_ON.
(infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
>From posts on lkml and even recent one in linux-scsi about the arcmsr driver
it looks that all a driver can do is work around it with different kernel mechanisms
and driver rewrites. I'm afraid I need your help here. I'm not sure I understand
why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and what
is needed to replace it. Could you please have a look in gdth_proc.c and also in
gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and advise
what can I do in it's place. Please bear in mind that we need it for 2.6.24, as
a bugfix.

Apart from the above issue, please accept patches 3,4,5 above they have now
been tested and are reported to bring broken system back to production.
(Given that you approve off course). And mark them for inclusion to the
2.6.24 stable releases. (Or is there some thing that I should do)

---
Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
pose any harm. Some people have reported stability with temporarily disabling
it. For testers that want to try, here it is below. At your own risk.

---
>From 50d3657bf6a138ee63ad1ce00052380edc75ace7 Mon Sep 17 00:00:00 2001
From: Boaz Harrosh <bharrosh@panasas.com>
Date: Sun, 17 Feb 2008 12:49:35 +0200
Subject: [PATCH] gdth: Hack to remove WARN_ON in arch/x86/kernel/pci-dma_32.c

  gdth uses dma_free_coherent() with interrupts disabled. Which
  is not portable, but is safe on the HW that supports gdth.

NOT Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 arch/x86/kernel/pci-dma_32.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/pci-dma_32.c b/arch/x86/kernel/pci-dma_32.c
index 5133032..350dcfd 100644
--- a/arch/x86/kernel/pci-dma_32.c
+++ b/arch/x86/kernel/pci-dma_32.c
@@ -63,7 +63,7 @@ void dma_free_coherent(struct device *dev, size_t size,
 	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
 	int order = get_order(size);
 
-	WARN_ON(irqs_disabled());	/* for portability */
+/*	WARN_ON(irqs_disabled());*/	/* for portability */
 	if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + (mem->size << PAGE_SHIFT))) {
 		int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
 
-- 
1.5.3.3
 
 

Re: gdth new set of patches for 2.6.24 stable

[James Bottomley]

On Sun, 2008-02-17 at 18:46 +0200, Boaz Harrosh wrote:
> On Thu, Feb 14 2008 at 20:47 +0200, Boaz Harrosh <bharrosh@panasas.com> wrote:
> > Submitted are a new set of patches, that fix lots of problems
> > with the gdth driver.
> > 
> > It fixes the following problems:
> > - scan for drives on hosts. (Already in mainline)
> > - truly fixes the exit/reboot problems but does call flush() before
> >   reboot.
> > - fix crash when accessing array with icpcon management application.
> > - fix crash when doing $ cat /proc/sys/gdth/0.
> >   This one still has the below WARN_ON in messages (see <gdth_info> below)
> >   So there is one more thing hiding in there.
> > - use pci_get_device
> >   One of the testers requested if we can also put the move to pci_get_device 
> >   patch with removal of dependency on PCI_LEGACY, to the stable release.
> > 
> > The patches are for and based on Linux-2.6.24. here is the list of patches:
> >   [PATCH 1/5] gdth: update deprecated pci_find_device
> >   [PATCH 2/5] gdth: scan for scsi devices
> >   [PATCH 3/5] gdth: bugfix for the at-exit problems
> >   [PATCH 4/5] gdth: fix to internal commands execution
> >   [PATCH 5/5] gdth: remove gdth cooked up command accessors
> > 
> > Please all test and report your findings.
> > 
> > Thanks in advance
> > Boaz
> > 
> > ---
> > <gdth_info>
> >   WARNING: at arch/x86/kernel/pci-dma_32.c:66 dma_free_coherent()
> >   Pid: 5501, comm: cat Not tainted 2.6.24 #43
> >    [<c0107137>] dma_free_coherent+0x93/0x95
> >    [<c025ef73>] gdth_ioctl_free+0x4c/0x69
> >    [<c0264a36>] gdth_proc_info+0x165f/0x182c
> >    [<c0111f7a>] update_curr+0xeb/0xf2
> >    [<c01132aa>] task_rq_lock+0x29/0x50
> >    [<c0113706>] try_to_wake_up+0x42/0x342
> >    [<c0113706>] try_to_wake_up+0x42/0x342
> >    [<c0111a9f>] __wake_up_common+0x46/0x6d
> >    [<c0113569>] __wake_up+0x32/0x42
> >    [<c022dad9>] n_tty_receive_buf+0x2e8/0xe97
> >    [<c022dad9>] n_tty_receive_buf+0x2e8/0xe97
> >    [<c0111f0a>] update_curr+0x7b/0xf2
> >    [<c0112625>] enqueue_task_fair+0x27/0x30
> >    [<c0111783>] enqueue_task+0xa/0x14
> >    [<c025e351>] proc_scsi_read+0x29/0x3d
> >    [<c025e328>] proc_scsi_read+0x0/0x3d
> >    [<c0189704>] proc_file_read+0x1c6/0x279
> >    [<c018953e>] proc_file_read+0x0/0x279
> >    [<c0185eca>] proc_reg_read+0x53/0x71
> >    [<c0185e77>] proc_reg_read+0x0/0x71
> >    [<c0159968>] vfs_read+0x85/0x11b
> >    [<c0159d9d>] sys_read+0x41/0x6a
> >    [<c0102822>] sysenter_past_esp+0x5f/0x85
> >  </gdth_info>
> > -
> James hi.
> 
> All my testers have reported back that with these 5 patches applied they can
> now run with a 2.6.24 kernel the same way they ran before. However there is
> that reported issue, with the dma_free_coherent WARN_ON (above). The code was 
> like that from day one and it is a very old issue, however it is a regression 
> because 2.6.24 introduced that new WARN_ON.
> (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
> >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
> it looks that all a driver can do is work around it with different kernel mechanisms
> and driver rewrites. I'm afraid I need your help here. I'm not sure I understand
> why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and what
> is needed to replace it. Could you please have a look in gdth_proc.c and also in
> gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and advise
> what can I do in it's place. Please bear in mind that we need it for 2.6.24, as
> a bugfix.
> 
> Apart from the above issue, please accept patches 3,4,5 above they have now
> been tested and are reported to bring broken system back to production.
> (Given that you approve off course). And mark them for inclusion to the
> 2.6.24 stable releases. (Or is there some thing that I should do)
> 
> ---
> Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
> pose any harm. Some people have reported stability with temporarily disabling
> it. For testers that want to try, here it is below. At your own risk.

Isn't this the correct fix?  pscratch is a permanent address (it's
allocated at boot time and never changes).  All you need the smp lock
for is mediating the scratch in use flag.

James

diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c
index de57734..ce0228e 100644
--- a/drivers/scsi/gdth_proc.c
+++ b/drivers/scsi/gdth_proc.c
@@ -694,15 +694,13 @@ static void gdth_ioctl_free(gdth_ha_str *ha, int size, char *buf, ulong64 paddr)
 {
     ulong flags;
 
-    spin_lock_irqsave(&ha->smp_lock, flags);
-
     if (buf == ha->pscratch) {
+	spin_lock_irqsave(&ha->smp_lock, flags);
         ha->scratch_busy = FALSE;
+	spin_unlock_irqrestore(&ha->smp_lock, flags);
     } else {
         pci_free_consistent(ha->pdev, size, buf, paddr);
     }
-
-    spin_unlock_irqrestore(&ha->smp_lock, flags);
 }
 
 #ifdef GDTH_IOCTL_PROC

Re: gdth new set of patches for 2.6.24 stable

[Boaz Harrosh]

On Sun, Feb 17 2008 at 19:24 +0200, James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> On Sun, 2008-02-17 at 18:46 +0200, Boaz Harrosh wrote:
>> On Thu, Feb 14 2008 at 20:47 +0200, Boaz Harrosh <bharrosh@panasas.com> wrote:
>>> Submitted are a new set of patches, that fix lots of problems
>>> with the gdth driver.
>>>
>>> It fixes the following problems:
>>> - scan for drives on hosts. (Already in mainline)
>>> - truly fixes the exit/reboot problems but does call flush() before
>>>   reboot.
>>> - fix crash when accessing array with icpcon management application.
>>> - fix crash when doing $ cat /proc/sys/gdth/0.
>>>   This one still has the below WARN_ON in messages (see <gdth_info> below)
>>>   So there is one more thing hiding in there.
>>> - use pci_get_device
>>>   One of the testers requested if we can also put the move to pci_get_device 
>>>   patch with removal of dependency on PCI_LEGACY, to the stable release.
>>>
>>> The patches are for and based on Linux-2.6.24. here is the list of patches:
>>>   [PATCH 1/5] gdth: update deprecated pci_find_device
>>>   [PATCH 2/5] gdth: scan for scsi devices
>>>   [PATCH 3/5] gdth: bugfix for the at-exit problems
>>>   [PATCH 4/5] gdth: fix to internal commands execution
>>>   [PATCH 5/5] gdth: remove gdth cooked up command accessors
>>>
>>> Please all test and report your findings.
>>>
>>> Thanks in advance
>>> Boaz
>>>
>>> ---
>>> <gdth_info>
>>>   WARNING: at arch/x86/kernel/pci-dma_32.c:66 dma_free_coherent()
>>>   Pid: 5501, comm: cat Not tainted 2.6.24 #43
>>>    [<c0107137>] dma_free_coherent+0x93/0x95
>>>    [<c025ef73>] gdth_ioctl_free+0x4c/0x69
>>>    [<c0264a36>] gdth_proc_info+0x165f/0x182c
>>>    [<c0111f7a>] update_curr+0xeb/0xf2
>>>    [<c01132aa>] task_rq_lock+0x29/0x50
>>>    [<c0113706>] try_to_wake_up+0x42/0x342
>>>    [<c0113706>] try_to_wake_up+0x42/0x342
>>>    [<c0111a9f>] __wake_up_common+0x46/0x6d
>>>    [<c0113569>] __wake_up+0x32/0x42
>>>    [<c022dad9>] n_tty_receive_buf+0x2e8/0xe97
>>>    [<c022dad9>] n_tty_receive_buf+0x2e8/0xe97
>>>    [<c0111f0a>] update_curr+0x7b/0xf2
>>>    [<c0112625>] enqueue_task_fair+0x27/0x30
>>>    [<c0111783>] enqueue_task+0xa/0x14
>>>    [<c025e351>] proc_scsi_read+0x29/0x3d
>>>    [<c025e328>] proc_scsi_read+0x0/0x3d
>>>    [<c0189704>] proc_file_read+0x1c6/0x279
>>>    [<c018953e>] proc_file_read+0x0/0x279
>>>    [<c0185eca>] proc_reg_read+0x53/0x71
>>>    [<c0185e77>] proc_reg_read+0x0/0x71
>>>    [<c0159968>] vfs_read+0x85/0x11b
>>>    [<c0159d9d>] sys_read+0x41/0x6a
>>>    [<c0102822>] sysenter_past_esp+0x5f/0x85
>>>  </gdth_info>
>>> -
>> James hi.
>>
>> All my testers have reported back that with these 5 patches applied they can
>> now run with a 2.6.24 kernel the same way they ran before. However there is
>> that reported issue, with the dma_free_coherent WARN_ON (above). The code was 
>> like that from day one and it is a very old issue, however it is a regression 
>> because 2.6.24 introduced that new WARN_ON.
>> (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
>> >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
>> it looks that all a driver can do is work around it with different kernel mechanisms
>> and driver rewrites. I'm afraid I need your help here. I'm not sure I understand
>> why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and what
>> is needed to replace it. Could you please have a look in gdth_proc.c and also in
>> gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and advise
>> what can I do in it's place. Please bear in mind that we need it for 2.6.24, as
>> a bugfix.
>>
>> Apart from the above issue, please accept patches 3,4,5 above they have now
>> been tested and are reported to bring broken system back to production.
>> (Given that you approve off course). And mark them for inclusion to the
>> 2.6.24 stable releases. (Or is there some thing that I should do)
>>
>> ---
>> Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
>> pose any harm. Some people have reported stability with temporarily disabling
>> it. For testers that want to try, here it is below. At your own risk.
> 
> Isn't this the correct fix?  pscratch is a permanent address (it's
> allocated at boot time and never changes).  All you need the smp lock
> for is mediating the scratch in use flag.
> 
> James
> 
> diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c
> index de57734..ce0228e 100644
> --- a/drivers/scsi/gdth_proc.c
> +++ b/drivers/scsi/gdth_proc.c
> @@ -694,15 +694,13 @@ static void gdth_ioctl_free(gdth_ha_str *ha, int size, char *buf, ulong64 paddr)
>  {
>      ulong flags;
>  
> -    spin_lock_irqsave(&ha->smp_lock, flags);
> -
>      if (buf == ha->pscratch) {
> +	spin_lock_irqsave(&ha->smp_lock, flags);
>          ha->scratch_busy = FALSE;
> +	spin_unlock_irqrestore(&ha->smp_lock, flags);
>      } else {
>          pci_free_consistent(ha->pdev, size, buf, paddr);
>      }
> -
> -    spin_unlock_irqrestore(&ha->smp_lock, flags);
>  }
>  
>  #ifdef GDTH_IOCTL_PROC
> 
> 
> -

James
You are bung on the money. It was tested and it works. So simple, I was 
thinking it was accessed by DMA and freed at interrupt. But no, just a 
simple lock like this.

So that's it then, all reported problems with gdth are now resolved. Please
Submit above together with the other patches.

Do I need to do anything else to get it into 2.6.24.x stable releases?

Thanks for everything
Boaz

Re: gdth new set of patches for 2.6.24 stable

[Andrew Morton]

On Sun, 17 Feb 2008 18:46:03 +0200 Boaz Harrosh <bharrosh@panasas.com> wrote:

> 
> ...
>
> All my testers have reported back that with these 5 patches applied they can
> now run with a 2.6.24 kernel the same way they ran before. However there is
> that reported issue, with the dma_free_coherent WARN_ON (above). The code was 
> like that from day one and it is a very old issue, however it is a regression 
> because 2.6.24 introduced that new WARN_ON.
> (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
> >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
> it looks that all a driver can do is work around it with different kernel mechanisms
> and driver rewrites. I'm afraid I need your help here. I'm not sure I understand
> why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and what
> is needed to replace it. Could you please have a look in gdth_proc.c and also in
> gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and advise
> what can I do in it's place. Please bear in mind that we need it for 2.6.24, as
> a bugfix.
> 
> Apart from the above issue, please accept patches 3,4,5 above they have now
> been tested and are reported to bring broken system back to production.
> (Given that you approve off course). And mark them for inclusion to the
> 2.6.24 stable releases. (Or is there some thing that I should do)
> 
> ---
> Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
> pose any harm. Some people have reported stability with temporarily disabling
> it. For testers that want to try, here it is below. At your own risk.
> 
> ---
> >From 50d3657bf6a138ee63ad1ce00052380edc75ace7 Mon Sep 17 00:00:00 2001
> From: Boaz Harrosh <bharrosh@panasas.com>
> Date: Sun, 17 Feb 2008 12:49:35 +0200
> Subject: [PATCH] gdth: Hack to remove WARN_ON in arch/x86/kernel/pci-dma_32.c
> 
>   gdth uses dma_free_coherent() with interrupts disabled. Which
>   is not portable, but is safe on the HW that supports gdth.
> 
> NOT Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
>  arch/x86/kernel/pci-dma_32.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/pci-dma_32.c b/arch/x86/kernel/pci-dma_32.c
> index 5133032..350dcfd 100644
> --- a/arch/x86/kernel/pci-dma_32.c
> +++ b/arch/x86/kernel/pci-dma_32.c
> @@ -63,7 +63,7 @@ void dma_free_coherent(struct device *dev, size_t size,
>  	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
>  	int order = get_order(size);
>  
> -	WARN_ON(irqs_disabled());	/* for portability */
> +/*	WARN_ON(irqs_disabled());*/	/* for portability */
>  	if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + (mem->size << PAGE_SHIFT))) {
>  		int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
>  

Yes.   Let's reprise aa24886e379d2b641c5117e178b15ce1d5d366ba:

: commit aa24886e379d2b641c5117e178b15ce1d5d366ba
: Author: David Brownell <david-b@pacbell.net>
: Date:   Fri Aug 10 13:10:27 2007 -0700
: 
:     dma_free_coherent() needs irqs enabled (sigh)
:     
:     On at least ARM (and I'm told MIPS too) dma_free_coherent() has a newish
:     call context requirement: unlike its dma_alloc_coherent() sibling, it may
:     not be called with IRQs disabled.  (This was new behavior on ARM as of late
:     2005, caused by ARM SMP updates.) This little surprise can be annoyingly
:     driver-visible.
:     
:     Since it looks like that restriction won't be removed, this patch changes
:     the definition of the API to include that requirement.  Also, to help catch
:     nonportable drivers, it updates the x86 and swiotlb versions to include the
:     relevant warnings.  (I already observed that it trips on the
:     bus_reset_tasklet of the new firewire_ohci driver.)
: 

In general, all Linux memory-freeing functions can be called from all
contexts.  (vfree is an irritating exception).  This is good, and provides
maximum usefulness to callees, as all utility functions should seek to do. 
It would be best to fix arm and mips.

But arm and mips require enabled local irqs because their
dma_free_coherent() needs to do a cross-cpu IPI call.  Presumably because
of certain unusual TLB protocols.

I'm not sure what we should do about this.  Presumably the gdth-on-arm
usage base is, umm, zero, so we could lamely add
CONFIG_DMA_FREE_COHERENT_WITH_LOCAL_IRQS_DISABLED_IS_OK and then use that
to disable gdth (and similar) on arm amd mips.  But ugh.

Russell, Ralf: is there something we can do here to relax this requirement?

I'm thinking that perhaps we can do some rcu/refcounting tricks: launch the
IPI from within dma_free_coherent(), but don't wait for it to complete. 
When all CPUs have handled the IPI then (and only then) the virtual address
becomes recyclable, or something like that?

<double-checks>

Actually I think David might have been wrong about mips.  afaict its
dma_free_coherent() is callable under local_irq_disable(), so ARM SMP is
the sole exception?  

Re: gdth new set of patches for 2.6.24 stable

[Boaz Harrosh]

On Mon, Feb 18 2008 at 14:57 +0200, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Sun, 17 Feb 2008 18:46:03 +0200 Boaz Harrosh <bharrosh@panasas.com> wrote:
> 
>> ...
>>
>> All my testers have reported back that with these 5 patches applied they can
>> now run with a 2.6.24 kernel the same way they ran before. However there is
>> that reported issue, with the dma_free_coherent WARN_ON (above). The code was 
>> like that from day one and it is a very old issue, however it is a regression 
>> because 2.6.24 introduced that new WARN_ON.
>> (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
>> >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
>> it looks that all a driver can do is work around it with different kernel mechanisms
>> and driver rewrites. I'm afraid I need your help here. I'm not sure I understand
>> why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and what
>> is needed to replace it. Could you please have a look in gdth_proc.c and also in
>> gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and advise
>> what can I do in it's place. Please bear in mind that we need it for 2.6.24, as
>> a bugfix.
>>
>> Apart from the above issue, please accept patches 3,4,5 above they have now
>> been tested and are reported to bring broken system back to production.
>> (Given that you approve off course). And mark them for inclusion to the
>> 2.6.24 stable releases. (Or is there some thing that I should do)
>>
>> ---
>> Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
>> pose any harm. Some people have reported stability with temporarily disabling
>> it. For testers that want to try, here it is below. At your own risk.
>>
>> ---
>> >From 50d3657bf6a138ee63ad1ce00052380edc75ace7 Mon Sep 17 00:00:00 2001
>> From: Boaz Harrosh <bharrosh@panasas.com>
>> Date: Sun, 17 Feb 2008 12:49:35 +0200
>> Subject: [PATCH] gdth: Hack to remove WARN_ON in arch/x86/kernel/pci-dma_32.c
>>
>>   gdth uses dma_free_coherent() with interrupts disabled. Which
>>   is not portable, but is safe on the HW that supports gdth.
>>
>> NOT Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
>> ---
>>  arch/x86/kernel/pci-dma_32.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/kernel/pci-dma_32.c b/arch/x86/kernel/pci-dma_32.c
>> index 5133032..350dcfd 100644
>> --- a/arch/x86/kernel/pci-dma_32.c
>> +++ b/arch/x86/kernel/pci-dma_32.c
>> @@ -63,7 +63,7 @@ void dma_free_coherent(struct device *dev, size_t size,
>>  	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
>>  	int order = get_order(size);
>>  
>> -	WARN_ON(irqs_disabled());	/* for portability */
>> +/*	WARN_ON(irqs_disabled());*/	/* for portability */
>>  	if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + (mem->size << PAGE_SHIFT))) {
>>  		int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
>>  
> 
> Yes.   Let's reprise aa24886e379d2b641c5117e178b15ce1d5d366ba:
> 
> : commit aa24886e379d2b641c5117e178b15ce1d5d366ba
> : Author: David Brownell <david-b@pacbell.net>
> : Date:   Fri Aug 10 13:10:27 2007 -0700
> : 
> :     dma_free_coherent() needs irqs enabled (sigh)
> :     
> :     On at least ARM (and I'm told MIPS too) dma_free_coherent() has a newish
> :     call context requirement: unlike its dma_alloc_coherent() sibling, it may
> :     not be called with IRQs disabled.  (This was new behavior on ARM as of late
> :     2005, caused by ARM SMP updates.) This little surprise can be annoyingly
> :     driver-visible.
> :     
> :     Since it looks like that restriction won't be removed, this patch changes
> :     the definition of the API to include that requirement.  Also, to help catch
> :     nonportable drivers, it updates the x86 and swiotlb versions to include the
> :     relevant warnings.  (I already observed that it trips on the
> :     bus_reset_tasklet of the new firewire_ohci driver.)
> : 
> 
> In general, all Linux memory-freeing functions can be called from all
> contexts.  (vfree is an irritating exception).  This is good, and provides
> maximum usefulness to callees, as all utility functions should seek to do. 
> It would be best to fix arm and mips.
> 
> But arm and mips require enabled local irqs because their
> dma_free_coherent() needs to do a cross-cpu IPI call.  Presumably because
> of certain unusual TLB protocols.
> 
> I'm not sure what we should do about this.  Presumably the gdth-on-arm
> usage base is, umm, zero, so we could lamely add
> CONFIG_DMA_FREE_COHERENT_WITH_LOCAL_IRQS_DISABLED_IS_OK and then use that
> to disable gdth (and similar) on arm amd mips.  But ugh.
> 
> Russell, Ralf: is there something we can do here to relax this requirement?
> 
> I'm thinking that perhaps we can do some rcu/refcounting tricks: launch the
> IPI from within dma_free_coherent(), but don't wait for it to complete. 
> When all CPUs have handled the IPI then (and only then) the virtual address
> becomes recyclable, or something like that?
> 
> <double-checks>
> 
> Actually I think David might have been wrong about mips.  afaict its
> dma_free_coherent() is callable under local_irq_disable(), so ARM SMP is
> the sole exception?  
> 
> 

Thank you Andrew for your reply.

But actually James has resolved this issue for the gdth driver. And all testers
that witnessed a problem have reported success with his simple fix, which is for
sure a much more correct way then was done before.

So for gdth sake I is OK as it is, so far. 

Boaz

Re: gdth new set of patches for 2.6.24 stable

[Russell King]

On Mon, Feb 18, 2008 at 04:57:36AM -0800, Andrew Morton wrote:
> But arm and mips require enabled local irqs because their
> dma_free_coherent() needs to do a cross-cpu IPI call.  Presumably because
> of certain unusual TLB protocols.

Consider that TLB flushing needs to call a function on another CPU.
Now look at x86's implementation to do cross calls:

/**
 * smp_call_function_mask(): Run a function on a set of other CPUs.
 * @mask: The set of cpus to run on.  Must not include the current cpu.
 * @func: The function to run. This must be fast and non-blocking.
 * @info: An arbitrary pointer to pass to the function.
 * @wait: If true, wait (atomically) until function has completed on other CPUs. *
 * Returns 0 on success, else a negative status code.
 *
 * If @wait is true, then returns once @func has returned; otherwise
 * it returns just before the target cpu calls @func.
 *
 * You must not call this function with disabled interrupts or from a
 * hardware interrupt handler or from a bottom half handler.
 */
static int
native_smp_call_function_mask(cpumask_t mask,
                              void (*func)(void *), void *info,
                              int wait)

You can not call functions on other CPUs without having IRQs enabled,
otherwise this functionality deadlocks.  That restriction is on all
smp_call_function() implementations.

Unfortunately, to flush the TLB on other CPUs on ARM, we need to do a
cross call, which means using smp_call_function(), which introduces the
same sodding restrictions that smp_call_function() has on the functions
which call it.

> Russell, Ralf: is there something we can do here to relax this requirement?

Do what x86 people have so far been unable to resolve and find some
way to allow smp_call_function() to operate with IRQs disabled without
deadlocking?

Another solution jejb suggested was to make dma_free_coherent() lazy,
but (a) I'm unconvinced that this'll work with drivers which constantly
alloc+free in IRQ context since there's generally only 2MB of VM space
for such mappings, and it probably won't take long to eat through that
limited space with such a scheme.

Also, I don't have the facility to really test out these issues...

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: gdth new set of patches for 2.6.24 stable

[James Bottomley]

On Mon, 2008-02-18 at 14:02 +0000, Russell King wrote:
> Another solution jejb suggested was to make dma_free_coherent() lazy,
> but (a) I'm unconvinced that this'll work with drivers which
> constantly
> alloc+free in IRQ context since there's generally only 2MB of VM space
> for such mappings, and it probably won't take long to eat through that
> limited space with such a scheme.

I didn't say make the alloc/free lazy, I said make the mapping setup
teardown lazy.  So on alloc, you first look for existing space, if none,
you map some more.  On free you *don't* teardown the mappings (which is
what requires the IPI) but simply free to the existing space pool ready
for reuse.  You map and free in page size chunks, since the coherent
memory users that want > PAGE_SIZE usually all do boot time allocation
(and don't free until release time).

The reuse of the pages should keep even the drivers that do persistent
alloc/free of dma coherent memory happy, and it should reach a steady
state fairly quickly.

James

Re: gdth new set of patches for 2.6.24 stable

[Ralf Baechle]

On Mon, Feb 18, 2008 at 04:57:36AM -0800, Andrew Morton wrote:

> On Sun, 17 Feb 2008 18:46:03 +0200 Boaz Harrosh <bharrosh@panasas.com> wrote:
> 
> > ...
> >
> > All my testers have reported back that with these 5 patches applied they can
> > now run with a 2.6.24 kernel the same way they ran before. However there is
> > that reported issue, with the dma_free_coherent WARN_ON (above). The code was 
> > like that from day one and it is a very old issue, however it is a regression 
> > because 2.6.24 introduced that new WARN_ON.
> > (infamous commit aa24886e379d2b641c5117e178b15ce1d5d366ba)
> > >From posts on lkml and even recent one in linux-scsi about the arcmsr driver
> > it looks that all a driver can do is work around it with different kernel mechanisms
> > and driver rewrites. I'm afraid I need your help here. I'm not sure I understand
> > why does the gdth driver uses the pci_{alloc,free}_consistent() API's, and what
> > is needed to replace it. Could you please have a look in gdth_proc.c and also in
> > gdth.c for all the places that call gdth_ioctl_alloc/gdth_ioctl_free, and advise
> > what can I do in it's place. Please bear in mind that we need it for 2.6.24, as
> > a bugfix.
> > 
> > Apart from the above issue, please accept patches 3,4,5 above they have now
> > been tested and are reported to bring broken system back to production.
> > (Given that you approve off course). And mark them for inclusion to the
> > 2.6.24 stable releases. (Or is there some thing that I should do)
> > 
> > ---
> > Meanwhile on x86 systems I understand the WARN_ON is cosmetic, and does not
> > pose any harm. Some people have reported stability with temporarily disabling
> > it. For testers that want to try, here it is below. At your own risk.
> > 
> > ---
> > >From 50d3657bf6a138ee63ad1ce00052380edc75ace7 Mon Sep 17 00:00:00 2001
> > From: Boaz Harrosh <bharrosh@panasas.com>
> > Date: Sun, 17 Feb 2008 12:49:35 +0200
> > Subject: [PATCH] gdth: Hack to remove WARN_ON in arch/x86/kernel/pci-dma_32.c
> > 
> >   gdth uses dma_free_coherent() with interrupts disabled. Which
> >   is not portable, but is safe on the HW that supports gdth.
> > 
> > NOT Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> > ---
> >  arch/x86/kernel/pci-dma_32.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/pci-dma_32.c b/arch/x86/kernel/pci-dma_32.c
> > index 5133032..350dcfd 100644
> > --- a/arch/x86/kernel/pci-dma_32.c
> > +++ b/arch/x86/kernel/pci-dma_32.c
> > @@ -63,7 +63,7 @@ void dma_free_coherent(struct device *dev, size_t size,
> >  	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
> >  	int order = get_order(size);
> >  
> > -	WARN_ON(irqs_disabled());	/* for portability */
> > +/*	WARN_ON(irqs_disabled());*/	/* for portability */
> >  	if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + (mem->size << PAGE_SHIFT))) {
> >  		int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
> >  
> 
> Yes.   Let's reprise aa24886e379d2b641c5117e178b15ce1d5d366ba:
> 
> : commit aa24886e379d2b641c5117e178b15ce1d5d366ba
> : Author: David Brownell <david-b@pacbell.net>
> : Date:   Fri Aug 10 13:10:27 2007 -0700
> : 
> :     dma_free_coherent() needs irqs enabled (sigh)
> :     
> :     On at least ARM (and I'm told MIPS too) dma_free_coherent() has a newish
> :     call context requirement: unlike its dma_alloc_coherent() sibling, it may
> :     not be called with IRQs disabled.  (This was new behavior on ARM as of late
> :     2005, caused by ARM SMP updates.) This little surprise can be annoyingly
> :     driver-visible.
> :     
> :     Since it looks like that restriction won't be removed, this patch changes
> :     the definition of the API to include that requirement.  Also, to help catch
> :     nonportable drivers, it updates the x86 and swiotlb versions to include the
> :     relevant warnings.  (I already observed that it trips on the
> :     bus_reset_tasklet of the new firewire_ohci driver.)
> : 
> 
> In general, all Linux memory-freeing functions can be called from all
> contexts.  (vfree is an irritating exception).  This is good, and provides
> maximum usefulness to callees, as all utility functions should seek to do. 
> It would be best to fix arm and mips.
> 
> But arm and mips require enabled local irqs because their
> dma_free_coherent() needs to do a cross-cpu IPI call.  Presumably because
> of certain unusual TLB protocols.
> 
> I'm not sure what we should do about this.  Presumably the gdth-on-arm
> usage base is, umm, zero, so we could lamely add
> CONFIG_DMA_FREE_COHERENT_WITH_LOCAL_IRQS_DISABLED_IS_OK and then use that
> to disable gdth (and similar) on arm amd mips.  But ugh.
> 
> Russell, Ralf: is there something we can do here to relax this requirement?

The current MIPS implementation of dma_alloc_coherent / dma_free_coherent
is a glorified wrapper around __get_free_pages rsp. free_pages that is
it doesn't depend on interrupts enabled.

This works because the current dma_alloc_coherent will only return
lowmem which is accessible without TLB mappings through KSEG0/1.

The embedded world being as quirky as it is I expect support of highmem
which will require the use of TLB entries to become eventually relevant
on MIPS but I think the lazy teardown of the mappings can take care of
that and worst case a caller of dma_alloc_coherent() has to be prepared
to handle an error return anyway.

  Ralf

Re: gdth new set of patches for 2.6.24 stable

[David Brownell]

On Monday 18 February 2008, Andrew Morton wrote:
> Russell, Ralf: is there something we can do here to relax this requirement?
> 
> I'm thinking that perhaps we can do some rcu/refcounting tricks: launch the
> IPI from within dma_free_coherent(), but don't wait for it to complete. 
> When all CPUs have handled the IPI then (and only then) the virtual address
> becomes recyclable, or something like that?

Or the trick some drivers had to do:  just defer the actual work of
dma_free_coherent() into a tasklet.  Better have one such tasklet in
arch code than N of them in drivers.

To be clear:  I never thought that API restriction was a good idea.

(Although this discussion now seems moot for the gdth driver.)


> <double-checks>
> 
> Actually I think David might have been wrong about mips.  afaict its
> dma_free_coherent() is callable under local_irq_disable(), so ARM SMP is
> the sole exception?  

All I recall at this point was getting some arch-specific patches in that
area; I thought it was MIPS, maybe it was PPC.  The arch code may have
changed since then too.

- Dave
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: gdth new set of patches for 2.6.24 stable

[Ralf Baechle]

On Mon, Feb 18, 2008 at 12:27:42PM -0800, David Brownell wrote:

> > Actually I think David might have been wrong about mips.  afaict its
> > dma_free_coherent() is callable under local_irq_disable(), so ARM SMP is
> > the sole exception?  
> 
> All I recall at this point was getting some arch-specific patches in that
> area; I thought it was MIPS, maybe it was PPC.  The arch code may have
> changed since then too.

Mostly to avoid code duplication on the MIPS side but the core of the
implementation has always been similar to what it is currently.

  Ralf
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html