sym53c8xx timer and smp fixes
PATCH 1/1 sym53c8xx timer and smp fixes
All,
Attached is a patch for sym53c8xx.c to handle the error timer better, and be more proper for SMP. The changes are very simple, and have been beaten on by us. Please let me know if there are any problems accepting this patch for general inclusion.
Tim
Signature · 4 lines
Tim Hockin Systems Software Engineer Sun Microsystems, Cobalt Server Appliances [email protected]
drivers/scsi/sym53c8xx.c +28 -37
@@ -634,8 +636,11 @@#if LINUX_VERSION_CODE >=LinuxVersionCode (2 ,1 ,93 )spinlock_t sym53c8xx_lock =SPIN_LOCK_UNLOCKED ; +spinlock_t sym53c8xx_host_lock =SPIN_LOCK_UNLOCKED ;#define NCR_LOCK_DRIVER (flags) spin_lock_irqsave(&sym53c8xx_lock, flags)#define NCR_UNLOCK_DRIVER (flags) spin_unlock_irqrestore(&sym53c8xx_lock,flags) +#define NCR_LOCK_HOSTS (flags) spin_lock_irqsave(&sym53c8xx_host_lock, flags) +#define NCR_UNLOCK_HOSTS (flags) spin_unlock_irqrestore(&sym53c8xx_host_lock,flags)#define NCR_INIT_LOCK_NCB (np) spin_lock_init(&np->smp_lock);#define NCR_LOCK_NCB (np, flags) spin_lock_irqsave(&np->smp_lock, flags)
@@ -650,6 +655,8 @@#define NCR_LOCK_DRIVER (flags) do { save_flags(flags); cli(); } while (0)#define NCR_UNLOCK_DRIVER (flags) do { restore_flags(flags); } while (0) +#define NCR_LOCK_HOSTS (flags) do { save_flags(flags); cli(); } while (0) +#define NCR_UNLOCK_HOSTS (flags) do { restore_flags(flags); } while (0)#define NCR_INIT_LOCK_NCB (np) do { } while (0)#define NCR_LOCK_NCB (np, flags) do { save_flags(flags); cli(); } while (0)
@@ -695,7 +702,7 @@return page_remapped? (page_remapped + page_offs) :0UL ; } -static void __init unmap_pci_mem (u_long vaddr,u_long size) +static void unmap_pci_mem (u_long vaddr,u_long size) {if (vaddr)iounmap ((void *) (vaddr &PAGE_MASK ));
@@ -2249,7 +2265,6 @@**---------------------------------------------------------------- */ struct usrcmd user;/* Command from user */ -volatile u_char release_stage;/* Synchronisation stage on release */ /*---------------------------------------------------------------- ** Fields that are used (primarily) for integrity check
@@ -5868,7 +5883,12 @@** start the timeout daemon */ np->lasttime=0 ; -ncr_timeout (np); +#ifdef SCSI_NCR_PCIQ_BROKEN_INTR + np->timer.expires =ktime_get ((HZ +9 )/10 ); +#else + np->timer.expires =ktime_get (SCSI_NCR_TIMER_INTERVAL ); +#endif +add_timer (&np->timer);/* ** use SIMPLE TAG messages by default
@@ -7227,23 +7247,19 @@**========================================================== */ -#ifdef MODULE static int ncr_detach (ncb_p np) { -int i; +unsigned long flags;printk ("%s: detaching ...\n" ,ncr_name (np));/* -** Stop the ncr_timeout process -** Set release_stage to 1 and wait that ncr_timeout() set it to 2. +** Stop the ncr_timeout process - lock it to ensure no timer is running +** on a different CPU, or anything */ - np->release_stage =1 ; -for (i =50 ; i && np->release_stage !=2 ; i--)MDELAY (100 ); -if (np->release_stage !=2 ) -printk ("%s: the timer seems to be already stopped\n" , -ncr_name (np)); -else np->release_stage =2 ; +NCR_LOCK_NCB (np, flags); +del_timer (&np->timer); +NCR_UNLOCK_NCB (np, flags);/* ** Reset NCR chip.
@@ -7273,7 +7289,6 @@return 1 ; } -#endif /*========================================================== **
@@ -8600,23 +8615,11 @@ {u_long thistime =ktime_get (0 ); -/* -** If release process in progress, let's go -** Set the release stage from 1 to 2 to synchronize -** with the release process. -*/ - -if (np->release_stage) { -if (np->release_stage ==1 ) np->release_stage =2 ; -return ; - } -#ifdef SCSI_NCR_PCIQ_BROKEN_INTR - np->timer.expires =ktime_get ((HZ +9 )/10 ); +mod_timer (&np->timer,ktime_get ((HZ +9 )/10 ));#else - np->timer.expires =ktime_get (SCSI_NCR_TIMER_INTERVAL ); +mod_timer (&np->timer,ktime_get (SCSI_NCR_TIMER_INTERVAL ));#endif -add_timer (&np->timer);/* ** If we are resetting the ncr, wait for settle_time before
@@ -13071,7 +13075,7 @@ (int) (PciDeviceFn(pdev) & 7));#ifdef SCSI_NCR_DYNAMIC_DMA_MAPPING -if (pci_set_dma_mask (pdev, (dma_addr_t) (0xffffffffUL ))) { +if (!pci_dma_supported (pdev, (dma_addr_t) (0xffffffffUL ))) { printk(KERN_WARNING NAME53C8XX "32 BIT PCI BUS DMA ADDRESSING NOT SUPPORTED\n");return -1 ;
@@ -14181,13 +14215,15 @@ {struct Scsi_Host *host;struct host_data *host_data; +unsigned long flags;ncb_p ncb =0 ; -int retv; +int retv = -EINVAL ;#ifdef DEBUG_PROC_INFO printk ("sym53c8xx_proc_info: hostno=%d, func=%d\n" , hostno, func);#endif +NCR_LOCK_HOSTS (flags);for (host = first_host; host; host = host->next) {if (host->hostt != first_host->hostt)continue ;
@@ -14199,25 +14235,22 @@ }if (!ncb) -return -EINVAL ; + goto proc_done;if (func) {#ifdef SCSI_NCR_USER_COMMAND_SUPPORT retv =ncr_user_command (ncb, buffer, length); -#else - retv = -EINVAL ;#endif - } -else { + }else {if (start) *start = buffer;#ifdef SCSI_NCR_USER_INFO_SUPPORT retv =ncr_host_info (ncb, buffer, offset, length); -#else - retv = -EINVAL ;#endif } +NCR_UNLOCK_HOSTS (flags); +proc_done:return retv; }
drivers/scsi/sym53c8xx.c
Quoted 19 lines · Tim Hockin
spinlock_tsym53c8xx_lock=SPIN_LOCK_UNLOCKED;drivers/scsi/sym53c8xx.c +5
-0+spinlock_t sym53c8xx_host_lock =SPIN_LOCK_UNLOCKED ;#define NCR_LOCK_DRIVER (flags) spin_lock_irqsave(&sym53c8xx_lock, flags)#define NCR_UNLOCK_DRIVER (flags) spin_unlock_irqrestore(&sym53c8xx_lock,flags) +#define NCR_LOCK_HOSTS (flags) spin_lock_irqsave(&sym53c8xx_host_lock, flags) +#define NCR_UNLOCK_HOSTS (flags) spin_unlock_irqrestore(&sym53c8xx_host_lock,flags)#define NCR_INIT_LOCK_NCB (np) spin_lock_init(&np->smp_lock);#define NCR_LOCK_NCB (np, flags) spin_lock_irqsave(&np->smp_lock, flags)@@ -650,6 +655,8 @@#define NCR_LOCK_DRIVER (flags) do { save_flags(flags); cli(); } while (0)#define NCR_UNLOCK_DRIVER (flags) do { restore_flags(flags); } while (0) +#define NCR_LOCK_HOSTS (flags) do { save_flags(flags); cli(); } while (0) +#define NCR_UNLOCK_HOSTS (flags) do { restore_flags(flags); } while (0)#define NCR_INIT_LOCK_NCB (np) do { } while (0)#define NCR_LOCK_NCB (np, flags) do { save_flags(flags); cli(); } while (0)@@ -695,7 +702,7 @@
so, this driver is mixed spinlocks and save/restore_flags? Any chance this can be converted to all spinlocks?
drivers/scsi/sym53c8xx.c line 13078
Quoted 3 lines
drivers/scsi/sym53c8xx.c +1
-1#ifdef SCSI_NCR_DYNAMIC_DMA_MAPPING -if (pci_set_dma_mask (pdev, (dma_addr_t) (0xffffffffUL ))) { +if (!pci_dma_supported (pdev, (dma_addr_t) (0xffffffffUL ))) {
totally wrong. you are going backwards.
Signature · 3 lines
Jeff Garzik | Disbelief, that's why you fail. Building 1024 | MandrakeSoft |
Quoted 17 lines · Jeff Garzik
Quoted 16 lines · Tim Hockin
spinlock_tsym53c8xx_lock=SPIN_LOCK_UNLOCKED;drivers/scsi/sym53c8xx.c +5
-0+spinlock_t sym53c8xx_host_lock =SPIN_LOCK_UNLOCKED ;#define NCR_LOCK_DRIVER (flags) spin_lock_irqsave(&sym53c8xx_lock, flags)#define NCR_UNLOCK_DRIVER (flags) spin_unlock_irqrestore(&sym53c8xx_lock,flags) +#define NCR_LOCK_HOSTS (flags) spin_lock_irqsave(&sym53c8xx_host_lock, flags) +#define NCR_UNLOCK_HOSTS (flags) spin_unlock_irqrestore(&sym53c8xx_host_lock,flags)#define NCR_INIT_LOCK_NCB (np) spin_lock_init(&np->smp_lock);#define NCR_LOCK_NCB (np, flags) spin_lock_irqsave(&np->smp_lock, flags)@@ -650,6 +655,8 @@#define NCR_LOCK_DRIVER (flags) do { save_flags(flags); cli(); } while (0)#define NCR_UNLOCK_DRIVER (flags) do { restore_flags(flags); } while (0) +#define NCR_LOCK_HOSTS (flags) do { save_flags(flags); cli(); } while (0) +#define NCR_UNLOCK_HOSTS (flags) do { restore_flags(flags); } while (0)
Quoted 6 lines
drivers/scsi/sym53c8xx.c
Quoted 3 lines
drivers/scsi/sym53c8xx.c +0
-0#define NCR_INIT_LOCK_NCB (np) do { } while (0)#define NCR_LOCK_NCB (np, flags) do { save_flags(flags); cli(); } while (0)@@ -695,7 +702,7 @@so, this driver is mixed spinlocks and save/restore_flags? Any chance this can be converted to all spinlocks?
This has been done years ago for linux 2.1.93.
The save/restore flags locking methods are conditionnaly compiled for earlier kernels. This makes the corresponding code very probably quite useless nowadays and I should remove it from the source.
Gérard.
Quoted 6 lines · Tim Hockin
All,
Attached is a patch for
sym53c8xx.cto handle the error timer better, and be more proper for SMP. The changes are very simple, and have been beaten on by us. Please let me know if there are any problems accepting this patch for general inclusion.
I have no problems accepting your patch. Thanks for it.
I just want to have to deal with a human manageable finite number of actual driver versions :). I also want the same driver source to also be useable on recent 2.2 kernels.
About timers in modules and more generally either timers in drivers or modules unloading, you must keep in mind that this stuff has been racy for years in Linux. Allow time for me to check if it is really fixed in latest kernel and so to make sure it is worthwhile to apply your patch.
Gérard.