Re: [PATCH v9 3/3] cpus-common: implement dirty page limit on vCPU

[Peter Xu <peterx@redhat.com>]

On Mon, Dec 06, 2021 at 11:19:21PM +0800, Hyman wrote:
> > > +    if (has_cpu_index) {
> > > +        info = dirtylimit_query_vcpu(cpu_index);
> > > +        QAPI_LIST_APPEND(tail, info);
> > > +    } else {
> > > +        CPUState *cpu;
> > > +        CPU_FOREACH(cpu) {
> > > +            if (!cpu->unplug) {
> > > +                info = dirtylimit_query_vcpu(cpu->cpu_index);
> > > +                QAPI_LIST_APPEND(tail, info);
> > > +            }
> > 
> > There're special handling for unplug in a few places.  Could you explain why?
> > E.g. if the vcpu is unplugged then dirty rate is zero, then it seems fine to
> > even keep it there?
> > The dirty limit logic only allow plugged vcpu to be enabled throttle, so
> that the "dirtylimit-{cpu-index}" thread don't need to be forked and we can
> save the overhead. So in query logic we just filter the unplugged vcpu.

I've commented similarly in the other thread - please consider not using NVCPU
threads only for vcpu throttling, irrelevant of vcpu hot plug/unplug.

Per-vcpu throttle is totally not a cpu intensive workload, 1 thread should be
enough globally, imho.

A guest with hundreds of vcpus are becoming more common, we shouldn't waste OS
thread resources just for this.

> 
> Another reason is that i thought it could make user confused when we return
> the unplugged vcpu dirtylimit info. Uh, in most time of vm lifecycle,
> hotplugging vcpu may never happen.

I just think if plug/unplug does not affect the throttle logic then we should
treat them the same, it avoids unnecessary special care on those vcpus too.

-- 
Peter Xu