[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [Qemu-devel] [RFC v4 24/71] s390x: convert to cpu_halte
From: |
Cornelia Huck |
Subject: |
Re: [qemu-s390x] [Qemu-devel] [RFC v4 24/71] s390x: convert to cpu_halted |
Date: |
Fri, 9 Nov 2018 14:47:19 +0100 |
On Wed, 31 Oct 2018 16:56:54 +0000
Alex Bennée <address@hidden> wrote:
> Emilio G. Cota <address@hidden> 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.....
Yes, please.
Or introduce something like cpu_halted_flip(cs, new_state) where you
could convert the above to
if (cpu_halted_flip(cs, 0)) { /* cpu halted before */
cs_exception_index = -1;
}
Or is that actually an uncommon pattern?
>
> >
> >> Otherwise:
> >>
> >> Reviewed-by: Alex Bennée <address@hidden>
> >
> > Thanks!
> >
> > Emilio
>
>
> --
> Alex Bennée
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [qemu-s390x] [Qemu-devel] [RFC v4 24/71] s390x: convert to cpu_halted,
Cornelia Huck <=