On Sat, Dec 16, 2017 at 09:11:08AM +0000, Stefan Hajnoczi wrote: > On Sat, Dec 16, 2017 at 02:12:11PM +0800, Peter Xu wrote: > > On Wed, Dec 13, 2017 at 06:43:58PM +0000, Stefan Hajnoczi wrote: > > > On Tue, Dec 05, 2017 at 01:51:49PM +0800, Peter Xu wrote: > > > > Monitor code now can be run in more than one thread. Let it be thread > > > > safe when accessing suspend_cnt counter. > > > > > > Please add doc comments to the functions explaining which thread they > > > may be called from (especially public functions). Without this > > > information other people will have a hard time keeping code thread-safe. > > > > The two public functions would be monitor_resume() and > > monitor_suspend(). IIUC as long as these functions are using atomic > > operations correctly they should be able to be called anywhere? > > > > Could you help point out what I missed? > > I don't mean just the public functions. How are other people supposed > to know which monitor function executes in which thread without first > studying all the monitor code? This is where comments, naming schemes, > or grouping functions helps. > > Not every function needs to be documented in detail, but there needs to > be some structure to help people understand what happens in the IOThread > and what happens in the main loop. > > Just adding threading to the existing code without leaving clues for > people who come after you is likely to result in future bugs.
Yes, I fully agree with you that we need to do better commenting when doing things like this. I am just not sure what you would prefer me to add since AFAICT these functions (after with the atomic protections) are really free to be called anywhere, and also in any thread (even newly created threads rather than main thread or monitor IO thread), and they are free of locks too, like monitor_lock. Here I only mean monitor_suspend() and monitor_resume() for sure. For the rest of functions, IMHO since they are internal to monitor code so people who will call them do need to know something about monitors. I would be more appreciated if you can provide some examples here to show what you mean. Thanks, -- Peter Xu
