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 48454C4332F for ; Tue, 13 Dec 2022 14:31:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235831AbiLMObU (ORCPT ); Tue, 13 Dec 2022 09:31:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52954 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235794AbiLMObL (ORCPT ); Tue, 13 Dec 2022 09:31:11 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8CCDE1F9CE for ; Tue, 13 Dec 2022 06:31:10 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 247896155A for ; Tue, 13 Dec 2022 14:31:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BCFAAC433EF; Tue, 13 Dec 2022 14:31:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1670941869; bh=ZJunwccLyP7VfyFolEsIq9RAMaZzY1QZT7UToBTpj8Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Pbvo7a7K6YUmDxML3SyCX8Ld7C0Cq5pxVmHq7XJ4crSB3niF2OVpUifdz2U4pok1g Ji4xJVTLktet6hT2LsWAo9IHn4hTENPIKjWDmQKkX9QypqT6cGwhM2NKEQRxVYQFT0 cpZMBy+F0jPXT3QB91hTkbiYzxBM0eWHyFaFd9wY= Date: Tue, 13 Dec 2022 15:31:06 +0100 From: Greg Kroah-Hartman To: Phil Auld Cc: Vishal Chourasia , Peter Zijlstra , linux-kernel@vger.kernel.org, mingo@redhat.com, vincent.guittot@linaro.org, vschneid@redhat.com, srikar@linux.vnet.ibm.com, sshegde@linux.ibm.com, linuxppc-dev@lists.ozlabs.org, ritesh.list@gmail.com, aneesh.kumar@linux.ibm.com Subject: Re: sched/debug: CPU hotplug operation suffers in a large cpu systems Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 13, 2022 at 08:22:58AM -0500, Phil Auld wrote: > On Tue, Dec 13, 2022 at 07:23:54AM +0100 Greg Kroah-Hartman wrote: > > On Mon, Dec 12, 2022 at 02:17:58PM -0500, Phil Auld wrote: > > > Hi, > > > > > > On Tue, Nov 08, 2022 at 01:24:39PM +0100 Greg Kroah-Hartman wrote: > > > > On Tue, Nov 08, 2022 at 03:30:46PM +0530, Vishal Chourasia wrote: > > > > > > > > > > Thanks Greg & Peter for your direction. > > > > > > > > > > While we pursue the idea of having debugfs based on kernfs, we thought about > > > > > having a boot time parameter which would disable creating and updating of the > > > > > sched_domain debugfs files and this would also be useful even when the kernfs > > > > > solution kicks in, as users who may not care about these debugfs files would > > > > > benefit from a faster CPU hotplug operation. > > > > > > > > Ick, no, you would be adding a new user/kernel api that you will be > > > > required to support for the next 20+ years. Just to get over a > > > > short-term issue before you solve the problem properly. > > > > > > I'm not convinced moving these files from debugfs to kernfs is the right > > > fix. That will take it from ~50 back to ~20 _minutes_ on these systems. > > > I don't think either of those numbers is reasonable. > > > > > > The issue as I see it is the full rebuild for every change with no way to > > > batch the changes. How about something like the below? > > > > > > This puts the domains/* files under the sched_verbose flag. About the only > > > thing under that flag now are the detailed topology discovery printks anyway > > > so this fits together nicely. > > > > > > This way the files would be off by default (assuming you don't boot with > > > sched_verbose) and can be created at runtime by enabling verbose. Multiple > > > changes could also be batched by disabling/makeing changes/re-enabling. > > > > > > It does not create a new API, uses one that is already there. > > > > The idea seems good, the implementation might need a bit of work :) > > More than the one comment below? Let me know. No idea, resubmit a working patch and I'll review it properly :) > > > + r = kstrtobool_from_user(ubuf, cnt, &bv); > > > + if (!r) { > > > + mutex_lock(&sched_domains_mutex); > > > + r = debugfs_file_get(dentry); > > > + if (unlikely(r)) > > > + return r; > > > + sched_debug_verbose = bv; > > > + debugfs_file_put(dentry); > > > > Why the get/put of the debugfs dentry? for just this single value? > > That's what debugfs_file_write_bool() does, which is where I got that since > that's really what this is doing. I couldn't see a good way to make this > just call that. > > I suppose the get/put may not be needed since the only way this should > go away is under that mutex too. Yes, it should not be needed. > ... erm, yeah, that return is a problem ... I'll fix that. > > Also, this was originally on v6.1-rc7. I can rebase when I repost but I > didn't want to do it on a random commit so I picked (at the time) the latest > tag. Should I just use the head of Linux? Yes, or linux-next. thanks, greg k-h 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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (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 16BC2C10F1D for ; Tue, 13 Dec 2022 14:32:11 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4NWgtf1Gg1z3cMb for ; Wed, 14 Dec 2022 01:32:10 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.a=rsa-sha256 header.s=korg header.b=Pbvo7a7K; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=linuxfoundation.org (client-ip=139.178.84.217; helo=dfw.source.kernel.org; envelope-from=gregkh@linuxfoundation.org; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.a=rsa-sha256 header.s=korg header.b=Pbvo7a7K; dkim-atps=neutral Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4NWgsZ1pGgz3bXC for ; Wed, 14 Dec 2022 01:31:12 +1100 (AEDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 0CA186155B; Tue, 13 Dec 2022 14:31:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BCFAAC433EF; Tue, 13 Dec 2022 14:31:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1670941869; bh=ZJunwccLyP7VfyFolEsIq9RAMaZzY1QZT7UToBTpj8Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Pbvo7a7K6YUmDxML3SyCX8Ld7C0Cq5pxVmHq7XJ4crSB3niF2OVpUifdz2U4pok1g Ji4xJVTLktet6hT2LsWAo9IHn4hTENPIKjWDmQKkX9QypqT6cGwhM2NKEQRxVYQFT0 cpZMBy+F0jPXT3QB91hTkbiYzxBM0eWHyFaFd9wY= Date: Tue, 13 Dec 2022 15:31:06 +0100 From: Greg Kroah-Hartman To: Phil Auld Subject: Re: sched/debug: CPU hotplug operation suffers in a large cpu systems Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: ritesh.list@gmail.com, vschneid@redhat.com, vincent.guittot@linaro.org, srikar@linux.vnet.ibm.com, Peter Zijlstra , aneesh.kumar@linux.ibm.com, Vishal Chourasia , linux-kernel@vger.kernel.org, sshegde@linux.ibm.com, mingo@redhat.com, linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Tue, Dec 13, 2022 at 08:22:58AM -0500, Phil Auld wrote: > On Tue, Dec 13, 2022 at 07:23:54AM +0100 Greg Kroah-Hartman wrote: > > On Mon, Dec 12, 2022 at 02:17:58PM -0500, Phil Auld wrote: > > > Hi, > > > > > > On Tue, Nov 08, 2022 at 01:24:39PM +0100 Greg Kroah-Hartman wrote: > > > > On Tue, Nov 08, 2022 at 03:30:46PM +0530, Vishal Chourasia wrote: > > > > > > > > > > Thanks Greg & Peter for your direction. > > > > > > > > > > While we pursue the idea of having debugfs based on kernfs, we thought about > > > > > having a boot time parameter which would disable creating and updating of the > > > > > sched_domain debugfs files and this would also be useful even when the kernfs > > > > > solution kicks in, as users who may not care about these debugfs files would > > > > > benefit from a faster CPU hotplug operation. > > > > > > > > Ick, no, you would be adding a new user/kernel api that you will be > > > > required to support for the next 20+ years. Just to get over a > > > > short-term issue before you solve the problem properly. > > > > > > I'm not convinced moving these files from debugfs to kernfs is the right > > > fix. That will take it from ~50 back to ~20 _minutes_ on these systems. > > > I don't think either of those numbers is reasonable. > > > > > > The issue as I see it is the full rebuild for every change with no way to > > > batch the changes. How about something like the below? > > > > > > This puts the domains/* files under the sched_verbose flag. About the only > > > thing under that flag now are the detailed topology discovery printks anyway > > > so this fits together nicely. > > > > > > This way the files would be off by default (assuming you don't boot with > > > sched_verbose) and can be created at runtime by enabling verbose. Multiple > > > changes could also be batched by disabling/makeing changes/re-enabling. > > > > > > It does not create a new API, uses one that is already there. > > > > The idea seems good, the implementation might need a bit of work :) > > More than the one comment below? Let me know. No idea, resubmit a working patch and I'll review it properly :) > > > + r = kstrtobool_from_user(ubuf, cnt, &bv); > > > + if (!r) { > > > + mutex_lock(&sched_domains_mutex); > > > + r = debugfs_file_get(dentry); > > > + if (unlikely(r)) > > > + return r; > > > + sched_debug_verbose = bv; > > > + debugfs_file_put(dentry); > > > > Why the get/put of the debugfs dentry? for just this single value? > > That's what debugfs_file_write_bool() does, which is where I got that since > that's really what this is doing. I couldn't see a good way to make this > just call that. > > I suppose the get/put may not be needed since the only way this should > go away is under that mutex too. Yes, it should not be needed. > ... erm, yeah, that return is a problem ... I'll fix that. > > Also, this was originally on v6.1-rc7. I can rebase when I repost but I > didn't want to do it on a random commit so I picked (at the time) the latest > tag. Should I just use the head of Linux? Yes, or linux-next. thanks, greg k-h