Hi Finn,
thanks - I'm pretty sure tried that early on but botched it by excessive
locking (i.e., keeping preemption disabled when calling
get_zeroed_page()!).
So I don't think you do use too much locking here.
I'm running stress tests for a while now, without any trouble so far.
Need to add a few other stressors back in, and repeat all that on a
slower ARAnyM instance but I'm quite confident you found the solution.
Geert: with this data race fixed, it does appear my RFC patch is no
longer needed. Finn or I probably ought to prepare a new RFC patch to go
on top of your preemption patch. There is no commit ID to use in a
Fixes: tag for that one, correct?
Cheers,
Michael
On 26/03/24 22:58, Finn Thain wrote:
On Tue, 26 Mar 2024, Michael Schmitz wrote:
see below (hopefully not whitespace-damaged).
Following your approach, I added mutual exclusion to the other candidate
in the same file, namely get_pointer_table(). Together with your patch,
that seems to be sufficient to make stress-ng happy. I don't know the real
extent of the data race here so I've probably used too much locking in
this experiment (?)
diff --git a/arch/m68k/mm/motorola.c b/arch/m68k/mm/motorola.c
index 068ad7c9bb5c..d1ad55ee1b60 100644
--- a/arch/m68k/mm/motorola.c
+++ b/arch/m68k/mm/motorola.c
@@ -140,10 +140,14 @@ void __init init_pointer_table(void *table, int type)
void *get_pointer_table(int type)
{
- ptable_desc *dp = ptable_list[type].next;
- unsigned int mask = list_empty(&ptable_list[type]) ? 0 : PD_MARKBITS(dp);
+ ptable_desc *dp;
+ unsigned int mask;
unsigned int tmp, off;
+ preempt_disable();
+
+ dp = ptable_list[type].next;
+ mask = list_empty(&ptable_list[type]) ? 0 : PD_MARKBITS(dp);
/*
* For a pointer table for a user process address space, a
* table is taken from a page allocated for the purpose. Each
@@ -154,8 +158,13 @@ void *get_pointer_table(int type)
void *page;
ptable_desc *new;
- if (!(page = (void *)get_zeroed_page(GFP_KERNEL)))
+ sched_preempt_enable_no_resched();
+
+ if (!(page = (void *)get_zeroed_page(GFP_KERNEL))) {
return NULL;
+ }
+
+ preempt_disable();
if (type == TABLE_PTE) {
/*
@@ -171,6 +180,7 @@ void *get_pointer_table(int type)
PD_MARKBITS(new) = ptable_mask(type) - 1;
list_add_tail(new, dp);
+ sched_preempt_enable_no_resched();
return (pmd_t *)page;
}
@@ -181,6 +191,7 @@ void *get_pointer_table(int type)
/* move to end of list */
list_move_tail(dp, &ptable_list[type]);
}
+ sched_preempt_enable_no_resched();
return page_address(PD_PAGE(dp)) + off;
}
