From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4A392C433EF for ; Wed, 9 Feb 2022 19:22:59 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A63FF10E57C; Wed, 9 Feb 2022 19:22:58 +0000 (UTC) Received: from mail-lj1-f181.google.com (mail-lj1-f181.google.com [209.85.208.181]) by gabe.freedesktop.org (Postfix) with ESMTPS id 085FD10E57C for ; Wed, 9 Feb 2022 19:22:58 +0000 (UTC) Received: by mail-lj1-f181.google.com with SMTP id h18so937346lja.13 for ; Wed, 09 Feb 2022 11:22:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TAb9RMF5rdP3oF08yRzn8MffJjQ5kNb8IB5u5y/hypk=; b=iRYeSBTHbemMrN8mpC0VrQO7Yj4IBkpZGy+xPCDIPZ/SIaLatzC200NR8cxDnWJtDc sTA1vjv9WnCtHKyVW6Ztp4F40tvlNN+kkK+uALqs4Q/V2Qnw2eYN1fr+MRoqdYCaojMf KD4eBMPqcbGei93c5Gyn/X2Ixu2+HnMdiXqzh5PP/yG2GIoq3A1NdzJRZePDCDERVTl8 SpqGAK3A13H26WgYf2gcSkJAe9ZyBnGE2uBdTJAFcjA1SE/CBcmUOpIxMlN1YyV9vKM1 +VogPCzebBhgpjjp+SyTabo07QtGoFE7BtZqfO8rLRm7dVUTff+oRSrejFYqJnNtFilf s88g== X-Gm-Message-State: AOAM531xfmBm3dvmMFy4akgr1uSc19jildAum5rgAzvhfBim5T7wRsov iRuJZ1ivBlR3KE9ob7vX7JSXCsxJnaTbSjrAv+8= X-Google-Smtp-Source: ABdhPJw08QD4wUSfZkYC8eoOu9YPXO1KD3DoKCorkCNdm3c9GLfZqlvWEz+RohSDhEAzIwcPChf/Tq1QQHAWfxCujNM= X-Received: by 2002:a2e:5352:: with SMTP id t18mr2459533ljd.241.1644434575676; Wed, 09 Feb 2022 11:22:55 -0800 (PST) MIME-Version: 1.0 References: <20220208184208.79303-1-namhyung@kernel.org> <20220209090908.GK23216@worktop.programming.kicks-ass.net> <24fe6a08-5931-8e8d-8d77-459388c4654e@redhat.com> <919214156.50301.1644431371345.JavaMail.zimbra@efficios.com> <69e5f778-8715-4acf-c027-58b6ec4a9e77@redhat.com> In-Reply-To: <69e5f778-8715-4acf-c027-58b6ec4a9e77@redhat.com> From: Namhyung Kim Date: Wed, 9 Feb 2022 11:22:44 -0800 Message-ID: To: Waiman Long Content-Type: text/plain; charset="UTF-8" Subject: Re: [Intel-gfx] [RFC 00/12] locking: Separate lock tracepoints from lockdep/lock_stat (v1) X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: rcu , "Paul E. McKenney" , Peter Zijlstra , Boqun Feng , linux-kernel , rostedt , Radoslaw Burny , Byungchul Park , Mathieu Desnoyers , intel-gfx@lists.freedesktop.org, Tejun Heo , cgroups , Thomas Gleixner , Will Deacon , Ingo Molnar , linux-btrfs Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Hello, On Wed, Feb 9, 2022 at 11:02 AM Waiman Long wrote: > > On 2/9/22 13:29, Mathieu Desnoyers wrote: > > ----- On Feb 9, 2022, at 1:19 PM, Waiman Long longman@redhat.com wrote: > > > >> On 2/9/22 04:09, Peter Zijlstra wrote: > >>> On Tue, Feb 08, 2022 at 10:41:56AM -0800, Namhyung Kim wrote: > >>> > >>>> Eventually I'm mostly interested in the contended locks only and I > >>>> want to reduce the overhead in the fast path. By moving that, it'd be > >>>> easy to track contended locks with timing by using two tracepoints. > >>> So why not put in two new tracepoints and call it a day? > >>> > >>> Why muck about with all that lockdep stuff just to preserve the name > >>> (and in the process continue to blow up data structures etc..). This > >>> leaves distros in a bind, will they enable this config and provide > >>> tracepoints while bloating the data structures and destroying things > >>> like lockref (which relies on sizeof(spinlock_t)), or not provide this > >>> at all. > >>> > >>> Yes, the name is convenient, but it's just not worth it IMO. It makes > >>> the whole proposition too much of a trade-off. > >>> > >>> Would it not be possible to reconstruct enough useful information from > >>> the lock callsite? > >>> > >> I second that as I don't want to see the size of a spinlock exceeds 4 > >> bytes in a production system. > >> > >> Instead of storing additional information (e.g. lock name) directly into > >> the lock itself. Maybe we can store it elsewhere and use the lock > >> address as the key to locate it in a hash table. We can certainly extend > >> the various lock init functions to do that. It will be trickier for > >> statically initialized locks, but we can probably find a way to do that too. > > If we go down that route, it would be nice if we can support a few different > > use-cases for various tracers out there. > > > > One use-case (a) requires the ability to query the lock name based on its address as key. > > For this a hash table is a good fit. This would allow tracers like ftrace to > > output lock names in its human-readable output which is formatted within the kernel. > > > > Another use-case (b) is to be able to "dump" the lock { name, address } tuples > > into the trace stream (we call this statedump events in lttng), and do the > > translation from address to name at post-processing. This simply requires > > that this information is available for iteration for both the core kernel > > and module locks, so the tracer can dump this information on trace start > > and module load. > > > > Use-case (b) is very similar to what is done for the kernel tracepoints. Based > > on this, implementing the init code that iterates on those sections and populates > > a hash table for use-case (a) should be easy enough. > > Yes, that are good use cases for this type of functionality. I do need > to think about how to do it for statically initialized lock first. Thank you all for the review and good suggestions. I'm also concerning dynamic allocated locks in a data structure. If we keep the info in a hash table, we should delete it when the lock is gone. I'm not sure we have a good place to hook it up all. Thanks, Namhyung From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5C0EBC433F5 for ; Wed, 9 Feb 2022 19:31:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235303AbiBITbu (ORCPT ); Wed, 9 Feb 2022 14:31:50 -0500 Received: from gmail-smtp-in.l.google.com ([23.128.96.19]:35180 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235883AbiBITbf (ORCPT ); Wed, 9 Feb 2022 14:31:35 -0500 Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DF6B9C1DC06A; Wed, 9 Feb 2022 11:22:57 -0800 (PST) Received: by mail-lj1-f175.google.com with SMTP id t14so4842705ljh.8; Wed, 09 Feb 2022 11:22:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TAb9RMF5rdP3oF08yRzn8MffJjQ5kNb8IB5u5y/hypk=; b=1MyEJIKn09+g71ZGo4sfbjB6JpK4pIgqZv140B6IXhwCbq87UNojDO+Z4kzQExxmCl lPVaxagvpIkvOihaA6lfkZG8dRJ0q4NWO98Ox7WehBxX4fXR71PaO3vLUXneWIT23MkM yKzhQYWWRwRBBfM1ttRzzn1bbJRQ5yrR/R6lJs0xIJyQHsdJxqSc/9LOAeIBqjDvaBMs Y1qGCcdIbGof/eIJ/gK03TgSP1LZBUjF0iY135Ic9XodIsoLsHhg1TtDZWiMI74NryOP L8KUDPYF5n6aYmvHlpy1ZPii2hQ4Ulk5zhRYhQ7Z5L/MlvTX6nrCGtCcEXlFg1a0cCNe ZhHA== X-Gm-Message-State: AOAM532H1fR2XRsXn+PrPcWmvfr3Mhxti0GS5WSfekNQkvgOaBzwTsWb iW+YOsl2iX9VbWUrBlIJvtCgyv+pwQHJWBxPGVI= X-Google-Smtp-Source: ABdhPJw08QD4wUSfZkYC8eoOu9YPXO1KD3DoKCorkCNdm3c9GLfZqlvWEz+RohSDhEAzIwcPChf/Tq1QQHAWfxCujNM= X-Received: by 2002:a2e:5352:: with SMTP id t18mr2459533ljd.241.1644434575676; Wed, 09 Feb 2022 11:22:55 -0800 (PST) MIME-Version: 1.0 References: <20220208184208.79303-1-namhyung@kernel.org> <20220209090908.GK23216@worktop.programming.kicks-ass.net> <24fe6a08-5931-8e8d-8d77-459388c4654e@redhat.com> <919214156.50301.1644431371345.JavaMail.zimbra@efficios.com> <69e5f778-8715-4acf-c027-58b6ec4a9e77@redhat.com> In-Reply-To: <69e5f778-8715-4acf-c027-58b6ec4a9e77@redhat.com> From: Namhyung Kim Date: Wed, 9 Feb 2022 11:22:44 -0800 Message-ID: Subject: Re: [RFC 00/12] locking: Separate lock tracepoints from lockdep/lock_stat (v1) To: Waiman Long Cc: Mathieu Desnoyers , Peter Zijlstra , Ingo Molnar , Will Deacon , Boqun Feng , linux-kernel , Thomas Gleixner , rostedt , Byungchul Park , Radoslaw Burny , Tejun Heo , rcu , cgroups , linux-btrfs , intel-gfx@lists.freedesktop.org, "Paul E. McKenney" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org Hello, On Wed, Feb 9, 2022 at 11:02 AM Waiman Long wrote: > > On 2/9/22 13:29, Mathieu Desnoyers wrote: > > ----- On Feb 9, 2022, at 1:19 PM, Waiman Long longman@redhat.com wrote: > > > >> On 2/9/22 04:09, Peter Zijlstra wrote: > >>> On Tue, Feb 08, 2022 at 10:41:56AM -0800, Namhyung Kim wrote: > >>> > >>>> Eventually I'm mostly interested in the contended locks only and I > >>>> want to reduce the overhead in the fast path. By moving that, it'd be > >>>> easy to track contended locks with timing by using two tracepoints. > >>> So why not put in two new tracepoints and call it a day? > >>> > >>> Why muck about with all that lockdep stuff just to preserve the name > >>> (and in the process continue to blow up data structures etc..). This > >>> leaves distros in a bind, will they enable this config and provide > >>> tracepoints while bloating the data structures and destroying things > >>> like lockref (which relies on sizeof(spinlock_t)), or not provide this > >>> at all. > >>> > >>> Yes, the name is convenient, but it's just not worth it IMO. It makes > >>> the whole proposition too much of a trade-off. > >>> > >>> Would it not be possible to reconstruct enough useful information from > >>> the lock callsite? > >>> > >> I second that as I don't want to see the size of a spinlock exceeds 4 > >> bytes in a production system. > >> > >> Instead of storing additional information (e.g. lock name) directly into > >> the lock itself. Maybe we can store it elsewhere and use the lock > >> address as the key to locate it in a hash table. We can certainly extend > >> the various lock init functions to do that. It will be trickier for > >> statically initialized locks, but we can probably find a way to do that too. > > If we go down that route, it would be nice if we can support a few different > > use-cases for various tracers out there. > > > > One use-case (a) requires the ability to query the lock name based on its address as key. > > For this a hash table is a good fit. This would allow tracers like ftrace to > > output lock names in its human-readable output which is formatted within the kernel. > > > > Another use-case (b) is to be able to "dump" the lock { name, address } tuples > > into the trace stream (we call this statedump events in lttng), and do the > > translation from address to name at post-processing. This simply requires > > that this information is available for iteration for both the core kernel > > and module locks, so the tracer can dump this information on trace start > > and module load. > > > > Use-case (b) is very similar to what is done for the kernel tracepoints. Based > > on this, implementing the init code that iterates on those sections and populates > > a hash table for use-case (a) should be easy enough. > > Yes, that are good use cases for this type of functionality. I do need > to think about how to do it for statically initialized lock first. Thank you all for the review and good suggestions. I'm also concerning dynamic allocated locks in a data structure. If we keep the info in a hash table, we should delete it when the lock is gone. I'm not sure we have a good place to hook it up all. Thanks, Namhyung From mboxrd@z Thu Jan 1 00:00:00 1970 From: Namhyung Kim Subject: Re: [RFC 00/12] locking: Separate lock tracepoints from lockdep/lock_stat (v1) Date: Wed, 9 Feb 2022 11:22:44 -0800 Message-ID: References: <20220208184208.79303-1-namhyung@kernel.org> <20220209090908.GK23216@worktop.programming.kicks-ass.net> <24fe6a08-5931-8e8d-8d77-459388c4654e@redhat.com> <919214156.50301.1644431371345.JavaMail.zimbra@efficios.com> <69e5f778-8715-4acf-c027-58b6ec4a9e77@redhat.com> Mime-Version: 1.0 Return-path: In-Reply-To: <69e5f778-8715-4acf-c027-58b6ec4a9e77@redhat.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Waiman Long Cc: rcu , "Paul E. McKenney" , Peter Zijlstra , Boqun Feng , linux-kernel , rostedt , Radoslaw Burny , Byungchul Park , Mathieu Desnoyers , intel-gfx@lists.freedesktop.org, Tejun Heo , cgroups , Thomas Gleixner , Will Deacon , Ingo Molnar , linux-btrfs Hello, On Wed, Feb 9, 2022 at 11:02 AM Waiman Long wrote: > > On 2/9/22 13:29, Mathieu Desnoyers wrote: > > ----- On Feb 9, 2022, at 1:19 PM, Waiman Long longman@redhat.com wrote: > > > >> On 2/9/22 04:09, Peter Zijlstra wrote: > >>> On Tue, Feb 08, 2022 at 10:41:56AM -0800, Namhyung Kim wrote: > >>> > >>>> Eventually I'm mostly interested in the contended locks only and I > >>>> want to reduce the overhead in the fast path. By moving that, it'd be > >>>> easy to track contended locks with timing by using two tracepoints. > >>> So why not put in two new tracepoints and call it a day? > >>> > >>> Why muck about with all that lockdep stuff just to preserve the name > >>> (and in the process continue to blow up data structures etc..). This > >>> leaves distros in a bind, will they enable this config and provide > >>> tracepoints while bloating the data structures and destroying things > >>> like lockref (which relies on sizeof(spinlock_t)), or not provide this > >>> at all. > >>> > >>> Yes, the name is convenient, but it's just not worth it IMO. It makes > >>> the whole proposition too much of a trade-off. > >>> > >>> Would it not be possible to reconstruct enough useful information from > >>> the lock callsite? > >>> > >> I second that as I don't want to see the size of a spinlock exceeds 4 > >> bytes in a production system. > >> > >> Instead of storing additional information (e.g. lock name) directly into > >> the lock itself. Maybe we can store it elsewhere and use the lock > >> address as the key to locate it in a hash table. We can certainly extend > >> the various lock init functions to do that. It will be trickier for > >> statically initialized locks, but we can probably find a way to do that too. > > If we go down that route, it would be nice if we can support a few different > > use-cases for various tracers out there. > > > > One use-case (a) requires the ability to query the lock name based on its address as key. > > For this a hash table is a good fit. This would allow tracers like ftrace to > > output lock names in its human-readable output which is formatted within the kernel. > > > > Another use-case (b) is to be able to "dump" the lock { name, address } tuples > > into the trace stream (we call this statedump events in lttng), and do the > > translation from address to name at post-processing. This simply requires > > that this information is available for iteration for both the core kernel > > and module locks, so the tracer can dump this information on trace start > > and module load. > > > > Use-case (b) is very similar to what is done for the kernel tracepoints. Based > > on this, implementing the init code that iterates on those sections and populates > > a hash table for use-case (a) should be easy enough. > > Yes, that are good use cases for this type of functionality. I do need > to think about how to do it for statically initialized lock first. Thank you all for the review and good suggestions. I'm also concerning dynamic allocated locks in a data structure. If we keep the info in a hash table, we should delete it when the lock is gone. I'm not sure we have a good place to hook it up all. Thanks, Namhyung