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.
|