Messages in this thread | | | Date | Mon, 14 Sep 2015 23:47:23 -0400 | From | Waiman Long <> | Subject | Re: [PATCH v6 4/6] locking/pvqspinlock: Collect slowpath lock statistics |
| |
On 09/14/2015 05:41 PM, Davidlohr Bueso wrote: > On Mon, 14 Sep 2015, Waiman Long wrote: > >> You can't use debugfs if we want to have per-cpu stats. We will have >> to use sysfs instead. This will require more code changes. It is >> certainly doable, but we have to choose between simplicity and >> performance overhead. Right now, I am assuming that lock PV lockstat >> is used primarily for debugging purpose and won't be enabled on >> production system. If we want to have this capability in production >> systems, we will certainly need to change it to per-cpu stats and use >> sysfs instead. >> >> The original PV ticketlock code used debugfs and I was just following >> its footstep. Do you think it is worthwhile to have this capability >> available on production system by default? > > If we can prove that the overhead is small enough, and do it correctly > (ie see how we do vmstats), it would be _very_ useful data to have > enabled by default for debugging performance issues; methinks. But right > now we have nowhere near that kind of data, not even with this atomic > variant -- although I recall you did mention a workload in a previous > iteration (which would be good to have in the changelog).
Using the per-cpu stats, the overhead should be pretty small as atomic instructions are not needed. I would probably need to encapsulate the stat code into another header file (e.g. qspinlock_pvstat.h) to avoid making the qspinlock_paravirt.h too complex. This will probably be a separate patch once this patch series can be merged.
Cheers, Longman
| |