Emilio G. Cota <c...@braap.org> writes:
> On Wed, Oct 31, 2018 at 16:13:05 +0000, Alex Bennée wrote:
>> > @@ -353,10 +355,12 @@ void s390_cpu_unhalt(S390CPU *cpu)
>> > CPUState *cs = CPU(cpu);
>> > trace_cpu_unhalt(cs->cpu_index);
>> >
>> > - if (cs->halted) {
>> > - cs->halted = 0;
>> > + cpu_mutex_lock(cs);
>> > + if (cpu_halted(cs)) {
>> > + cpu_halted_set(cs, 0);
>> > cs->exception_index = -1;
>> > }
>> > + cpu_mutex_unlock(cs);
>>
>> I think this locking is superfluous as you already added locking when
>> you introduced the helper.
>
> It gives a small perf gain, since it avoids locking & unlocking twice
> in a row (cpu_halted and cpu_halted_set both take the lock if needed).
Maybe a one line comment then above the first lock:
/* lock outside cpu_halted(_set) to avoid bouncing */
Or some such.....
>
>> Otherwise:
>>
>> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
>
> Thanks!
>
> Emilio
--
Alex Bennée
