Re: [PATCH v1 3/8] migration: Introduce dirty-limit capability

[Markus Armbruster <armbru@redhat.com>]

Hyman Huang <huangy81@chinatelecom.cn> writes:

> 在 2022/9/5 17:32, Markus Armbruster 写道:
>> Hyman Huang <huangy81@chinatelecom.cn> writes:
>> 
>>> 在 2022/9/2 16:07, Markus Armbruster 写道:
>>>> huangy81@chinatelecom.cn writes:
>>>>
>>>>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>>>
>>>>> Introduce migration dirty-limit capability, which can
>>>>> be turned on before live migration and limit dirty
>>>>> page rate durty live migration.
>>>>>
>>>>> Introduce migrate_dirty_limit function to help check
>>>>> if dirty-limit capability enabled during live migration.
>>>>>
>>>>> Meanwhile, refactor vcpu_dirty_rate_stat_collect
>>>>> so that period can be configured instead of hardcoded.
>>>>>
>>>>> dirty-limit capability is kind of like auto-converge
>>>>> but using dirty limit instead of traditional cpu-throttle
>>>>> to throttle guest down. To enable this feature, turn on
>>>>> the dirty-limit capability before live migration using
>>>>> migratioin-set-capabilities, and set the parameters
>>>>
>>>> migrate-set-capabilities
>>>>
>>>>> "x-vcpu-dirty-limit-period", "vcpu-dirty-limit" suitably
>>>>
>>>> "x-vcpu-dirty-limit"
>>>>
>>>>> to speed up convergence.
>>>>>
>>>>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>>>
>>>> Hmm.  You make dirty-limiting as a whole a stable interface (evidence:
>>>> capability "dirty-limit" is stable), but keep its two parameters
>>>> unstable.  Rationale behind that?
>>>>
>>> Thanks Markus's comments. :)
>>>
>>> x-vcpu-dirty-limit-period is an experimental parameter indeed, as to x-vcpu-dirty-limit, i think it's resonable to be a stable parameter.
>>> These 2 parameters are introduced first time and none of them suffer heavily tests, so i also made vcpu-dirty-limit experimental last version.
>>>
>>> For dirty-limit interface, it improves the vCPU computational performance during migration indeed(see the test results in cover
>>> letter), so it sounds ok to be an stable interface.
>>>
>>> The 'x-vcpu-dirty-limit-period' parameter can be dropped, IMHO, after being proved insignificant for migration in the future, and meanwhile,
>>> x-vcpu-dirty-limit be made stable.
>>>
>>> Since I don't have much experience to introducing fresh new interface,
>>> any suggestions are welcome.
>> Is the new interface fit for purpose without use of any experimental
>> parameter?
>>  > If the answer is something like "command dirty-limit improves things
>> even without use of experimental parameters, but using them may well
>> improve things more (but we need more testing to know for sure)", then
>> your current use of 'unstable' may make sense.
>> 
> Yes, with the default value of parameter，the new interface works ok and improve performance.
>
> For x-vcpu-dirty-limit, we provide it because user may not want virtual CPU throttle heavily, x-vcpu-dirty-limit is kind of like 
> cpu-throttle-percentage, which is used to setup the threshold when making guest down.
>
> For x-vcpu-dirty-limit-period, it is just as you said: "command dirty-limit improves things even without use of experimental parameters, 
> but using them may wellimprove things more (but we need more testing to know for sure)"
>
> So, should i make x-vcpu-dirty-limit non-experimental next version?

I think this depends on what exactly you want to signal to users.

Your current patch has command dirty-limit stable and the parameters
unstable.  This signals "go ahead and use dirty-limit, don't worry about
the parameters; even if they become stable later, using just dirty-limit
will remain okay."

If you keep the command unstable as well, you signal the entire
interface isn't quite baked, yet.  That's a much weaker proposition.
So weak in fact that you cannot go wrong :)

In short, it boils down to whether you want to encourage use of a part
of the evolving interface *now*.  Make that part stable.  Requires
confidence in that part, obviously.