On Wed, 19 Jul 2023, Chuck Lever III wrote: > > > On Jul 18, 2023, at 9:16 PM, NeilBrown <neilb@xxxxxxx> wrote: > > > > On Tue, 18 Jul 2023, Chuck Lever wrote: > >> On Tue, Jul 18, 2023 at 04:38:08PM +1000, NeilBrown wrote: > >>> No callers of svc_pool_wake_idle_thread() care which thread was woken - > >>> except one that wants to trace the wakeup. For now we drop that > >>> tracepoint. > >> > >> That's an important tracepoint, IMO. > >> > >> It might be better to have svc_pool_wake_idle_thread() return void > >> right from it's introduction, and move the tracepoint into that > >> function. I can do that and respin if you agree. > > > > Mostly I agree. > > > > It isn't clear to me how you would handle trace_svc_xprt_enqueue(), > > as there would be no code that can see both the trigger xprt, and the > > woken rqst. > > > > I also wonder if having the trace point when the wake-up is requested > > makes any sense, as there is no guarantee that thread with handle that > > xprt. > > > > Maybe the trace point should report when the xprt is dequeued. i.e. > > maybe trace_svc_pool_awoken() should report the pid, and we could have > > trace_svc_xprt_enqueue() only report the xprt, not the rqst. > > I'll come up with something that rearranges the tracepoints so that > svc_pool_wake_idle_thread() can return void. My current draft code has svc_pool_wake_idle_thread() returning bool - if it found something to wake up - purely for logging. I think it is worth logging whether an event triggered a wake up or not, and which event did that. I'm less you that the pid is relevant. But as you say - this will probably become clearer as the code settles down. Thanks, NeilBrown > > svc_pool_wake_idle_thread() can save the waker's PID in svc_rqst > somewhere, for example. The dequeue tracepoint can then report that > (if it's still interesting when we're all done with this work). > > > -- > Chuck Lever > > >
