Re: [Qemu-devel] [PATCH] rtc: placing RTC memory region outside BQL

[Gonglei (Arei)]

> -----Original Message-----
> From: Paolo Bonzini [mailto:address@hidden
> Sent: Thursday, February 01, 2018 10:24 PM
> To: Gonglei (Arei); address@hidden
> Cc: Huangweidong (C)
> Subject: Re: [PATCH] rtc: placing RTC memory region outside BQL
> 
> On 01/02/2018 08:47, Gonglei wrote:
> > As windows guest use rtc as the clock source device,
> > and access rtc frequently. Let's move the rtc memory
> > region outside BQL to decrease overhead for windows guests.
> >
> > strace -tt -p $1 -c -o result_$1.log &
> > sleep $2
> > pid=$(pidof strace)
> > kill $pid
> > cat result_$1.log
> >
> > Before appling this change:
> >
> > % time     seconds  usecs/call     calls    errors syscall
> > ------ ----------- ----------- --------- --------- ----------------
> >  93.87    0.119070          30      4000           ppoll
> >   3.27    0.004148           2      2038           ioctl
> >   2.66    0.003370           2      2014           futex
> >   0.09    0.000113           1       106           read
> >   0.09    0.000109           1       104           io_getevents
> >   0.02    0.000029           1        30           poll
> >   0.00    0.000000           0         1           write
> > ------ ----------- ----------- --------- --------- ----------------
> > 100.00    0.126839                  8293           total
> >
> > After appling the change:
> >
> > % time     seconds  usecs/call     calls    errors syscall
> > ------ ----------- ----------- --------- --------- ----------------
> >  92.86    0.067441          16      4094           ppoll
> >   4.85    0.003522           2      2136           ioctl
> >   1.17    0.000850           4       189           futex
> >   0.54    0.000395           2       202           read
> >   0.52    0.000379           2       202           io_getevents
> >   0.05    0.000037           1        30           poll
> > ------ ----------- ----------- --------- --------- ----------------
> > 100.00    0.072624                  6853           total
> >
> > The futex call number decreases ~90.6% on an idle windows 7 guest.
> >
> > Signed-off-by: Gonglei <address@hidden>
> > ---
> >  hw/timer/mc146818rtc.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> > index 35a05a6..d9d99c5 100644
> > --- a/hw/timer/mc146818rtc.c
> > +++ b/hw/timer/mc146818rtc.c
> > @@ -986,6 +986,7 @@ static void rtc_realizefn(DeviceState *dev, Error
> **errp)
> >      qemu_register_suspend_notifier(&s->suspend_notifier);
> >
> >      memory_region_init_io(&s->io, OBJECT(s), &cmos_ops, s, "rtc", 2);
> > +    memory_region_clear_global_locking(&s->io);
> >      isa_register_ioport(isadev, &s->io, base);
> >
> >      qdev_set_legacy_instance_id(dev, base, 3);
> >
> 
> This is not enough, you need to add a new lock or something like that.
> Otherwise two vCPUs can access the RTC together and make a mess.
> 

Hi Paolo,

Yes, that's true, although I have not encountered any problems yet.
Let me enhance it in v2.

Thanks,
-Gonglei