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. 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. Thanks, -- Peter Xu