* Double spin_unlock in bte.c
@ 2004-10-14 11:40 Robin Holt
2004-10-14 21:00 ` Jesse Barnes
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Robin Holt @ 2004-10-14 11:40 UTC (permalink / raw)
To: linux-ia64
If all bte interfaces are in use, the current code will leave the
bte pointer set when exiting from the loop trying to locate an
interface to use. This results in two processes using the same
interface and both trying to free the same one. With a
premptible kernel, this results in the preempt count getting
off.
Signed-off-by: Robin Holt
Index: linux-2.6/arch/ia64/sn/kernel/bte.c
=================================--- linux-2.6.orig/arch/ia64/sn/kernel/bte.c 2004-10-06 11:48:25.000000000 -0500
+++ linux-2.6/arch/ia64/sn/kernel/bte.c 2004-10-14 06:34:45.000000000 -0500
@@ -131,12 +131,12 @@
(BTE_LNSTAT_LOAD(bte) & BTE_ACTIVE)) {
/* Got the lock but BTE still busy */
spin_unlock(&bte->spinlock);
- bte = NULL;
} else {
/* we got the lock and it's not busy */
break;
}
}
+ bte = NULL;
}
if (bte != NULL) {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: Double spin_unlock in bte.c 2004-10-14 11:40 Double spin_unlock in bte.c Robin Holt @ 2004-10-14 21:00 ` Jesse Barnes 2004-10-15 5:18 ` Luck, Tony ` (3 subsequent siblings) 4 siblings, 0 replies; 6+ messages in thread From: Jesse Barnes @ 2004-10-14 21:00 UTC (permalink / raw) To: linux-ia64 Tony, I don't think there's any hurry on this one, so you can just send it along post-2.6.9 if you want. Hopefully it won't conflict with Pat's big sn2 patches. Acked-by: Jesse Barnes <jbarnes@sgi.com> On Thursday, October 14, 2004 6:40 am, Robin Holt wrote: > If all bte interfaces are in use, the current code will leave the > bte pointer set when exiting from the loop trying to locate an > interface to use. This results in two processes using the same > interface and both trying to free the same one. With a > premptible kernel, this results in the preempt count getting > off. > > Signed-off-by: Robin Holt > > > Index: linux-2.6/arch/ia64/sn/kernel/bte.c > =================================> --- linux-2.6.orig/arch/ia64/sn/kernel/bte.c 2004-10-06 11:48:25.000000000 > -0500 +++ linux-2.6/arch/ia64/sn/kernel/bte.c 2004-10-14 06:34:45.000000000 > -0500 @@ -131,12 +131,12 @@ > (BTE_LNSTAT_LOAD(bte) & BTE_ACTIVE)) { > /* Got the lock but BTE still busy */ > spin_unlock(&bte->spinlock); > - bte = NULL; > } else { > /* we got the lock and it's not busy */ > break; > } > } > + bte = NULL; > } > > if (bte != NULL) { > - > To unsubscribe from this list: send the line "unsubscribe linux-ia64" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: Double spin_unlock in bte.c 2004-10-14 11:40 Double spin_unlock in bte.c Robin Holt 2004-10-14 21:00 ` Jesse Barnes @ 2004-10-15 5:18 ` Luck, Tony 2004-10-18 17:02 ` Pat Gefre ` (2 subsequent siblings) 4 siblings, 0 replies; 6+ messages in thread From: Luck, Tony @ 2004-10-15 5:18 UTC (permalink / raw) To: linux-ia64 >Tony, I don't think there's any hurry on this one, so you can >just send it along post-2.6.9 if you want. Hopefully it won't conflict >with Pat's big sn2 patches. It looks like it applies corectly on top of Pat's re-org patch. So I'll send it along after that goes in. -Tony ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: Double spin_unlock in bte.c 2004-10-14 11:40 Double spin_unlock in bte.c Robin Holt 2004-10-14 21:00 ` Jesse Barnes 2004-10-15 5:18 ` Luck, Tony @ 2004-10-18 17:02 ` Pat Gefre 2004-10-18 21:34 ` Robin Holt 2004-10-19 19:45 ` Pat Gefre 4 siblings, 0 replies; 6+ messages in thread From: Pat Gefre @ 2004-10-18 17:02 UTC (permalink / raw) To: linux-ia64 On Thu, 14 Oct 2004, Luck, Tony wrote: + >Tony, I don't think there's any hurry on this one, so you can + >just send it along post-2.6.9 if you want. Hopefully it won't conflict + >with Pat's big sn2 patches. + + It looks like it applies corectly on top of Pat's re-org patch. So + I'll send it along after that goes in. + + -Tony + - + To unsubscribe from this list: send the line "unsubscribe linux-ia64" in + the body of a message to majordomo@vger.kernel.org + More majordomo info at http://vger.kernel.org/majordomo-info.html + Tony, I have another bte mod too - I've added it to the list - it's 008-bugon-fix on the oss site. # This is a BitKeeper generated diff -Nru style patch. # # ChangeSet # 2004/10/14 10:43:41-05:00 pfg@sgi.com # arch/ia64/sn/kernel/bte.c # BUG_ON test was backwards # # arch/ia64/sn/kernel/bte.c # 2004/10/14 10:43:32-05:00 pfg@sgi.com +3 -3 # BUG_ON test was backwards # diff -Nru a/arch/ia64/sn/kernel/bte.c b/arch/ia64/sn/kernel/bte.c --- a/arch/ia64/sn/kernel/bte.c 2004-10-14 10:44:34 -05:00 +++ b/arch/ia64/sn/kernel/bte.c 2004-10-14 10:44:34 -05:00 @@ -79,9 +79,9 @@ return BTE_SUCCESS; } - BUG_ON(!((len & L1_CACHE_MASK) || - (src & L1_CACHE_MASK) || (dest & L1_CACHE_MASK))); - BUG_ON(len < ((BTE_LEN_MASK + 1) << L1_CACHE_SHIFT)); + BUG_ON((len & L1_CACHE_MASK) || + (src & L1_CACHE_MASK) || (dest & L1_CACHE_MASK)); + BUG_ON(!(len < ((BTE_LEN_MASK + 1) << L1_CACHE_SHIFT))); if (mode & BTE_USE_DEST) { /* try remote then local */ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Double spin_unlock in bte.c 2004-10-14 11:40 Double spin_unlock in bte.c Robin Holt ` (2 preceding siblings ...) 2004-10-18 17:02 ` Pat Gefre @ 2004-10-18 21:34 ` Robin Holt 2004-10-19 19:45 ` Pat Gefre 4 siblings, 0 replies; 6+ messages in thread From: Robin Holt @ 2004-10-18 21:34 UTC (permalink / raw) To: linux-ia64 [-- Attachment #1: Type: text/plain, Size: 545 bytes --] > I have another bte mod too - I've added it to the list - it's > 008-bugon-fix on the oss site. Pat and Tony, I have created a 009-bte_double_unlock patch which is the same patch that was sent to the list last week. Russ Anderson and I have worked up two additional patches for the bte. The first works around an extremely infrequent hardware issue and the other reduces contention on the bte interface locks by distributing based upon the slice that the requestors cpu is on. Pat, can you add those to your patch set? Thanks, Robin Holt [-- Attachment #2: 009-bte_double_unlock --] [-- Type: text/plain, Size: 566 bytes --] Index: linux-2.6/arch/ia64/sn/kernel/bte.c =================================================================== --- linux-2.6.orig/arch/ia64/sn/kernel/bte.c 2004-10-18 06:59:45.000000000 -0500 +++ linux-2.6/arch/ia64/sn/kernel/bte.c 2004-10-18 07:00:26.000000000 -0500 @@ -125,12 +125,12 @@ (BTE_LNSTAT_LOAD(bte) & BTE_ACTIVE)) { /* Got the lock but BTE still busy */ spin_unlock(&bte->spinlock); - bte = NULL; } else { /* we got the lock and it's not busy */ break; } } + bte = NULL; } if (bte != NULL) { [-- Attachment #3: 010-notification_war --] [-- Type: text/plain, Size: 3520 bytes --] Index: linux-2.6/include/asm-ia64/sn/bte.h =================================================================== --- linux-2.6.orig/include/asm-ia64/sn/bte.h 2004-10-18 07:12:40.000000000 -0500 +++ linux-2.6/include/asm-ia64/sn/bte.h 2004-10-18 15:47:29.000000000 -0500 @@ -55,7 +55,9 @@ /* macro to force the IBCT0 value valid */ #define BTE_VALID_MODE(x) ((x) & (IBCT_NOTIFY | IBCT_ZFIL_MODE)) -#define BTE_ACTIVE (IBLS_BUSY | IBLS_ERROR) +#define BTE_ACTIVE (IBLS_BUSY | IBLS_ERROR) +#define BTE_WORD_AVAILABLE (IBLS_BUSY << 1) +#define BTE_WORD_BUSY (~BTE_WORD_AVAILABLE) /* * Some macros to simplify reading. Index: linux-2.6/arch/ia64/sn/kernel/bte.c =================================================================== --- linux-2.6.orig/arch/ia64/sn/kernel/bte.c 2004-10-18 15:45:52.000000000 -0500 +++ linux-2.6/arch/ia64/sn/kernel/bte.c 2004-10-18 15:49:03.000000000 -0500 @@ -15,6 +15,7 @@ #include <asm/sn/pda.h> #include "shubio.h" #include <asm/nodedata.h> +#include <asm/delay.h> #include <linux/bootmem.h> #include <linux/string.h> @@ -69,6 +70,7 @@ struct bteinfo_s *bte; bte_result_t bte_status; unsigned long irq_flags; + unsigned long itc_end = 0; struct bteinfo_s *btes_to_try[MAX_INTERFACES_TO_TRY]; int bte_if_index; @@ -107,6 +109,7 @@ } } +retry_bteop: do { local_irq_save(irq_flags); @@ -121,7 +124,7 @@ } if (spin_trylock(&bte->spinlock)) { - if ((*bte->most_rcnt_na & BTE_ACTIVE) || + if (!(*bte->most_rcnt_na & BTE_WORD_AVAILABLE) || (BTE_LNSTAT_LOAD(bte) & BTE_ACTIVE)) { /* Got the lock but BTE still busy */ spin_unlock(&bte->spinlock); @@ -155,7 +158,7 @@ transfer_size = ((len >> L1_CACHE_SHIFT) & BTE_LEN_MASK); /* Initialize the notification to a known value. */ - *bte->most_rcnt_na = -1L; + *bte->most_rcnt_na = BTE_WORD_BUSY; /* Set the status reg busy bit and transfer length */ BTE_PRINTKV(("IBLS = 0x%lx\n", IBLS_BUSY | transfer_size)); @@ -177,13 +180,25 @@ BTE_PRINTK(("IBCT = 0x%lx)\n", BTE_VALID_MODE(mode))); BTE_CTRL_STORE(bte, BTE_VALID_MODE(mode)); + itc_end = ia64_get_itc() + (40000000 * local_cpu_data->cyc_per_usec); + spin_unlock_irqrestore(&bte->spinlock, irq_flags); if (notification != NULL) { return BTE_SUCCESS; } - while ((transfer_stat = *bte->most_rcnt_na) == -1UL) { + while ((transfer_stat = *bte->most_rcnt_na) == BTE_WORD_BUSY) { + if (ia64_get_itc() > itc_end) { + BTE_PRINTK(("BTE timeout nasid 0x%x bte%d IBLS = 0x%lx na 0x%lx\n", + NASID_GET(bte->bte_base_addr), bte->bte_num, + BTE_LNSTAT_LOAD(bte), *bte->most_rcnt_na) ); + bte->bte_error_count++; + bte->bh_error = IBLS_ERROR; + bte_error_handler((unsigned long)NODEPDA(bte->bte_cnode)); + *bte->most_rcnt_na = BTE_WORD_AVAILABLE; + goto retry_bteop; + } } BTE_PRINTKV((" Delay Done. IBLS = 0x%lx, most_rcnt_na = 0x%lx\n", @@ -191,10 +206,11 @@ if (transfer_stat & IBLS_ERROR) { bte_status = transfer_stat & ~IBLS_ERROR; - *bte->most_rcnt_na = 0L; } else { bte_status = BTE_SUCCESS; } + *bte->most_rcnt_na = BTE_WORD_AVAILABLE; + BTE_PRINTK(("Returning status is 0x%lx and most_rcnt_na is 0x%lx\n", BTE_LNSTAT_LOAD(bte), *bte->most_rcnt_na)); @@ -414,7 +430,7 @@ */ mynodepda->bte_if[i].most_rcnt_na = &(mynodepda->bte_if[i].notify); - mynodepda->bte_if[i].notify = 0L; + mynodepda->bte_if[i].notify = BTE_WORD_AVAILABLE; spin_lock_init(&mynodepda->bte_if[i].spinlock); mynodepda->bte_if[i].bte_cnode = cnode; [-- Attachment #4: 011-bte_cpu_pref --] [-- Type: text/plain, Size: 2072 bytes --] Index: linux-2.6/arch/ia64/sn/kernel/bte.c =================================================================== --- linux-2.6.orig/arch/ia64/sn/kernel/bte.c 2004-10-18 15:51:04.000000000 -0500 +++ linux-2.6/arch/ia64/sn/kernel/bte.c 2004-10-18 16:18:54.000000000 -0500 @@ -73,6 +73,7 @@ unsigned long itc_end = 0; struct bteinfo_s *btes_to_try[MAX_INTERFACES_TO_TRY]; int bte_if_index; + int bte_pri, bte_sec; BTE_PRINTK(("bte_copy(0x%lx, 0x%lx, 0x%lx, 0x%lx, 0x%p)\n", src, dest, len, mode, notification)); @@ -85,24 +86,33 @@ (src & L1_CACHE_MASK) || (dest & L1_CACHE_MASK)); BUG_ON(!(len < ((BTE_LEN_MASK + 1) << L1_CACHE_SHIFT))); + /* CPU 0 (per node) tries bte0 first, CPU 1 try bte1 first */ + if (cpuid_to_subnode(smp_processor_id()) == 0) { + bte_pri = 0; + bte_sec = 1; + } else { + bte_pri = 1; + bte_sec = 0; + } + if (mode & BTE_USE_DEST) { /* try remote then local */ - btes_to_try[0] = bte_if_on_node(NASID_GET(dest), 0); - btes_to_try[1] = bte_if_on_node(NASID_GET(dest), 1); + btes_to_try[0] = bte_if_on_node(NASID_GET(dest), bte_pri); + btes_to_try[1] = bte_if_on_node(NASID_GET(dest), bte_sec); if (mode & BTE_USE_ANY) { - btes_to_try[2] = bte_if_on_node(get_nasid(), 0); - btes_to_try[3] = bte_if_on_node(get_nasid(), 1); + btes_to_try[2] = bte_if_on_node(get_nasid(), bte_pri); + btes_to_try[3] = bte_if_on_node(get_nasid(), bte_sec); } else { btes_to_try[2] = NULL; btes_to_try[3] = NULL; } } else { /* try local then remote */ - btes_to_try[0] = bte_if_on_node(get_nasid(), 0); - btes_to_try[1] = bte_if_on_node(get_nasid(), 1); + btes_to_try[0] = bte_if_on_node(get_nasid(), bte_pri); + btes_to_try[1] = bte_if_on_node(get_nasid(), bte_sec); if (mode & BTE_USE_ANY) { - btes_to_try[2] = bte_if_on_node(NASID_GET(dest), 0); - btes_to_try[3] = bte_if_on_node(NASID_GET(dest), 1); + btes_to_try[2] = bte_if_on_node(NASID_GET(dest), bte_pri); + btes_to_try[3] = bte_if_on_node(NASID_GET(dest), bte_sec); } else { btes_to_try[2] = NULL; btes_to_try[3] = NULL; ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: Double spin_unlock in bte.c 2004-10-14 11:40 Double spin_unlock in bte.c Robin Holt ` (3 preceding siblings ...) 2004-10-18 21:34 ` Robin Holt @ 2004-10-19 19:45 ` Pat Gefre 4 siblings, 0 replies; 6+ messages in thread From: Pat Gefre @ 2004-10-19 19:45 UTC (permalink / raw) To: linux-ia64 On Mon, 18 Oct 2004, Pat Gefre wrote: + On Thu, 14 Oct 2004, Luck, Tony wrote: + + + >Tony, I don't think there's any hurry on this one, so you can + + >just send it along post-2.6.9 if you want. Hopefully it won't conflict + + >with Pat's big sn2 patches. + + + + It looks like it applies corectly on top of Pat's re-org patch. So + + I'll send it along after that goes in. + + + + -Tony + + - + + To unsubscribe from this list: send the line "unsubscribe linux-ia64" in + + the body of a message to majordomo@vger.kernel.org + + More majordomo info at http://vger.kernel.org/majordomo-info.html + + + + Tony, + + I have another bte mod too - I've added it to the list - it's + 008-bugon-fix on the oss site. + Sorry .... forgot again... Signed-off-by: Patrick Gefre <pfg@sgi.com> + + + # This is a BitKeeper generated diff -Nru style patch. + # + # ChangeSet + # 2004/10/14 10:43:41-05:00 pfg@sgi.com + # arch/ia64/sn/kernel/bte.c + # BUG_ON test was backwards + # + # arch/ia64/sn/kernel/bte.c + # 2004/10/14 10:43:32-05:00 pfg@sgi.com +3 -3 + # BUG_ON test was backwards + # + diff -Nru a/arch/ia64/sn/kernel/bte.c b/arch/ia64/sn/kernel/bte.c + --- a/arch/ia64/sn/kernel/bte.c 2004-10-14 10:44:34 -05:00 + +++ b/arch/ia64/sn/kernel/bte.c 2004-10-14 10:44:34 -05:00 + @@ -79,9 +79,9 @@ + return BTE_SUCCESS; + } + + - BUG_ON(!((len & L1_CACHE_MASK) || + - (src & L1_CACHE_MASK) || (dest & L1_CACHE_MASK))); + - BUG_ON(len < ((BTE_LEN_MASK + 1) << L1_CACHE_SHIFT)); + + BUG_ON((len & L1_CACHE_MASK) || + + (src & L1_CACHE_MASK) || (dest & L1_CACHE_MASK)); + + BUG_ON(!(len < ((BTE_LEN_MASK + 1) << L1_CACHE_SHIFT))); + + if (mode & BTE_USE_DEST) { + /* try remote then local */ + ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-10-19 19:45 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-10-14 11:40 Double spin_unlock in bte.c Robin Holt 2004-10-14 21:00 ` Jesse Barnes 2004-10-15 5:18 ` Luck, Tony 2004-10-18 17:02 ` Pat Gefre 2004-10-18 21:34 ` Robin Holt 2004-10-19 19:45 ` Pat Gefre
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox