Skip to contentSkip to conversation

: c-r4k.c 4/7 flush_cache_mm cleanup

PATCH In review Juan Quintela 3 participants 5 messages Mar 28, 2003 – Mar 29, 2003 MIPS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 : c-r4k.c 4/7 flush_cache_mm cleanup

Hi
flush_cache_mm can use __flush_cache_all.

Later, Juan.

 build/arch/mips/mm/c-r4k.c |   18 +++++-------------
 1 files changed, 5 insertions(+), 13 deletions(-)
build/arch/mips/mm/c-r4k.c renamed from build/arch/mips/mm/c-r4k.c~c-r4k_flush_cache_mm +5 -13
diff -puN build/arch/mips/mm/c-r4k.c~c-r4k_flush_cache_mm build/arch/mips/mm/c-r4k.c
@@ -255,18 +255,10 @@ static void r4k_flush_cache_range(struct
  * the cache created only by a certain context, but on the MIPS
  * (and actually certain Sparc's) we cannot.
  */
-static void r4k_flush_scache_mm(struct mm_struct *mm)
+static void r4k_flush_cache_mm(struct mm_struct *mm)
 {
-	if (cpu_context(smp_processor_id(), mm) != 0) {
-		r4k_flush_scache_all();
-	}
-}
-
-static void r4k_flush_pcache_mm(struct mm_struct *mm)
-{
-	if (cpu_context(smp_processor_id(), mm) != 0) {
-		r4k_flush_pcache_all();
-	}
+	if (cpu_context(smp_processor_id(), mm) != 0)
+		__flush_cache_all();
 }
 
 static void r4k_flush_cache_page(struct vm_area_struct *vma,
@@ -709,7 +701,7 @@ static void __init setup_noscache_funcs(
 	}
 	_flush_cache_all = r4k_flush_pcache_all;
 	___flush_cache_all = r4k_flush_pcache_all;
-	_flush_cache_mm = r4k_flush_pcache_mm;
+	_flush_cache_mm = r4k_flush_cache_mm;
 	_flush_cache_page = r4k_flush_cache_page;
 	_flush_icache_page = r4k_flush_icache_page;
 	_flush_cache_range = r4k_flush_cache_range;
@@ -745,7 +737,7 @@ static void __init setup_scache_funcs(vo
 
 	_flush_cache_all = r4k_flush_pcache_all;
 	___flush_cache_all = r4k_flush_scache_all;
-	_flush_cache_mm = r4k_flush_scache_mm;
+	_flush_cache_mm = r4k_flush_cache_mm;
 	_flush_cache_range = r4k_flush_cache_range;
 	_flush_cache_page = r4k_flush_cache_page;
 	_flush_icache_page = r4k_flush_icache_page;

_

Signature · 2 lines
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy
#1 lore raw
Quoted 1 line · Juan Quintela

flush_cache_mm can use __flush_cache_all.

Wrong, it should use r4k_flush_pcache_all() unconditionally, but I'm told
such a setup triggers a bug somewhere, that needs to be tracked down before committing that change to the CVS.

Signature · 3 lines
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +
#2 lore raw
Quoted 5 lines · Maciej W. Rozycki
Quoted 1 line

flush_cache_mm can use __flush_cache_all.

Wrong, it should use r4k_flush_pcache_all() unconditionally, but I'm told
such a setup triggers a bug somewhere, that needs to be tracked down before committing that change to the CVS.

Now that the problem is mentioned on the list lemme elaborate a bit. The problem mentioned only affects R4000SC and R4400SC processors. Flush_cache_mm is only used when a mm is either copied on fork or when it's finally destroyed. Because the S-cache is is physically indexed and the P-cache is refilled from the S-cache if data should be still in there we don't need to flush the S-cache ever for any of the mm's cacheflushing functions. So the observation that things are only working properly if we do flush the S-cache also suggest we're either having a bug elsewhere in the cache code or we're hitting a hardware problem.

Ralf

#3 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"ralf" == Ralf Baechle <[email protected]> writes:

Quoted 5 lines · Maciej W. Rozycki
Quoted 5 lines
Quoted 1 line

flush_cache_mm can use __flush_cache_all.

Wrong, it should use r4k_flush_pcache_all() unconditionally, but I'm told such a setup triggers a bug somewhere, that needs to be tracked down before committing that change to the CVS.

ralf> Now that the problem is mentioned on the list lemme elaborate a bit. The ralf> problem mentioned only affects R4000SC and R4400SC processors. ralf> Flush_cache_mm is only used when a mm is either copied on fork or when ralf> it's finally destroyed. Because the S-cache is is physically indexed ralf> and the P-cache is refilled from the S-cache if data should be still in ralf> there we don't need to flush the S-cache ever for any of the mm's ralf> cacheflushing functions. So the observation that things are only ralf> working properly if we do flush the S-cache also suggest we're either ralf> having a bug elsewhere in the cache code or we're hitting a hardware ralf> problem.

Ok, will took a look at that (casually my I2 has an R4400SC), but I still think that patch can be applied. It has the same behaviour than actual code. And it is a cleanup. When we found the bug, we can do a s/__flush_cache_all/r4k_flush_pcache_all/

on that function.

Later, Juan.

Signature · 2 lines
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy
#4 lore raw
In reply to Ralf Baechle
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"ralf" == Ralf Baechle <[email protected]> writes:

Quoted 5 lines · Maciej W. Rozycki
Quoted 5 lines
Quoted 1 line

flush_cache_mm can use __flush_cache_all.

Wrong, it should use r4k_flush_pcache_all() unconditionally, but I'm told such a setup triggers a bug somewhere, that needs to be tracked down before committing that change to the CVS.

ralf> Now that the problem is mentioned on the list lemme elaborate a bit. The ralf> problem mentioned only affects R4000SC and R4400SC processors. ralf> Flush_cache_mm is only used when a mm is either copied on fork or when ralf> it's finally destroyed. Because the S-cache is is physically indexed ralf> and the P-cache is refilled from the S-cache if data should be still in ralf> there we don't need to flush the S-cache ever for any of the mm's ralf> cacheflushing functions. So the observation that things are only ralf> working properly if we do flush the S-cache also suggest we're either ralf> having a bug elsewhere in the cache code or we're hitting a hardware ralf> problem.

Just to add some more data points. flush_cache_mm() is only called from two places:

- kernel/fork.c::dup_mmap()
- mm/mmap.c::exit_mmap()

I just changed flush_cache_mm() to be r4k_flush_pcache_all() and put after the two calls a __flush_cache_all(). As expected everything worked :)

Now if I removed teh __flush_cache_all() for any of the callers, everything goes well. But if I remove it for both of them things crashed during boot. I am looking at the code of both functions, and can't see a good reason for them to fail :(

Does that ring any bells on you?

I am still investigating that one.

Later, Juan.

Signature · 2 lines
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy