Re: [PATCH v2] kvm: reuse per-vcpu stats fd to avoid vcpu interruption

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Marcelo,

On 16/6/23 19:44, Marcelo Tosatti wrote:
> 
> A regression has been detected in latency testing of KVM guests.
> More specifically, it was observed that the cyclictest
> numbers inside of an isolated vcpu (running on isolated pcpu) are:
>                                              
> # Max Latencies: 00090 00096 00141
>    
> Where a maximum of 50us is acceptable.
>                     
> The implementation of KVM_GET_STATS_FD uses run_on_cpu to query
> per vcpu statistics, which interrupts the vcpu (and is unnecessary).
>                  
> To fix this, open the per vcpu stats fd on vcpu initialization,
> and read from that fd from QEMU's main thread.
>               
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>


>   static void query_stats(StatsResultList **result, StatsTarget target,
> -                        strList *names, int stats_fd, Error **errp)
> +                        strList *names, int stats_fd, CPUState *cpu,
> +                        Error **errp)
>   {
>       struct kvm_stats_desc *kvm_stats_desc;
>       struct kvm_stats_header *kvm_stats_header;
> @@ -4096,7 +4099,7 @@ static void query_stats(StatsResultList **result, StatsTarget target,
>           break;
>       case STATS_TARGET_VCPU:
>           add_stats_entry(result, STATS_PROVIDER_KVM,
> -                        current_cpu->parent_obj.canonical_path,
> +                        cpu->parent_obj.canonical_path,

Can we get a NULL deref here ...

>                           stats_list);
>           break;
>       default:


>   static void query_stats_cb(StatsResultList **result, StatsTarget target,
> @@ -4180,7 +4180,7 @@ static void query_stats_cb(StatsResultList **result, StatsTarget target,
>               error_setg_errno(errp, errno, "KVM stats: ioctl failed");
>               return;
>           }
> -        query_stats(result, target, names, stats_fd, errp);
> +        query_stats(result, target, names, stats_fd, NULL, errp);

... from here?

>           close(stats_fd);
>           break;
>       }