Messages in this thread | | | Date | Wed, 25 Oct 2023 08:53:41 +0800 | Subject | Re: [PATCH] stop_machine: pass curstate to ack_state() | From | Rong Tao <> |
| |
On 10/25/23 8:50 AM, Rong Tao wrote: > > On 10/25/23 8:43 AM, Rong Tao wrote: >> The multi_cpu_stop() state machine uses multi_stop_data::state to hold >> the current state, and this is read and written atomically except in >> ack_state(), which performs a non-atomic read. >> >> As ack_state() only performs this non-atomic read when there is a single >> writer, this is benign, but it makes reasoning about the state machine a >> little harder. >> >> Remove the non-atomic read and pass the (atomically read) curstate in >> instead. This makes it clear that we do not expect any racy writes, and >> avoids a redundant load. > > I'd prefer if we make this comment: > > stop_machine: pass curstate to ack_state() OK, thanks a lot, i'll try that. > >> >> Acked-by: Mark Rutland <mark.rutland@arm.com> >> Signed-off-by: Rong Tao <rongtao@cestc.cn> >> --- >> kernel/stop_machine.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c >> index cedb17ba158a..268c2e581698 100644 >> --- a/kernel/stop_machine.c >> +++ b/kernel/stop_machine.c >> @@ -188,10 +188,11 @@ static void set_state(struct multi_stop_data >> *msdata, >> } >> >> /* Last one to ack a state moves to the next state. */ >> -static void ack_state(struct multi_stop_data *msdata) >> +static void ack_state(struct multi_stop_data *msdata, >> + enum multi_stop_state curstate) >> { >> if (atomic_dec_and_test(&msdata->thread_ack)) >> - set_state(msdata, msdata->state + 1); >> + set_state(msdata, curstate + 1); >> } >> >> notrace void __weak stop_machine_yield(const struct cpumask *cpumask) >> @@ -242,7 +243,7 @@ static int multi_cpu_stop(void *data) >> default: >> break; >> } >> - ack_state(msdata); >> + ack_state(msdata, curstate); >> } else if (curstate > MULTI_STOP_PREPARE) { >> /* >> * At this stage all other CPUs we depend on must spin
| |