From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752746AbbIODr3 (ORCPT ); Mon, 14 Sep 2015 23:47:29 -0400 Received: from g2t4621.austin.hp.com ([15.73.212.80]:38670 "EHLO g2t4621.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752010AbbIODr2 (ORCPT ); Mon, 14 Sep 2015 23:47:28 -0400 Message-ID: <55F794CB.5040707@hpe.com> Date: Mon, 14 Sep 2015 23:47:23 -0400 From: Waiman Long User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130109 Thunderbird/10.0.12 MIME-Version: 1.0 To: Davidlohr Bueso CC: Peter Zijlstra , Ingo Molnar , Thomas Gleixner , "H. Peter Anvin" , x86@kernel.org, linux-kernel@vger.kernel.org, Scott J Norton , Douglas Hatch Subject: Re: [PATCH v6 4/6] locking/pvqspinlock: Collect slowpath lock statistics References: <1441996658-62854-1-git-send-email-Waiman.Long@hpe.com> <1441996658-62854-5-git-send-email-Waiman.Long@hpe.com> <20150911231355.GD19736@linux-q0g1.site> <55F6E6F2.10102@hpe.com> <20150914214105.GH19736@linux-q0g1.site> In-Reply-To: <20150914214105.GH19736@linux-q0g1.site> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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