From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id 597DEB7BFA for ; Fri, 31 Jul 2009 13:35:54 +1000 (EST) Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 8E3F0DDD0C for ; Fri, 31 Jul 2009 13:35:53 +1000 (EST) Message-Id: <6FD94305-B60D-4DAF-8296-88345D11187F@kernel.crashing.org> From: Kumar Gala To: Kumar Gala In-Reply-To: <2E027F3C-8FAA-42EC-99B2-9B7EC470094E@kernel.crashing.org> Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Mime-Version: 1.0 (Apple Message framework v935.3) Subject: Re: [PATCH 3/20] powerpc/mm: Add HW threads support to no_hash TLB management Date: Thu, 30 Jul 2009 22:35:47 -0500 References: <20090724091523.8AD8CDDD1B@ozlabs.org> <2E027F3C-8FAA-42EC-99B2-9B7EC470094E@kernel.crashing.org> Cc: linuxppc-dev@ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Jul 30, 2009, at 10:12 PM, Kumar Gala wrote: > > On Jul 24, 2009, at 4:15 AM, Benjamin Herrenschmidt wrote: > >> The current "no hash" MMU context management code is written with >> the assumption that one CPU == one TLB. This is not the case on >> implementations that support HW multithreading, where several >> linux CPUs can share the same TLB. >> >> This adds some basic support for this to our context management >> and our TLB flushing code. >> >> It also cleans up the optional debugging output a bit >> >> Signed-off-by: Benjamin Herrenschmidt >> --- > > I'm getting this nice oops on 32-bit book-e SMP (and I'm guessing > its because of this patch) > > Unable to handle kernel paging request for data at address 0x00000000 > Faulting instruction address: 0xc0016dac > Oops: Kernel access of bad area, sig: 11 [#1] > SMP NR_CPUS=8 MPC8572 DS > Modules linked in: > NIP: c0016dac LR: c0016d58 CTR: 0000001e > REGS: eed77ce0 TRAP: 0300 Not tainted (2.6.31-rc4-00442-gdb4c9c5) > MSR: 00021000 CR: 24288482 XER: 20000000 > DEAR: 00000000, ESR: 00000000 > TASK = eecfe140[1581] 'msgctl08' THREAD: eed76000 CPU: 0 > GPR00: 00400000 eed77d90 eecfe140 00000000 00000000 00000001 > c05bf074 c05c0cf4 > GPR08: 00000003 00000002 ff7fffff 00000000 00009b05 1004f894 > c05bdd24 00000001 > GPR16: ffffffff c05ab890 c05c0ce8 c04e0f58 c04da364 c05c0000 > 00000000 c04cfa04 > GPR24: 00000002 00000000 00000000 c05c0cd8 00000080 00000000 > ef056380 00000017 > NIP [c0016dac] switch_mmu_context+0x15c/0x520 > LR [c0016d58] switch_mmu_context+0x108/0x520 > Call Trace: > [eed77d90] [c0016d58] switch_mmu_context+0x108/0x520 (unreliable) > [eed77df0] [c040efec] schedule+0x2bc/0x800 > [eed77e70] [c01b9268] do_msgrcv+0x198/0x420 > [eed77ef0] [c01b9520] sys_msgrcv+0x30/0xa0 > [eed77f10] [c0003fe8] sys_ipc+0x1a8/0x2c0 > [eed77f40] [c00116c4] ret_from_syscall+0x0/0x3c > Instruction dump: > 57402834 7c00f850 3920fffe 5d2a003e 397b0010 5500103a 7ceb0214 > 60000000 > 60000000 81670000 39080001 38e70004 <7c0be82e> 7c005038 7c0be92e > 81260000 > ---[ end trace 3c4c3106446e1bd8 ]--- On Jul 24, 2009, at 4:15 AM, Benjamin Herrenschmidt wrote: > @@ -247,15 +261,20 @@ void switch_mmu_context(struct mm_struct > * local TLB for it and unmark it before we use it > */ > if (test_bit(id, stale_map[cpu])) { > - pr_devel("[%d] flushing stale context %d for mm @%p !\n", > - cpu, id, next); > + pr_hardcont(" | stale flush %d [%d..%d]", > + id, cpu_first_thread_in_core(cpu), > + cpu_last_thread_in_core(cpu)); > + > local_flush_tlb_mm(next); > > /* XXX This clear should ultimately be part of local_flush_tlb_mm */ > - __clear_bit(id, stale_map[cpu]); > + for (cpu = cpu_first_thread_in_core(cpu); > + cpu <= cpu_last_thread_in_core(cpu); cpu++) > + __clear_bit(id, stale_map[cpu]); > } This looks a bit dodgy. using 'cpu' as both the loop variable and what you are computing to determine loop start/end.. Changing this to: unsigned int i; ... for (i = cpu_first_thread_in_core(cpu); i <= cpu_last_thread_in_core(cpu); i++) __clear_bit(id, stale_map[i]); seems to clear up the oops. - k