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

[Paolo Bonzini]

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.

Paolo