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.
