Regarding this patch (commit e5eb925a1804c4a52994ba57f4f68ee7a9132905),
the fix is fine for 64-bit systems, as it is impossible to overflow a
64-bit ASID value.
For 32-bit systems, there is still a problem, we don't see the type
truncation issue that was present on 64-bit systems, but there can still
be badness on ASID generation wrap.
Scenario:
o Long live process (p0) that sleeps for a long time. It acquires
what we will call ASID_0 and then is scheduled off the CPU
o We cycle through 2^32 ASIDs, and the asid_cache wraps around (not
difficult to do, just write a program that does nothing but mmap()
munmap() in a loop). We have seen this happen every 6 days with ebizzy
benchmark program.
o Start new program (p1) that happens to also get ASID_0
o p0 wakes up, and is now sharing tlb entries with p1, chaos ensues.
A workaround for this would be to use u64 for both 32-bit and 64-bit for
all ASID related variables. I have a patch for this, is it worth
testing on 32-bit systems, and sending it in?
David Daney
On 05/22/2014 06:42 AM, Ralf Baechle wrote:
On Thu, May 22, 2014 at 10:06:11AM +0800, Yong Zhang wrote:
On Wed, May 21, 2014 at 01:29:36PM +0200, Ralf Baechle wrote:
On Wed, May 21, 2014 at 01:38:53PM +0800, Yong Zhang wrote:
Please check the V2 in which I add the reporter.
And thanks libin for reporting it :)
The bug was introduced in 5636919b5c909fee54a6ef5226475ecae012ad02
[MIPS: Outline udelay and fix a few issues.] in 2009 btw. I think
the intension was to avoid holes in the structure and minimize
the bloat. I instead applied aptch
Could you please show the patch?
which also moves another member
of the struct arond such that no hole will be created in the struct.
This is important because the strcture it accessed fairly frequently
so we want to fit the most important members into as few cache
lines as possible.
I have tried to move the struct member around, but I found that the
hole cann't be avoided completely because for exampe struct cache_desc
is a bit special.
Yes, struct cache_desc is still a problem. Easily solvable though -
some of it's members are excessivly large; by using smaller data types
both the struct and its required alignment will shrink. But that's
for another patch; as for this patch my goal to just not make things
any worse.
Ralf
---
arch/mips/include/asm/cpu-info.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index dc2135b..ff2707a 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -39,14 +39,14 @@ struct cache_desc {
#define MIPS_CACHE_PINDEX 0x00000020 /* Physically indexed cache */
struct cpuinfo_mips {
- unsigned int udelay_val;
- unsigned int asid_cache;
+ unsigned long asid_cache;
/*
* Capability and feature descriptor structure for MIPS CPU
*/
unsigned long options;
unsigned long ases;
+ unsigned int udelay_val;
unsigned int processor_id;
unsigned int fpu_id;
unsigned int msa_id;
