在 2022/1/14 0:22, Markus Armbruster 写道:
Peter Xu <pet...@redhat.com> writes:
On Fri, Dec 31, 2021 at 12:36:40AM +0800, Hyman Huang wrote:
+struct {
+ DirtyLimitState *states;
+ int max_cpus;
+ unsigned long *bmap; /* running thread bitmap */
+ unsigned long nr;
+ QemuThread thread;
+} *dirtylimit_state;
+
+static bool dirtylimit_quit = true;
Again, I think "quit" is not a good wording to show "whether dirtylimit is in
service". How about "dirtylimit_global_enabled"?
You can actually use "dirtylimit_state" to show whether it's enabled already
(then drop the global value) since it's a pointer. It shouldn't need to be
init-once-for-all, but we can alloc the strucuture wAhen dirty limit enabled
globally, and destroy it (and reset it to NULL) when globally disabled.
Then "whether it's enabled" is simply to check "!!dirtylimit_state" under BQL.
Yes, checking pointer is fairly straightforword, but since dirtylimit thread
also access the dirtylimit_state when doing the limit, if we free
dirtylimit_state after last limited vcpu be canceled, dirtylimit thread
would crash when reference null pointer. And this method turn out to
introduce a mutex lock to protect dirtylimit_state, comparing with qatomic
operation, which is better ?
I don't see much difference here on using either atomic or mutex, because it's
not a hot path.
Quick interjection without having bothered to understand the details:
correct use of atomics and memory barriers is *much* harder than correct
use of locks. Stick to locks unless you *know* they impair performance.
Ok, i get it, i removed most of atomic operations in v11 and use the
lock instead. But still thanks for the advice :)
[...]
--
Best regard
Hyman Huang(黄勇)
