在 2022/1/4 10:32, Peter Xu 写道:
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.
If to use mutex and not overload BQL we can use a dirtylimit_mutex then before
referencing the pointer anywhere we need to fetch it, and release when sleep.
Ok, i'm already try this in the next version :)
The only thing confusing to me about the global variable way is having
quit=true as initial value, and clearing it when start. I think it'll work,
but just reads weird.
How about having "enabled" and "quit" as a normal threaded app? Then:
- When init: enabled=0 quit=0
- When start: enabled=1 quit=0
- When stop
- main thread set enabled=1 quit=1
- dirtylimit sees quit=1, goes to join()
- main thread reset enable=quit=0
dirtylimit_in_service() should reference "enabled", and "quit" should be only
used for sync on exit.
Ok, no problem
Thanks,
--
Best regard
Hyman Huang(黄勇)
