Re: [PATCH 2.6.24][MIPS]Work in progress: fix HIGHMEM-enabled dcache flushing on 32-bit processor

[David VomLehn <dvomlehn@cisco.com>]

Lance Richardson wrote:
> ...I have tracked down the cause of a crash that only
> occurs with SMP enabled, and wondered there might be a better approach than
> the one I took for fixing it.
> 
> The crash scenario involves one CPU having an atomic mapping of type KM_USER0
> in use when the other CPU happens to call r4k_flush_cachee_page(),
> which in turn
> calls r4k_on_each_cpu() for local_r4k_flush_cache_page().  The original CPU is
> interrupted (still with an active KM_USER0 mapping),
> local_r4k_flush_cache_page()
> is called, and in the process another KM_USER0 mapping is attempted (and fails
> in flames.)
> 
> The diffs below (against 2.6.26.1) appear to have eliminated this
> problem - does this
> make sense, and is there a better way?

I think it does make sense.
...
>  static inline void local_r4k_flush_cache_page(void *args)
> @@ -452,6 +453,12 @@
>  	pmd_t *pmdp;
>  	pte_t *ptep;
>  	void *vaddr;
> +        enum km_type kmtype;
> +
> +        if (fcp_args->cpu == smp_processor_id())
> +                kmtype = KM_USER0;
> +        else
> +                kmtype = KM_FLUSH_CACHE_PAGE;

The basis for checking the CPU number is slightly obscure, and caching is hard 
enough to understand as it is. How about always dedicating your new km_type enum 
KM_FLUSH_CACHE_PAGE to cross-processor cache flushing?

First, take the guts of local_r4k_flush_cache_page and move them to a new 
function, common_r4k_flush_cache_page, that takes a void* arg and an enum 
km_type. Change local_r4k_flush_cache_page to call this new function with a 
second argument of KM_USER0.

Next, have r4k_flush_cache_page call a new function which then calls 
common_r4k_flush_cache_page with a second argument of KM_FLUSH_CACHE_PAGE.

This approach may have very slightly better performance and lets you keep the 
size of flush_cache_page_args the same.