Re: [PATCH] sym53c8xx timer and smp fixes
On Thu, 31 May 2001, Tim Hockin wrote: > 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. 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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sym53c8xx timer and smp fixes
On Thu, 31 May 2001, Tim Hockin wrote: 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. 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. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sym53c8xx timer and smp fixes
On Fri, 1 Jun 2001, Jeff Garzik wrote:
> Tim Hockin wrote:
> > spinlock_t sym53c8xx_lock = SPIN_LOCK_UNLOCKED;
> > +spinlock_t sym53c8xx_host_lock = SPIN_LOCK_UNLOCKED;
> > #defineNCR_LOCK_DRIVER(flags) spin_lock_irqsave(_lock,
>flags)
> > #defineNCR_UNLOCK_DRIVER(flags)
>spin_unlock_irqrestore(_lock,flags)
> > +#defineNCR_LOCK_HOSTS(flags) spin_lock_irqsave(_host_lock,
>flags)
> > +#defineNCR_UNLOCK_HOSTS(flags)
>spin_unlock_irqrestore(_host_lock,flags)
> >
> > #define NCR_INIT_LOCK_NCB(np) spin_lock_init(>smp_lock);
> > #defineNCR_LOCK_NCB(np, flags)spin_lock_irqsave(>smp_lock, flags)
> > @@ -650,6 +655,8 @@
> >
> > #defineNCR_LOCK_DRIVER(flags) do { save_flags(flags); cli(); } while
>(0)
> > #defineNCR_UNLOCK_DRIVER(flags) do { restore_flags(flags); } while (0)
> > +#defineNCR_LOCK_HOSTS(flags) do { save_flags(flags); cli(); } while
>(0)
> > +#defineNCR_UNLOCK_HOSTS(flags) do { restore_flags(flags); } while (0)
> >
> > #defineNCR_INIT_LOCK_NCB(np) do { } while (0)
> > #defineNCR_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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sym53c8xx timer and smp fixes
Jeff wrote: > > so, this driver is mixed spinlocks and save/restore_flags? Any > chance this can be converted to all spinlocks? > It's spinlock for 2.2 and 2.4 kernels, and save_flags for 2.0. Tim, did you cc Gerard Roudier? He mainains the sym53c8xx driver. All mail archives strip the cc list :-( -- Manfred - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sym53c8xx timer and smp fixes
Tim Hockin wrote:
> spinlock_t sym53c8xx_lock = SPIN_LOCK_UNLOCKED;
> +spinlock_t sym53c8xx_host_lock = SPIN_LOCK_UNLOCKED;
> #defineNCR_LOCK_DRIVER(flags) spin_lock_irqsave(_lock, flags)
> #defineNCR_UNLOCK_DRIVER(flags)
>spin_unlock_irqrestore(_lock,flags)
> +#defineNCR_LOCK_HOSTS(flags) spin_lock_irqsave(_host_lock,
>flags)
> +#defineNCR_UNLOCK_HOSTS(flags)
>spin_unlock_irqrestore(_host_lock,flags)
>
> #define NCR_INIT_LOCK_NCB(np) spin_lock_init(>smp_lock);
> #defineNCR_LOCK_NCB(np, flags)spin_lock_irqsave(>smp_lock, flags)
> @@ -650,6 +655,8 @@
>
> #defineNCR_LOCK_DRIVER(flags) do { save_flags(flags); cli(); } while (0)
> #defineNCR_UNLOCK_DRIVER(flags) do { restore_flags(flags); } while (0)
> +#defineNCR_LOCK_HOSTS(flags) do { save_flags(flags); cli(); } while (0)
> +#defineNCR_UNLOCK_HOSTS(flags) do { restore_flags(flags); } while (0)
>
> #defineNCR_INIT_LOCK_NCB(np) do { } while (0)
> #defineNCR_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?
> #ifdef SCSI_NCR_DYNAMIC_DMA_MAPPING
> - if (pci_set_dma_mask(pdev, (dma_addr_t) (0xUL))) {
> + if (!pci_dma_supported(pdev, (dma_addr_t) (0xUL))) {
totally wrong. you are going backwards.
--
Jeff Garzik | Disbelief, that's why you fail.
Building 1024|
MandrakeSoft |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sym53c8xx timer and smp fixes
Tim Hockin wrote:
spinlock_t sym53c8xx_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t sym53c8xx_host_lock = SPIN_LOCK_UNLOCKED;
#defineNCR_LOCK_DRIVER(flags) spin_lock_irqsave(sym53c8xx_lock, flags)
#defineNCR_UNLOCK_DRIVER(flags)
spin_unlock_irqrestore(sym53c8xx_lock,flags)
+#defineNCR_LOCK_HOSTS(flags) spin_lock_irqsave(sym53c8xx_host_lock,
flags)
+#defineNCR_UNLOCK_HOSTS(flags)
spin_unlock_irqrestore(sym53c8xx_host_lock,flags)
#define NCR_INIT_LOCK_NCB(np) spin_lock_init(np-smp_lock);
#defineNCR_LOCK_NCB(np, flags)spin_lock_irqsave(np-smp_lock, flags)
@@ -650,6 +655,8 @@
#defineNCR_LOCK_DRIVER(flags) do { save_flags(flags); cli(); } while (0)
#defineNCR_UNLOCK_DRIVER(flags) do { restore_flags(flags); } while (0)
+#defineNCR_LOCK_HOSTS(flags) do { save_flags(flags); cli(); } while (0)
+#defineNCR_UNLOCK_HOSTS(flags) do { restore_flags(flags); } while (0)
#defineNCR_INIT_LOCK_NCB(np) do { } while (0)
#defineNCR_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?
#ifdef SCSI_NCR_DYNAMIC_DMA_MAPPING
- if (pci_set_dma_mask(pdev, (dma_addr_t) (0xUL))) {
+ if (!pci_dma_supported(pdev, (dma_addr_t) (0xUL))) {
totally wrong. you are going backwards.
--
Jeff Garzik | Disbelief, that's why you fail.
Building 1024|
MandrakeSoft |
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sym53c8xx timer and smp fixes
Jeff wrote: so, this driver is mixed spinlocks and save/restore_flags? Any chance this can be converted to all spinlocks? It's spinlock for 2.2 and 2.4 kernels, and save_flags for 2.0. Tim, did you cc Gerard Roudier? He mainains the sym53c8xx driver. All mail archives strip the cc list :-( -- Manfred - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] sym53c8xx timer and smp fixes
On Fri, 1 Jun 2001, Jeff Garzik wrote:
Tim Hockin wrote:
spinlock_t sym53c8xx_lock = SPIN_LOCK_UNLOCKED;
+spinlock_t sym53c8xx_host_lock = SPIN_LOCK_UNLOCKED;
#defineNCR_LOCK_DRIVER(flags) spin_lock_irqsave(sym53c8xx_lock,
flags)
#defineNCR_UNLOCK_DRIVER(flags)
spin_unlock_irqrestore(sym53c8xx_lock,flags)
+#defineNCR_LOCK_HOSTS(flags) spin_lock_irqsave(sym53c8xx_host_lock,
flags)
+#defineNCR_UNLOCK_HOSTS(flags)
spin_unlock_irqrestore(sym53c8xx_host_lock,flags)
#define NCR_INIT_LOCK_NCB(np) spin_lock_init(np-smp_lock);
#defineNCR_LOCK_NCB(np, flags)spin_lock_irqsave(np-smp_lock, flags)
@@ -650,6 +655,8 @@
#defineNCR_LOCK_DRIVER(flags) do { save_flags(flags); cli(); } while
(0)
#defineNCR_UNLOCK_DRIVER(flags) do { restore_flags(flags); } while (0)
+#defineNCR_LOCK_HOSTS(flags) do { save_flags(flags); cli(); } while
(0)
+#defineNCR_UNLOCK_HOSTS(flags) do { restore_flags(flags); } while (0)
#defineNCR_INIT_LOCK_NCB(np) do { } while (0)
#defineNCR_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.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] 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
--
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[EMAIL PROTECTED]
diff -ruN dist-2.4.5/drivers/scsi/sym53c8xx.c cobalt-2.4.5/drivers/scsi/sym53c8xx.c
--- dist-2.4.5/drivers/scsi/sym53c8xx.c Fri Apr 27 13:59:19 2001
+++ cobalt-2.4.5/drivers/scsi/sym53c8xx.c Thu May 31 14:32:43 2001
@@ -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;
#defineNCR_LOCK_DRIVER(flags) spin_lock_irqsave(_lock, flags)
#defineNCR_UNLOCK_DRIVER(flags)
spin_unlock_irqrestore(_lock,flags)
+#defineNCR_LOCK_HOSTS(flags) spin_lock_irqsave(_host_lock,
+flags)
+#defineNCR_UNLOCK_HOSTS(flags)
+spin_unlock_irqrestore(_host_lock,flags)
#define NCR_INIT_LOCK_NCB(np) spin_lock_init(>smp_lock);
#defineNCR_LOCK_NCB(np, flags)spin_lock_irqsave(>smp_lock, flags)
@@ -650,6 +655,8 @@
#defineNCR_LOCK_DRIVER(flags) do { save_flags(flags); cli(); } while (0)
#defineNCR_UNLOCK_DRIVER(flags) do { restore_flags(flags); } while (0)
+#defineNCR_LOCK_HOSTS(flags) do { save_flags(flags); cli(); } while (0)
+#defineNCR_UNLOCK_HOSTS(flags) do { restore_flags(flags); } while (0)
#defineNCR_INIT_LOCK_NCB(np) do { } while (0)
#defineNCR_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(>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(>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(>timer, ktime_get((HZ+9)/10));
#else
- np->timer.expires = ktime_get(SCSI_NCR_TIMER_INTERVAL);
+ mod_timer(>timer, ktime_get(SCSI_NCR_TIMER_INTERVAL));
#endif
- add_timer(>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) (0xUL))) {
+ if (!pci_dma_supported(pdev, (dma_addr_t) (0xUL))) {
printk(KERN_WARNING NAME53C8XX
"32 BIT PCI BUS DMA ADDRESSING NOT SUPPORTED\n");
return -1;
@@ -14181,13
[PATCH] 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
--
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[EMAIL PROTECTED]
diff -ruN dist-2.4.5/drivers/scsi/sym53c8xx.c cobalt-2.4.5/drivers/scsi/sym53c8xx.c
--- dist-2.4.5/drivers/scsi/sym53c8xx.c Fri Apr 27 13:59:19 2001
+++ cobalt-2.4.5/drivers/scsi/sym53c8xx.c Thu May 31 14:32:43 2001
@@ -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;
#defineNCR_LOCK_DRIVER(flags) spin_lock_irqsave(sym53c8xx_lock, flags)
#defineNCR_UNLOCK_DRIVER(flags)
spin_unlock_irqrestore(sym53c8xx_lock,flags)
+#defineNCR_LOCK_HOSTS(flags) spin_lock_irqsave(sym53c8xx_host_lock,
+flags)
+#defineNCR_UNLOCK_HOSTS(flags)
+spin_unlock_irqrestore(sym53c8xx_host_lock,flags)
#define NCR_INIT_LOCK_NCB(np) spin_lock_init(np-smp_lock);
#defineNCR_LOCK_NCB(np, flags)spin_lock_irqsave(np-smp_lock, flags)
@@ -650,6 +655,8 @@
#defineNCR_LOCK_DRIVER(flags) do { save_flags(flags); cli(); } while (0)
#defineNCR_UNLOCK_DRIVER(flags) do { restore_flags(flags); } while (0)
+#defineNCR_LOCK_HOSTS(flags) do { save_flags(flags); cli(); } while (0)
+#defineNCR_UNLOCK_HOSTS(flags) do { restore_flags(flags); } while (0)
#defineNCR_INIT_LOCK_NCB(np) do { } while (0)
#defineNCR_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) (0xUL))) {
+ if (!pci_dma_supported(pdev, (dma_addr_t) (0xUL))) {
printk(KERN_WARNING NAME53C8XX
32 BIT PCI BUS DMA ADDRESSING NOT SUPPORTED\n);
