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
