From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CB44211CA1; Wed, 21 Jun 2023 13:17:33 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 171A6199E; Wed, 21 Jun 2023 06:17:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1687353450; x=1718889450; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=9eMWiG7JI6/9oPvFtdl/3iOGYIkGUexXwjKhm4v9hgQ=; b=bCKCupaYl+EEOS5QZZqFy7/P+ML1IyBgPHBQi6X+UU8780XpNTpf4U6B gFrDdR8bKiHWep8CBCfe+n3QcNA4kM2mVaRIrbZBZZkzO5Pc41a93wxH/ deR0l3dJH1ofc/Owpgw21aSPf9Oltci2/V99rUNO20vRvyv0ZsS5VB7Ts sKB8gqtYRiN3W+n++5YvMKBFRyWHoyBW5/rm47wwDkrLmFGQYaz2Uhi/e WBbKSEilvIyOf60jil0GTDrDM6Wa+s8i8SKVh3rSRX6UE6fHg3afFOtl+ OPjnTHLAfOkzS0xosvF88juEap/GL0nYFRRaas3CCUFF95JN9fehY2Wu8 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="446546905" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="446546905" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2023 06:16:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="664658294" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="664658294" Received: from unknown (HELO localhost) ([10.237.66.162]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2023 06:15:49 -0700 From: Jani Nikula To: Joel Granados Cc: mcgrof@kernel.org, Russell King , Catalin Marinas , Will Deacon , Michael Ellerman , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Gerald Schaefer , Andy Lutomirski , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, Herbert Xu , "David S. Miller" , Russ Weight , Greg Kroah-Hartman , Phillip Potter , Clemens Ladisch , Arnd Bergmann , Corey Minyard , Theodore Ts'o , "Jason A. Donenfeld" , Joonas Lahtinen , Rodrigo Vivi , Tvrtko Ursulin , David Airlie , Daniel Vetter , "K. Y. Srinivasan" , Haiyang Zhang , Wei Liu , Dexuan Cui , Jason Gunthorpe , Leon Romanovsky , Benjamin Herrenschmidt , Song Liu , Robin Holt , Steve Wahl , David Ahern , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Sudip Mukherjee , Mark Rutland , "James E.J. Bottomley" , "Martin K. Petersen" , Doug Gilbert , Jiri Slaby , Juergen Gross , Stefano Stabellini , Alexander Viro , Christian Brauner , Benjamin LaHaise , David Howells , Jan Harkes , coda@cs.cmu.edu, Trond Myklebust , Anna Schumaker , Chuck Lever , Jeff Layton , Jan Kara , Anton Altaparmakov , Mark Fasheh , Joel Becker , Joseph Qi , Kees Cook , Iurii Zaikin , Eric Biggers , "Darrick J. Wong" , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Balbir Singh , Eric Biederman , "Naveen N. Rao" , Anil S Keshavamurthy , Masami Hiramatsu , Peter Zijlstra , Petr Mladek , Sergey Senozhatsky , Juri Lelli , Vincent Guittot , John Stultz , Steven Rostedt , Andrew Morton , Mike Kravetz , Muchun Song , Naoya Horiguchi , "Matthew Wilcox (Oracle)" , Joerg Reuter , Ralf Baechle , Pablo Neira Ayuso , Jozsef Kadlecsik , Florian Westphal , Roopa Prabhu , Nikolay Aleksandrov , Alexander Aring , Stefan Schmidt , Miquel Raynal , Steffen Klassert , Matthieu Baerts , Mat Martineau , Simon Horman , Julian Anastasov , Remi Denis-Courmont , Santosh Shilimkar , Marc Dionne , Neil Horman , Marcelo Ricardo Leitner , Xin Long , Karsten Graul , Wenjia Zhang , Jan Karcher , Jon Maloy , Ying Xue , Martin Schiller , John Johansen , Paul Moore , James Morris , "Serge E. Hallyn" , Jarkko Sakkinen , Nicholas Piggin , Christophe Leroy , Christian Borntraeger , Sven Schnelle , "H. Peter Anvin" , "Rafael J. Wysocki" , Mike Travis , Oleksandr Tyshchenko , Amir Goldstein , Matthew Bobrowski , John Fastabend , Martin KaFai Lau , Yonghong Song , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Waiman Long , Boqun Feng , John Ogness , Dietmar Eggemann , Ben Segall , Mel Gorman , Daniel Bristot de Oliveira , Valentin Schneider , Andy Lutomirski , Will Drewry , Stephen Boyd , Miaohe Lin , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-ia64@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, linux-crypto@vger.kernel.org, openipmi-developer@lists.sourceforge.net, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org, linux-rdma@vger.kernel.org, linux-raid@vger.kernel.org, netdev@vger.kernel.org, linux-scsi@vger.kernel.org, xen-devel@lists.xenproject.org, linux-fsdevel@vger.kernel.org, linux-aio@kvack.org, linux-cachefs@redhat.com, codalist@telemann.coda.cs.cmu.edu, linux-mm@kvack.org, linux-nfs@vger.kernel.org, linux-ntfs-dev@lists.sourceforge.net, ocfs2-devel@oss.oracle.com, fsverity@lists.linux.dev, linux-xfs@vger.kernel.org, bpf@vger.kernel.org, kexec@lists.infradead.org, linux-trace-kernel@vger.kernel.org, linux-hams@vger.kernel.org, netfilter-devel@vger.kernel.org, coreteam@netfilter.org, bridge@lists.linux-foundation.org, dccp@vger.kernel.org, linux-wpan@vger.kernel.org, mptcp@lists.linux.dev, lvs-devel@vger.kernel.org, rds-devel@oss.oracle.com, linux-afs@lists.infradead.org, linux-sctp@vger.kernel.org, tipc-discussion@lists.sourceforge.net, linux-x25@vger.kernel.org, apparmor@lists.ubuntu.com, linux-security-module@vger.kernel.org, keyrings@vger.kernel.org Subject: Re: [PATCH 09/11] sysctl: Remove the end element in sysctl table arrays In-Reply-To: <20230621130614.s36w4u7dzmb5d5p3@localhost> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20230621091000.424843-1-j.granados@samsung.com> <20230621094817.433842-1-j.granados@samsung.com> <87o7l92hg8.fsf@intel.com> <20230621130614.s36w4u7dzmb5d5p3@localhost> Date: Wed, 21 Jun 2023 16:15:46 +0300 Message-ID: <878rcd2by5.fsf@intel.com> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net On Wed, 21 Jun 2023, Joel Granados wrote: > On Wed, Jun 21, 2023 at 02:16:55PM +0300, Jani Nikula wrote: >> On Wed, 21 Jun 2023, Joel Granados wrote: >> > Remove the empty end element from all the arrays that are passed to the >> > register sysctl calls. In some files this means reducing the explicit >> > array size by one. Also make sure that we are using the size in >> > ctl_table_header instead of evaluating the .procname element. >> >> Where's the harm in removing the end elements driver by driver? This is >> an unwieldy patch to handle. > > I totally agree. Its a big one!!! but I'm concerned of breaking bisectibility: > * I could for example separate all the removes into separate commits and > then have a final commit that removes the check for the empty element. > But this will leave the tree in a state where the for loop will have > undefined behavior when it looks for the empty end element. It might > or might not work (probably not :) until the final commit where I fix > that. > > * I could also change the logic that looks for the final element, > commit that first and then remove the empty element one commit per > driver after that. But then for all the arrays that still have an > empty element, there would again be undefined behavior as it would > think that the last element is valid (when it is really the sentinel). > > Any ideas on how to get around these? First add size to the register calls, and allow the last one to be sentinel but do not require the sentinel. Start removing sentinels, adjusting the size passed in. Once enough sentinels have been removed, add warning if the final entry is a sentinel. Never really remove the check? (But surely you can rework the logic to not count the number of elements up front, only while iterating.) BR, Jani. >> >> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> > index f43950219ffc..e4d7372afb10 100644 >> > --- a/drivers/gpu/drm/i915/i915_perf.c >> > +++ b/drivers/gpu/drm/i915/i915_perf.c >> > @@ -4884,24 +4884,23 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data, >> > >> > static struct ctl_table oa_table[] = { >> > { >> > - .procname = "perf_stream_paranoid", >> > - .data = &i915_perf_stream_paranoid, >> > - .maxlen = sizeof(i915_perf_stream_paranoid), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = SYSCTL_ONE, >> > - }, >> > + .procname = "perf_stream_paranoid", >> > + .data = &i915_perf_stream_paranoid, >> > + .maxlen = sizeof(i915_perf_stream_paranoid), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = SYSCTL_ONE, >> > + }, >> > { >> > - .procname = "oa_max_sample_rate", >> > - .data = &i915_oa_max_sample_rate, >> > - .maxlen = sizeof(i915_oa_max_sample_rate), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = &oa_sample_rate_hard_limit, >> > - }, >> > - {} >> > + .procname = "oa_max_sample_rate", >> > + .data = &i915_oa_max_sample_rate, >> > + .maxlen = sizeof(i915_oa_max_sample_rate), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = &oa_sample_rate_hard_limit, >> > + } >> > }; >> >> The existing indentation is off, but fixing it doesn't really belong in >> this patch. > > Agreed. But I actually was trying to fix something that checkpatch > flagged. I'll change these back (which will cause this patch to be > flagged). > > An alternative solution would be to fix the indentation as part of the > preparation patches. Tell me what you think. > > Thx > >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 1F389418F2 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 8AB64418BB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1687353444; x=1718889444; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=9eMWiG7JI6/9oPvFtdl/3iOGYIkGUexXwjKhm4v9hgQ=; b=MV3CQEIcY4q2XiyN4FXyDo2SQTi2SMejrdTbsCR98V2rxSB7nrEFOey7 KJJA0C5LnjucpQ+1BpzffbQ01RTgqJ8iErYl7tXw+GtJ65LJBmK1N38RA O1WRP//ggUQtXSPsgDJJhheTBPU4xIG3JD2MHIs1cqxjRzKBCAiVMVSx2 KdEsAJZT0i7+XjaD2mfAdxh03D982Xe65dpQmf9cd3cuhUX6XsWT+yW2o Ya2qPDRa98TUWvnqRFk/IXqot8OyfPt+Y6m7/gHDYmNzZsmvDu4qiYUma 816HGg9QiAMOiLyJ0oxZqnH1RVlEgLARBExzrKtVVpekJYCriFD9HH1Bs g==; From: Jani Nikula In-Reply-To: <20230621130614.s36w4u7dzmb5d5p3@localhost> References: <20230621091000.424843-1-j.granados@samsung.com> <20230621094817.433842-1-j.granados@samsung.com> <87o7l92hg8.fsf@intel.com> <20230621130614.s36w4u7dzmb5d5p3@localhost> Date: Wed, 21 Jun 2023 16:15:46 +0300 Message-ID: <878rcd2by5.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Bridge] [PATCH 09/11] sysctl: Remove the end element in sysctl table arrays List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Joel Granados Cc: Juri Lelli , Miaohe Lin , "Rafael J. Wysocki" , Catalin Marinas , dri-devel@lists.freedesktop.org, Ben Segall , linux-sctp@vger.kernel.org, ocfs2-devel@oss.oracle.com, Miquel Raynal , Alexander Gordeev , "K. Y. Srinivasan" , Stefan Schmidt , Wei Liu , Vincent Guittot , Michael Ellerman , bridge@lists.linux-foundation.org, James Morris , Christophe Leroy , Jozsef Kadlecsik , Eric Biggers , linux-cachefs@redhat.com, Mel Gorman , "Darrick J. Wong" , Waiman Long , Christian Borntraeger , Petr Mladek , Martin Schiller , Russ Weight , Tvrtko Ursulin , Boqun Feng , Nicholas Piggin , John Ogness , Alexander Viro , Andy Lutomirski , Remi Denis-Courmont , xen-devel@lists.xenproject.org, Thomas Gleixner , Trond Myklebust , Anton Altaparmakov , Christian Brauner , Will Drewry , Neil Horman , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Ralf Baechle , Sergey Senozhatsky , mcgrof@kernel.org, Chuck Lever , netfilter-devel@vger.kernel.org, Masami Hiramatsu , Jiri Olsa , linux-fsdevel@vger.kernel.org, Matthieu Baerts , Andrew Morton , linux-trace-kernel@vger.kernel.org, linux-wpan@vger.kernel.org, Mark Rutland , linux-aio@kvack.org, "Jason A. Donenfeld" , linux-ia64@vger.kernel.org, Naoya Horiguchi , Dave Hansen , Clemens Ladisch , Phillip Potter , Song Liu , Eric Dumazet , keyrings@vger.kernel.org, John Stultz , Stanislav Fomichev , Jan Karcher , codalist@telemann.coda.cs.cmu.edu, linux-s390@vger.kernel.org, Valentin Schneider , Stefano Stabellini , Corey Minyard , Leon Romanovsky , Oleksandr Tyshchenko , Will Deacon , John Fastabend , Andrii Nakryiko , Anil S Keshavamurthy , Mat Martineau , Matthew Bobrowski , Julian Anastasov , coreteam@netfilter.org, Roopa Prabhu , Yonghong Song , Iurii Zaikin , Sven Schnelle , Vasily Gorbik , Mike Travis , Wenjia Zhang , Simon Horman , Xin Long , linux-arm-kernel@lists.infradead.org, fsverity@lists.linux.dev, Hao Luo , Theodore Ts'o , Stephen Boyd , Muchun Song , Florian Westphal , Robin Holt , "David S. Miller" , Jon Maloy , Jarkko Sakkinen , Eric Biederman , Anna Schumaker , Daniel Bristot de Oliveira , Mike Kravetz , Marcelo Ricardo Leitner , Benjamin Herrenschmidt , linux-hams@vger.kernel.org, Nikolay Aleksandrov , Joonas Lahtinen , Alexei Starovoitov , Marc Dionne , Jiri Slaby , linux-afs@lists.infradead.org, Daniel Borkmann , linux-rdma@vger.kernel.org, Dexuan Cui , "Matthew Wilcox (Oracle)" , lvs-devel@vger.kernel.org, coda@cs.cmu.edu, Doug Gilbert , "Naveen N. Rao" , Gerald Schaefer , Paolo Abeni , Pablo Neira Ayuso , "Serge E. Hallyn" , Kees Cook , Arnd Bergmann , Haiyang Zhang , intel-gfx@lists.freedesktop.org, Steven Rostedt , linux-crypto@vger.kernel.org, Borislav Petkov , Rodrigo Vivi , openipmi-developer@lists.sourceforge.net, mptcp@lists.linux.dev, Jan Harkes , linux-nfs@vger.kernel.org, "Martin K. Petersen" , linux-mm@kvack.org, Jeff Layton , Andy Lutomirski , linux-xfs@vger.kernel.org, linux-ntfs-dev@lists.sourceforge.net, netdev@vger.kernel.org, Ying Xue , bpf@vger.kernel.org, Sudip Mukherjee , Dietmar Eggemann , Alexander Aring , Jan Kara , Steve Wahl , Peter Zijlstra , Balbir Singh , Amir Goldstein , KP Singh , David Howells , Joseph Qi , "H. Peter Anvin" , David Airlie , Steffen Klassert , rds-devel@oss.oracle.com, Herbert Xu , linux-scsi@vger.kernel.org, dccp@vger.kernel.org, Mark Fasheh , x86@kernel.org, Russell King , Jason Gunthorpe , Ingo Molnar , Jakub Kicinski , "James E.J. Bottomley" , Joerg Reuter , linux-hyperv@vger.kernel.org, Heiko Carstens , Santosh Shilimkar , apparmor@lists.ubuntu.com, linux-raid@vger.kernel.org, Paul Moore , Juergen Gross , John Johansen , linux-x25@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, David Ahern , kexec@lists.infradead.org, linux-security-module@vger.kernel.org, Benjamin LaHaise , tipc-discussion@lists.sourceforge.net, Daniel Vetter , Martin KaFai Lau , Karsten Graul , Joel Becker On Wed, 21 Jun 2023, Joel Granados wrote: > On Wed, Jun 21, 2023 at 02:16:55PM +0300, Jani Nikula wrote: >> On Wed, 21 Jun 2023, Joel Granados wrote: >> > Remove the empty end element from all the arrays that are passed to the >> > register sysctl calls. In some files this means reducing the explicit >> > array size by one. Also make sure that we are using the size in >> > ctl_table_header instead of evaluating the .procname element. >> >> Where's the harm in removing the end elements driver by driver? This is >> an unwieldy patch to handle. > > I totally agree. Its a big one!!! but I'm concerned of breaking bisectibility: > * I could for example separate all the removes into separate commits and > then have a final commit that removes the check for the empty element. > But this will leave the tree in a state where the for loop will have > undefined behavior when it looks for the empty end element. It might > or might not work (probably not :) until the final commit where I fix > that. > > * I could also change the logic that looks for the final element, > commit that first and then remove the empty element one commit per > driver after that. But then for all the arrays that still have an > empty element, there would again be undefined behavior as it would > think that the last element is valid (when it is really the sentinel). > > Any ideas on how to get around these? First add size to the register calls, and allow the last one to be sentinel but do not require the sentinel. Start removing sentinels, adjusting the size passed in. Once enough sentinels have been removed, add warning if the final entry is a sentinel. Never really remove the check? (But surely you can rework the logic to not count the number of elements up front, only while iterating.) BR, Jani. >> >> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> > index f43950219ffc..e4d7372afb10 100644 >> > --- a/drivers/gpu/drm/i915/i915_perf.c >> > +++ b/drivers/gpu/drm/i915/i915_perf.c >> > @@ -4884,24 +4884,23 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data, >> > >> > static struct ctl_table oa_table[] = { >> > { >> > - .procname = "perf_stream_paranoid", >> > - .data = &i915_perf_stream_paranoid, >> > - .maxlen = sizeof(i915_perf_stream_paranoid), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = SYSCTL_ONE, >> > - }, >> > + .procname = "perf_stream_paranoid", >> > + .data = &i915_perf_stream_paranoid, >> > + .maxlen = sizeof(i915_perf_stream_paranoid), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = SYSCTL_ONE, >> > + }, >> > { >> > - .procname = "oa_max_sample_rate", >> > - .data = &i915_oa_max_sample_rate, >> > - .maxlen = sizeof(i915_oa_max_sample_rate), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = &oa_sample_rate_hard_limit, >> > - }, >> > - {} >> > + .procname = "oa_max_sample_rate", >> > + .data = &i915_oa_max_sample_rate, >> > + .maxlen = sizeof(i915_oa_max_sample_rate), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = &oa_sample_rate_hard_limit, >> > + } >> > }; >> >> The existing indentation is off, but fixing it doesn't really belong in >> this patch. > > Agreed. But I actually was trying to fix something that checkpatch > flagged. I'll change these back (which will cause this patch to be > flagged). > > An alternative solution would be to fix the indentation as part of the > preparation patches. Tell me what you think. > > Thx > >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center 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 94549EB64DD for ; Tue, 27 Jun 2023 14:39:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BC27210E348; Tue, 27 Jun 2023 14:39:13 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4956F10E474; Wed, 21 Jun 2023 13:17:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1687353447; x=1718889447; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=9eMWiG7JI6/9oPvFtdl/3iOGYIkGUexXwjKhm4v9hgQ=; b=VKdU+4JgbX4NpbUJ1pGQwx7rP4tCzihjtawrYes6gzCDNYtQiih+jwVe F873dwIsvhod0YARO9VKUhkj06aRarr7vuGmzoEiVQ5fk5wly5W6omK9t +iNA+hX6lctBvXKvd6x1QxXNWe7qto5MfagiH0EzUG6mn84V9O5UOzwoq F/aRtyuUwktofQl51JDVKYz5pF4rWhcdEX3XoC7ki2x3sTgM+O6J9yJog 5JEgyf/oEuKfTSadtzoz9HPLrnTtASM+FNLftPl81nPy05SF7GnHL/9Ot 22pUVV6zo5lB1BC+Z8d7QHj5e58COuQdxXBIR0/wXLEUCpqKND8y5XrMz w==; X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="446546927" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="446546927" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2023 06:16:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="664658294" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="664658294" Received: from unknown (HELO localhost) ([10.237.66.162]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2023 06:15:49 -0700 From: Jani Nikula To: Joel Granados In-Reply-To: <20230621130614.s36w4u7dzmb5d5p3@localhost> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20230621091000.424843-1-j.granados@samsung.com> <20230621094817.433842-1-j.granados@samsung.com> <87o7l92hg8.fsf@intel.com> <20230621130614.s36w4u7dzmb5d5p3@localhost> Date: Wed, 21 Jun 2023 16:15:46 +0300 Message-ID: <878rcd2by5.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain X-Mailman-Approved-At: Tue, 27 Jun 2023 14:38:30 +0000 Subject: Re: [Intel-gfx] [PATCH 09/11] sysctl: Remove the end element in sysctl table arrays 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: Juri Lelli , Miaohe Lin , "Rafael J. Wysocki" , Catalin Marinas , dri-devel@lists.freedesktop.org, Ben Segall , linux-sctp@vger.kernel.org, ocfs2-devel@oss.oracle.com, Miquel Raynal , Alexander Gordeev , "K. Y. Srinivasan" , Stefan Schmidt , Wei Liu , Michael Ellerman , bridge@lists.linux-foundation.org, James Morris , Christophe Leroy , Jozsef Kadlecsik , Eric Biggers , linux-cachefs@redhat.com, Mel Gorman , "Darrick J. Wong" , Waiman Long , Christian Borntraeger , Petr Mladek , Martin Schiller , Russ Weight , Boqun Feng , Nicholas Piggin , John Ogness , Alexander Viro , Andy Lutomirski , Remi Denis-Courmont , xen-devel@lists.xenproject.org, Thomas Gleixner , Trond Myklebust , Anton Altaparmakov , Christian Brauner , Will Drewry , Neil Horman , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Ralf Baechle , Sergey Senozhatsky , mcgrof@kernel.org, Chuck Lever , netfilter-devel@vger.kernel.org, Masami Hiramatsu , Jiri Olsa , linux-fsdevel@vger.kernel.org, Matthieu Baerts , Andrew Morton , linux-trace-kernel@vger.kernel.org, linux-wpan@vger.kernel.org, Mark Rutland , linux-aio@kvack.org, "Jason A. Donenfeld" , linux-ia64@vger.kernel.org, Naoya Horiguchi , Dave Hansen , Clemens Ladisch , Phillip Potter , Song Liu , Eric Dumazet , keyrings@vger.kernel.org, John Stultz , Stanislav Fomichev , Jan Karcher , codalist@telemann.coda.cs.cmu.edu, linux-s390@vger.kernel.org, Valentin Schneider , Stefano Stabellini , Corey Minyard , Leon Romanovsky , Oleksandr Tyshchenko , Will Deacon , John Fastabend , Andrii Nakryiko , Anil S Keshavamurthy , Mat Martineau , Matthew Bobrowski , Julian Anastasov , coreteam@netfilter.org, Roopa Prabhu , Yonghong Song , Iurii Zaikin , Sven Schnelle , Vasily Gorbik , Mike Travis , Wenjia Zhang , Simon Horman , Xin Long , linux-arm-kernel@lists.infradead.org, fsverity@lists.linux.dev, Hao Luo , Theodore Ts'o , Stephen Boyd , Muchun Song , Florian Westphal , Robin Holt , "David S. Miller" , Jon Maloy , Jarkko Sakkinen , Eric Biederman , Anna Schumaker , Daniel Bristot de Oliveira , Mike Kravetz , Marcelo Ricardo Leitner , Benjamin Herrenschmidt , linux-hams@vger.kernel.org, Nikolay Aleksandrov , Alexei Starovoitov , Marc Dionne , Jiri Slaby , linux-afs@lists.infradead.org, Daniel Borkmann , linux-rdma@vger.kernel.org, Dexuan Cui , "Matthew Wilcox \(Oracle\)" , lvs-devel@vger.kernel.org, coda@cs.cmu.edu, Doug Gilbert , "Naveen N. Rao" , Gerald Schaefer , Paolo Abeni , Pablo Neira Ayuso , "Serge E. Hallyn" , Kees Cook , Arnd Bergmann , Haiyang Zhang , intel-gfx@lists.freedesktop.org, Steven Rostedt , linux-crypto@vger.kernel.org, Borislav Petkov , Rodrigo Vivi , openipmi-developer@lists.sourceforge.net, mptcp@lists.linux.dev, Jan Harkes , linux-nfs@vger.kernel.org, "Martin K. Petersen" , linux-mm@kvack.org, Jeff Layton , Andy Lutomirski , linux-xfs@vger.kernel.org, linux-ntfs-dev@lists.sourceforge.net, netdev@vger.kernel.org, Ying Xue , bpf@vger.kernel.org, Sudip Mukherjee , Dietmar Eggemann , Alexander Aring , Jan Kara , Steve Wahl , Peter Zijlstra , Balbir Singh , Amir Goldstein , KP Singh , David Howells , Joseph Qi , "H. Peter Anvin" , David Airlie , Steffen Klassert , rds-devel@oss.oracle.com, Herbert Xu , linux-scsi@vger.kernel.org, dccp@vger.kernel.org, Mark Fasheh , x86@kernel.org, Russell King , Jason Gunthorpe , Ingo Molnar , Jakub Kicinski , "James E.J. Bottomley" , Joerg Reuter , linux-hyperv@vger.kernel.org, Heiko Carstens , Santosh Shilimkar , apparmor@lists.ubuntu.com, linux-raid@vger.kernel.org, Paul Moore , Juergen Gross , John Johansen , linux-x25@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, David Ahern , kexec@lists.infradead.org, linux-security-module@vger.kernel.org, Benjamin LaHaise , tipc-discussion@lists.sourceforge.net, Daniel Vetter , Martin KaFai Lau , Karsten Graul , Joel Becker Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Wed, 21 Jun 2023, Joel Granados wrote: > On Wed, Jun 21, 2023 at 02:16:55PM +0300, Jani Nikula wrote: >> On Wed, 21 Jun 2023, Joel Granados wrote: >> > Remove the empty end element from all the arrays that are passed to the >> > register sysctl calls. In some files this means reducing the explicit >> > array size by one. Also make sure that we are using the size in >> > ctl_table_header instead of evaluating the .procname element. >> >> Where's the harm in removing the end elements driver by driver? This is >> an unwieldy patch to handle. > > I totally agree. Its a big one!!! but I'm concerned of breaking bisectibility: > * I could for example separate all the removes into separate commits and > then have a final commit that removes the check for the empty element. > But this will leave the tree in a state where the for loop will have > undefined behavior when it looks for the empty end element. It might > or might not work (probably not :) until the final commit where I fix > that. > > * I could also change the logic that looks for the final element, > commit that first and then remove the empty element one commit per > driver after that. But then for all the arrays that still have an > empty element, there would again be undefined behavior as it would > think that the last element is valid (when it is really the sentinel). > > Any ideas on how to get around these? First add size to the register calls, and allow the last one to be sentinel but do not require the sentinel. Start removing sentinels, adjusting the size passed in. Once enough sentinels have been removed, add warning if the final entry is a sentinel. Never really remove the check? (But surely you can rework the logic to not count the number of elements up front, only while iterating.) BR, Jani. >> >> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> > index f43950219ffc..e4d7372afb10 100644 >> > --- a/drivers/gpu/drm/i915/i915_perf.c >> > +++ b/drivers/gpu/drm/i915/i915_perf.c >> > @@ -4884,24 +4884,23 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data, >> > >> > static struct ctl_table oa_table[] = { >> > { >> > - .procname = "perf_stream_paranoid", >> > - .data = &i915_perf_stream_paranoid, >> > - .maxlen = sizeof(i915_perf_stream_paranoid), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = SYSCTL_ONE, >> > - }, >> > + .procname = "perf_stream_paranoid", >> > + .data = &i915_perf_stream_paranoid, >> > + .maxlen = sizeof(i915_perf_stream_paranoid), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = SYSCTL_ONE, >> > + }, >> > { >> > - .procname = "oa_max_sample_rate", >> > - .data = &i915_oa_max_sample_rate, >> > - .maxlen = sizeof(i915_oa_max_sample_rate), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = &oa_sample_rate_hard_limit, >> > - }, >> > - {} >> > + .procname = "oa_max_sample_rate", >> > + .data = &i915_oa_max_sample_rate, >> > + .maxlen = sizeof(i915_oa_max_sample_rate), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = &oa_sample_rate_hard_limit, >> > + } >> > }; >> >> The existing indentation is off, but fixing it doesn't really belong in >> this patch. > > Agreed. But I actually was trying to fix something that checkpatch > flagged. I'll change these back (which will cause this patch to be > flagged). > > An alternative solution would be to fix the indentation as part of the > preparation patches. Tell me what you think. > > Thx > >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH 09/11] sysctl: Remove the end element in sysctl table arrays Date: Wed, 21 Jun 2023 16:15:46 +0300 Message-ID: <878rcd2by5.fsf@intel.com> References: <20230621091000.424843-1-j.granados@samsung.com> <20230621094817.433842-1-j.granados@samsung.com> <87o7l92hg8.fsf@intel.com> <20230621130614.s36w4u7dzmb5d5p3@localhost> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1687353450; x=1718889450; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=9eMWiG7JI6/9oPvFtdl/3iOGYIkGUexXwjKhm4v9hgQ=; b=bCKCupaYl+EEOS5QZZqFy7/P+ML1IyBgPHBQi6X+UU8780XpNTpf4U6B gFrDdR8bKiHWep8CBCfe+n3QcNA4kM2mVaRIrbZBZZkzO5Pc41a93wxH/ deR0l3dJH1ofc/Owpgw21aSPf9Oltci2/V99rUNO20vRvyv0ZsS5VB7Ts sKB8gqtYRiN3W+n++5YvMKBFRyWHoyBW5/rm47wwDkrLmFGQYaz2Uhi/e WBbKSEilvIyOf60jil0GTDrDM6Wa+s8i8SKVh3rSRX6UE6fHg3afFOtl+ OPjnTHLAfOkzS0xosvF88juEap/GL0nYFRRaas3CCUFF95JN9fehY2Wu8 Q==; In-Reply-To: <20230621130614.s36w4u7dzmb5d5p3@localhost> List-Id: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Joel Granados Cc: mcgrof@kernel.org, Russell King , Catalin Marinas , Will Deacon , Michael Ellerman , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Gerald Schaefer , Andy Lutomirski , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, Herbert Xu , "David S. Miller" , Russ Weight , Greg Kroah-Hartman , Phillip Potter , Clemens Ladisch , Arnd Bergmann , Corey Minyard , Theodore Ts'o , Jason On Wed, 21 Jun 2023, Joel Granados wrote: > On Wed, Jun 21, 2023 at 02:16:55PM +0300, Jani Nikula wrote: >> On Wed, 21 Jun 2023, Joel Granados wrote: >> > Remove the empty end element from all the arrays that are passed to the >> > register sysctl calls. In some files this means reducing the explicit >> > array size by one. Also make sure that we are using the size in >> > ctl_table_header instead of evaluating the .procname element. >> >> Where's the harm in removing the end elements driver by driver? This is >> an unwieldy patch to handle. > > I totally agree. Its a big one!!! but I'm concerned of breaking bisectibility: > * I could for example separate all the removes into separate commits and > then have a final commit that removes the check for the empty element. > But this will leave the tree in a state where the for loop will have > undefined behavior when it looks for the empty end element. It might > or might not work (probably not :) until the final commit where I fix > that. > > * I could also change the logic that looks for the final element, > commit that first and then remove the empty element one commit per > driver after that. But then for all the arrays that still have an > empty element, there would again be undefined behavior as it would > think that the last element is valid (when it is really the sentinel). > > Any ideas on how to get around these? First add size to the register calls, and allow the last one to be sentinel but do not require the sentinel. Start removing sentinels, adjusting the size passed in. Once enough sentinels have been removed, add warning if the final entry is a sentinel. Never really remove the check? (But surely you can rework the logic to not count the number of elements up front, only while iterating.) BR, Jani. >> >> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> > index f43950219ffc..e4d7372afb10 100644 >> > --- a/drivers/gpu/drm/i915/i915_perf.c >> > +++ b/drivers/gpu/drm/i915/i915_perf.c >> > @@ -4884,24 +4884,23 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data, >> > >> > static struct ctl_table oa_table[] = { >> > { >> > - .procname = "perf_stream_paranoid", >> > - .data = &i915_perf_stream_paranoid, >> > - .maxlen = sizeof(i915_perf_stream_paranoid), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = SYSCTL_ONE, >> > - }, >> > + .procname = "perf_stream_paranoid", >> > + .data = &i915_perf_stream_paranoid, >> > + .maxlen = sizeof(i915_perf_stream_paranoid), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = SYSCTL_ONE, >> > + }, >> > { >> > - .procname = "oa_max_sample_rate", >> > - .data = &i915_oa_max_sample_rate, >> > - .maxlen = sizeof(i915_oa_max_sample_rate), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = &oa_sample_rate_hard_limit, >> > - }, >> > - {} >> > + .procname = "oa_max_sample_rate", >> > + .data = &i915_oa_max_sample_rate, >> > + .maxlen = sizeof(i915_oa_max_sample_rate), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = &oa_sample_rate_hard_limit, >> > + } >> > }; >> >> The existing indentation is off, but fixing it doesn't really belong in >> this patch. > > Agreed. But I actually was trying to fix something that checkpatch > flagged. I'll change these back (which will cause this patch to be > flagged). > > An alternative solution would be to fix the indentation as part of the > preparation patches. Tell me what you think. > > Thx > >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center 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 aib29ajc255.phx1.oracleemaildelivery.com (aib29ajc255.phx1.oracleemaildelivery.com [192.29.103.255]) (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 5FA1CC001B3 for ; Wed, 21 Jun 2023 16:07:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; s=oss-phx-1109; d=oss.oracle.com; h=Date:To:From:Subject:Message-Id:MIME-Version:Sender; bh=rv/BXpMHJ1I8jUfS8Top23b8LI3dJ2f6aR0S3tVsFRI=; b=qbIouCZgcVyoMG3IaqKhrC8eCgpgtf2tA+zwBNMQIwDflIq4GSbNOmyCgopjdxJjfBf6QF6Zu00o ru1m2hoHqgKXTGjWaiKfMw51vhhZ4bLfl8kZnfDCt4LV7rkLhVJ/QcXlrhYNWwz6blmYS6jLYkoZ +tTF4Mj3U2i7ZBTzFShfBlLPjituBMSl/gWr3TqblQ9liAlb80QFA3NYJfSw+JdpcTcUd+Py/MKm t2/f1UCOx6pic8r92F8cNriVCxrSAeLarZArgNxQ29jpnkoOgvHKHUynPs+qrBwwAWnXcRjTF2jt NYfeEuyKdYeOOb+J3XxK+pI/zVXcdHxuYVRLiw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; s=prod-phx-20191217; d=phx1.rp.oracleemaildelivery.com; h=Date:To:From:Subject:Message-Id:MIME-Version:Sender; bh=rv/BXpMHJ1I8jUfS8Top23b8LI3dJ2f6aR0S3tVsFRI=; b=bO3f/K9PX0B0euh8nVFiLKLTIML2eUvl1zf3Bh0Ljz5Hre3hle2e+aH2ALciyFF9acCt1nQev9u2 J8nEMGlpBhf+UOlsqI8+Uo3Kz3yC0UPlrPx5xaEv6g02znhoIBS9ijKxw4o8fb50RCrrSqBy4+5C jBTqUUdvFoajuFQRT5jYccOOjvltP2iUbDtuO3jS5q6xmd0DdnMLVGSua1wXJ0Ue5JiB4RVZR5d1 3DHczc1UtJdRV0OnRY7ArPXvFNKkHSEY1ka4+v9rnh2jYwpzsUQRGzcXmTPivCZk3j5pa362MvJ/ aDtU2eX+dLMN9dbp/GoGZjqgjhXZQMAEyEz9hA== Received: by omta-ad3-fd3-302-us-phoenix-1.omtaad3.vcndpphx.oraclevcn.com (Oracle Communications Messaging Server 8.1.0.1.20230523 64bit (built May 23 2023)) with ESMTPS id <0RWM00GHX23SGR20@omta-ad3-fd3-302-us-phoenix-1.omtaad3.vcndpphx.oraclevcn.com> for ocfs2-devel@archiver.kernel.org; Wed, 21 Jun 2023 16:07:04 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1687353446; x=1718889446; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=9eMWiG7JI6/9oPvFtdl/3iOGYIkGUexXwjKhm4v9hgQ=; b=aekgZFGF3DD4VAc2/KVxORdTAcPzaLTmjjOwOaJiw8U2Lb85iLdYhA/+ nFDgVV4Y0UeY/Q1IeVVWSCpe0HDD/xwOYDysq3k9+gC3TMML6YZ3YnRKi MTOPRaxhz7asIGa3GpHBlxtffwRuv3IgcbeG+Gdk8PpgQKfrk1+tG31n3 m+0qJwh0b9OdmxAWgqW24MngsVPF7SvM+oZWmiHiH1TCO4POp+Rl9Latf aZZ0kYgws7JtVaf7IOVeA5SW5kJfxAPjNTlYhvO4+kdyl5o9Uuk1iZtv0 JZuJw3iyXSkNwVcbOvVPYd3t87nGJBZJynoYfNEE506umu2+74Ws58Xkt w==; To: Joel Granados In-reply-to: <20230621130614.s36w4u7dzmb5d5p3@localhost> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20230621091000.424843-1-j.granados@samsung.com> <20230621094817.433842-1-j.granados@samsung.com> <87o7l92hg8.fsf@intel.com> <20230621130614.s36w4u7dzmb5d5p3@localhost> Date: Wed, 21 Jun 2023 16:15:46 +0300 Message-id: <878rcd2by5.fsf@intel.com> MIME-version: 1.0 X-Source-IP: 192.55.52.43 X-Proofpoint-Virus-Version: vendor=nai engine=6500 definitions=10748 signatures=596816 X-Proofpoint-Spam-Details: rule=tap_notspam policy=tap score=0 mlxscore=0 mlxlogscore=999 bulkscore=0 clxscore=258 malwarescore=0 adultscore=0 priorityscore=343 suspectscore=0 impostorscore=0 spamscore=0 phishscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2305260000 definitions=main-2306210112 domainage_hfrom=13602 Cc: Juri Lelli , Miaohe Lin , "Rafael J. Wysocki" , Catalin Marinas , dri-devel@lists.freedesktop.org, Ben Segall , linux-sctp@vger.kernel.org, ocfs2-devel@oss.oracle.com, Miquel Raynal , Alexander Gordeev , "K. Y. Srinivasan" , Stefan Schmidt , Wei Liu , Vincent Guittot , Michael Ellerman , bridge@lists.linux-foundation.org, James Morris , Christophe Leroy , Jozsef Kadlecsik , Eric Biggers , linux-cachefs@redhat.com, Mel Gorman , Waiman Long , Christian Borntraeger , Petr Mladek , Martin Schiller , Russ Weight , Tvrtko Ursulin , Boqun Feng , Nicholas Piggin , John Ogness , Alexander Viro , Andy Lutomirski , Remi Denis-Courmont , xen-devel@lists.xenproject.org, Thomas Gleixner , Trond Myklebust , Anton Altaparmakov , Christian Brauner , Will Drewry , Neil Horman , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Ralf Baechle , Sergey Senozhatsky , mcgrof@kernel.org, Chuck Lever , netfilter-devel@vger.kernel.org, Masami Hiramatsu , Jiri Olsa , linux-fsdevel@vger.kernel.org, Matthieu Baerts , linux-trace-kernel@vger.kernel.org, linux-wpan@vger.kernel.org, Mark Rutland , linux-aio@kvack.org, "Jason A. Donenfeld" , linux-ia64@vger.kernel.org, Naoya Horiguchi , Dave Hansen , Clemens Ladisch , Phillip Potter , Song Liu , Eric Dumazet , keyrings@vger.kernel.org, John Stultz , Stanislav Fomichev , Jan Karcher , codalist@telemann.coda.cs.cmu.edu, linux-s390@vger.kernel.org, Valentin Schneider , Stefano Stabellini , Corey Minyard , Leon Romanovsky , Oleksandr Tyshchenko , Will Deacon , John Fastabend , Andrii Nakryiko , Anil S Keshavamurthy , Mat Martineau , Matthew Bobrowski , Julian Anastasov , coreteam@netfilter.org, Roopa Prabhu , Yonghong Song , Iurii Zaikin , Sven Schnelle , Vasily Gorbik , Mike Travis , Wenjia Zhang , Simon Horman , Xin Long , linux-arm-kernel@lists.infradead.org, fsverity@lists.linux.dev, Hao Luo , Theodore Ts'o , Stephen Boyd , Muchun Song , Florian Westphal , Robin Holt , "David S. Miller" , Jon Maloy , Jarkko Sakkinen , Eric Biederman , Anna Schumaker , Daniel Bristot de Oliveira , Mike Kravetz , Marcelo Ricardo Leitner , Benjamin Herrenschmidt , linux-hams@vger.kernel.org, Nikolay Aleksandrov , Joonas Lahtinen , Alexei Starovoitov , Marc Dionne , Jiri Slaby , linux-afs@lists.infradead.org, Daniel Borkmann , linux-rdma@vger.kernel.org, Dexuan Cui , lvs-devel@vger.kernel.org, coda@cs.cmu.edu, Doug Gilbert , "Naveen N. Rao" , Gerald Schaefer , Paolo Abeni , Pablo Neira Ayuso , "Serge E. Hallyn" , Kees Cook , Arnd Bergmann , Haiyang Zhang , intel-gfx@lists.freedesktop.org, Steven Rostedt , linux-crypto@vger.kernel.org, Borislav Petkov , Rodrigo Vivi , openipmi-developer@lists.sourceforge.net, mptcp@lists.linux.dev, Jan Harkes , linux-nfs@vger.kernel.org, linux-mm@kvack.org, Jeff Layton , Andy Lutomirski , linux-xfs@vger.kernel.org, linux-ntfs-dev@lists.sourceforge.net, netdev@vger.kernel.org, Ying Xue , bpf@vger.kernel.org, Sudip Mukherjee , Dietmar Eggemann , Alexander Aring , Jan Kara , Steve Wahl , Peter Zijlstra , Balbir Singh , Amir Goldstein , KP Singh , David Howells , "H. Peter Anvin" , David Airlie , Steffen Klassert , rds-devel@oss.oracle.com, Herbert Xu , linux-scsi@vger.kernel.org, dccp@vger.kernel.org, x86@kernel.org, Russell King , Jason Gunthorpe , Ingo Molnar , Jakub Kicinski , "James E.J. Bottomley" , Joerg Reuter , linux-hyperv@vger.kernel.org, Heiko Carstens , Santosh Shilimkar , apparmor@lists.ubuntu.com, linux-raid@vger.kernel.org, Paul Moore , Juergen Gross , John Johansen , linux-x25@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, David Ahern , kexec@lists.infradead.org, linux-security-module@vger.kernel.org, Benjamin LaHaise , tipc-discussion@lists.sourceforge.net, Daniel Vetter , Martin KaFai Lau , Karsten Graul Subject: Re: [Ocfs2-devel] [PATCH 09/11] sysctl: Remove the end element in sysctl table arrays X-BeenThere: ocfs2-devel@oss.oracle.com X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Jani Nikula via Ocfs2-devel Reply-to: Jani Nikula Content-type: text/plain; charset="us-ascii" Content-transfer-encoding: 7bit Errors-to: ocfs2-devel-bounces@oss.oracle.com X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="446546901" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="446546901" X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="664658294" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="664658294" X-ServerName: mga05.intel.com X-Proofpoint-SPF-Result: pass X-Proofpoint-SPF-Record: v=spf1 include:_spf.intel.com -all X-Spam: Clean X-Proofpoint-GUID: FgACQ9AMichSA58VFztO2v-aUk0qsZb8 X-Proofpoint-ORIG-GUID: FgACQ9AMichSA58VFztO2v-aUk0qsZb8 X-Mailman-Approved-At: Wed, 21 Jun 2023 16:07:01 +0000 Reporting-Meta: AAEXGAYBU7KZGoMveVj7RxAGYiY0iG3AhdcypMPo8VOaLmmIe/aXgyeSBrE3wUEW UIzCLkWMQr76m61q9R3zj61IKEbIMr7wYxvIeuliV21n5z+TvzMBGoeVmieeStWP PLTyNUBDpQtH15NglH0sxy7Twd8rmWkZAb/aixL0/CrTfsmuAMNBkGBImEVR8o6D qDfoWorkUKmlxyxs3uDYo7BShKGqlsIMnRvceE7XINvtwjXuS3TnujWid5NAo0Lm g5Y9vtgpTqj7ktzNXprO145a+k6AnQ9RzQvCHj7CelQRn6SwWqjYJxAL/JtHunYY QW91l/9mwCERno/k7G6vkwPSuH+Mx6m5VTCRW94rztmjbQdXQmpXF72C5q/kXyAF R0/dx0+YXeHBBRoSo9hewrnyN7gBpxqvSMv7TQWM/BJESlyhsW5eqiiSicDmWuTp wfjO64NaCh8KaFE5XN3wWj9eR2nDm60sHjIe6jTBgQ3A7P7betkdw1g9Gb/XIUuP VicLsT4bkn5UhFWR8tJzfT52fLiWADD5Q5Rczey0LQs= On Wed, 21 Jun 2023, Joel Granados wrote: > On Wed, Jun 21, 2023 at 02:16:55PM +0300, Jani Nikula wrote: >> On Wed, 21 Jun 2023, Joel Granados wrote: >> > Remove the empty end element from all the arrays that are passed to the >> > register sysctl calls. In some files this means reducing the explicit >> > array size by one. Also make sure that we are using the size in >> > ctl_table_header instead of evaluating the .procname element. >> >> Where's the harm in removing the end elements driver by driver? This is >> an unwieldy patch to handle. > > I totally agree. Its a big one!!! but I'm concerned of breaking bisectibility: > * I could for example separate all the removes into separate commits and > then have a final commit that removes the check for the empty element. > But this will leave the tree in a state where the for loop will have > undefined behavior when it looks for the empty end element. It might > or might not work (probably not :) until the final commit where I fix > that. > > * I could also change the logic that looks for the final element, > commit that first and then remove the empty element one commit per > driver after that. But then for all the arrays that still have an > empty element, there would again be undefined behavior as it would > think that the last element is valid (when it is really the sentinel). > > Any ideas on how to get around these? First add size to the register calls, and allow the last one to be sentinel but do not require the sentinel. Start removing sentinels, adjusting the size passed in. Once enough sentinels have been removed, add warning if the final entry is a sentinel. Never really remove the check? (But surely you can rework the logic to not count the number of elements up front, only while iterating.) BR, Jani. >> >> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> > index f43950219ffc..e4d7372afb10 100644 >> > --- a/drivers/gpu/drm/i915/i915_perf.c >> > +++ b/drivers/gpu/drm/i915/i915_perf.c >> > @@ -4884,24 +4884,23 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data, >> > >> > static struct ctl_table oa_table[] = { >> > { >> > - .procname = "perf_stream_paranoid", >> > - .data = &i915_perf_stream_paranoid, >> > - .maxlen = sizeof(i915_perf_stream_paranoid), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = SYSCTL_ONE, >> > - }, >> > + .procname = "perf_stream_paranoid", >> > + .data = &i915_perf_stream_paranoid, >> > + .maxlen = sizeof(i915_perf_stream_paranoid), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = SYSCTL_ONE, >> > + }, >> > { >> > - .procname = "oa_max_sample_rate", >> > - .data = &i915_oa_max_sample_rate, >> > - .maxlen = sizeof(i915_oa_max_sample_rate), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = &oa_sample_rate_hard_limit, >> > - }, >> > - {} >> > + .procname = "oa_max_sample_rate", >> > + .data = &i915_oa_max_sample_rate, >> > + .maxlen = sizeof(i915_oa_max_sample_rate), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = &oa_sample_rate_hard_limit, >> > + } >> > }; >> >> The existing indentation is off, but fixing it doesn't really belong in >> this patch. > > Agreed. But I actually was trying to fix something that checkpatch > flagged. I'll change these back (which will cause this patch to be > flagged). > > An alternative solution would be to fix the indentation as part of the > preparation patches. Tell me what you think. > > Thx > >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel 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 41A20EB64D7 for ; Wed, 21 Jun 2023 18:43:31 +0000 (UTC) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=QGWZ+FFA; dkim-atps=neutral Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4QmXSx5KTLz3c1H for ; Thu, 22 Jun 2023 04:43:29 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.a=rsa-sha256 header.s=Intel header.b=QGWZ+FFA; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=intel.com (client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=jani.nikula@intel.com; receiver=lists.ozlabs.org) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 4QmPDn0HPyz30hJ for ; Wed, 21 Jun 2023 23:17:27 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1687353449; x=1718889449; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=9eMWiG7JI6/9oPvFtdl/3iOGYIkGUexXwjKhm4v9hgQ=; b=QGWZ+FFAbcCUzPdvmLz5hfxXjnHJdsykuYtREUcqD+FfiRR/ZQeNEx/a pph2s9Kj6d1xT1JxL+4IZTuX29N7nwz45/QyM1Br9zdMhNPF3IFDOEcYH O+eOYSZ4pytvSTXeTHtN0WMsczBIingIUvQ4HftTm99ktfcK2ACBfmX+0 aIVvZL1N2slmGCT3Rc6cazDtLQeqx9Xrh+NvQ2AMghHLPlH/JZdY17aIC RoOJg58UGvx4ve/4KvdepEJknaAgXxY46weWp67Kc92gn3Xo/D05TUnIJ ednkWdbFldbLBiGQmmxClGkvARsycIL8gcookIWxudqYAl8dKZ2zos/dM Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="446546924" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="446546924" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2023 06:16:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="664658294" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="664658294" Received: from unknown (HELO localhost) ([10.237.66.162]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2023 06:15:49 -0700 From: Jani Nikula To: Joel Granados Subject: Re: [PATCH 09/11] sysctl: Remove the end element in sysctl table arrays In-Reply-To: <20230621130614.s36w4u7dzmb5d5p3@localhost> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20230621091000.424843-1-j.granados@samsung.com> <20230621094817.433842-1-j.granados@samsung.com> <87o7l92hg8.fsf@intel.com> <20230621130614.s36w4u7dzmb5d5p3@localhost> Date: Wed, 21 Jun 2023 16:15:46 +0300 Message-ID: <878rcd2by5.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain X-Mailman-Approved-At: Thu, 22 Jun 2023 04:32:35 +1000 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: Juri Lelli , Miaohe Lin , "Rafael J. Wysocki" , Catalin Marinas , dri-devel@lists.freedesktop.org, Ben Segall , linux-sctp@vger.kernel.org, ocfs2-devel@oss.oracle.com, Miquel Raynal , Alexander Gordeev , "K. Y. Srinivasan" , Stefan Schmidt , Wei Liu , Vincent Guittot , bridge@lists.linux-foundation.org, James Morris , Jozsef Kadlecsik , Eric Biggers , linux-cachefs@redhat.com, Mel Gorman , "Darrick J. Wong" , Waiman Long , Christian Borntraeger , Petr Mladek , Martin Schiller , Russ Weight , Tvrtko Ursulin , Boqun Feng , Nicholas Piggin , John Ogness , Alexander Viro , Andy Lutomirski , Remi Denis-Courmont , xen-devel@lists.xenproject.org, Thomas Gleixner , Trond Myklebust , Anton Altaparmakov , Christian Brauner , Will Drewry , Neil Horman , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Ralf Baechle , Sergey Senozhatsky , mcgrof@kernel.org, Chuck Lever , netfilter-devel@vger.kernel.org, Masami Hiramatsu , Jiri Olsa , linux-fsdevel@vger.kernel.org, Matthieu Baerts , Andrew Morton , linux-trace-kernel@vger.kernel.org, linux-wpan@vger.kernel.org, Mark Rutland , linux-aio@kvack.org, "Jason A. Donenfeld" , linux-ia64@vger.kernel.org, Naoya Horiguchi , Dave Hansen , Clemens Ladisch , Phillip Potter , Song Liu , Eric Dumazet , keyrings@vger.kernel.org, John Stultz , Stanislav Fomichev , Jan Karcher , codalist@telemann.coda.cs.cmu.edu, linux-s390@vger.kernel.org, Valentin Schneider , Stefano Stabellini , Corey Minyard , Leon Romanovsky , Oleksandr Tyshchenko , Will Deacon , John Fastabend , Andrii Nakryiko , Anil S Keshavamurthy , Mat Martineau , Matthew Bobrowski , Julian Anastasov , coreteam@netfilter.org, Roopa Prabhu , Yonghong Song , Iurii Zaikin , Sven Schnelle , Vasily Gorbik , Mike Travis , Wenjia Zhang , Simon Horman , Xin Long , linux-arm-kernel@lists.infradead.org, fsverity@lists.linux.dev, Hao Luo , Theodore Ts'o , Stephen Boyd , Muchun Song , Florian Westphal , Robin Holt , "David S. Miller" , Jon Maloy , Jarkko Sakkinen , Eric Biederman , Anna Schumaker , Daniel Bristot de Oliveira , Mike Kravetz , Marcelo Ricardo Leitner , linux-hams@vger.kernel.org, Nikolay Aleksandrov , Joonas Lahtinen , Alexei Starovoitov , Marc Dionne , Jiri Slaby , linux-afs@lists.infradead.org, Daniel Borkmann , linux-rdma@vger.kernel.org, Dexuan Cui , "Matthew Wilcox \(Oracle\)" , lvs-devel@vger.kernel.org, coda@cs.cmu.edu, Doug Gilbert , "Naveen N. Rao" , Gerald Schaefer , Paolo Abeni , Pablo Neira Ayuso , "Serge E. Hallyn" , Kees Cook , Arnd Bergmann , Haiyang Zhang , intel-gfx@lists.freedesktop.org, Steven Rostedt , linux-crypto@vger.kernel.org, Borislav Petkov , Rodrigo Vivi , openipmi-developer@lists.sourceforge.net, mptcp@lists.linux.dev, Jan Harkes , linux-nfs@vger.kernel.org, "Martin K. Petersen" , linux-mm@kvack.org, Jeff Layton , Andy Lutomirski , linux-xfs@vger.kernel.org, linux-ntfs-dev@lists.sourceforge.net, netdev@vger.kernel.org, Ying Xue , bpf@vger.kernel.org, Sudip Mukherjee , Dietmar Eggemann , Alexander Aring , Jan Kara , Steve Wahl , Peter Zijlstra , Amir Goldstein , KP Singh , David Howells , Joseph Qi , "H. Peter Anvin" , David Airlie , Steffen Klassert , rds-devel@oss.oracle.com, Herbert Xu , linux-scsi@vger.kernel.org, dccp@vger.kernel.org, Mark Fasheh , x86@kernel.org, Russell King , Jason Gunthorpe , Ingo Molnar , Jakub Kicinski , "James E.J. Bottomley" , Joerg Reuter , linux-hyperv@vger.kernel.org, Heiko Carstens , Santosh Shilimkar , apparmor@lists.ubuntu.com, linux-raid@vger.kernel.org, Paul Moore , Juergen Gross , John Johansen , linux-x25@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, David Ahern , kexec@lists.infradead.org, linux-security-module@vger.kernel.org, Benjamin LaHaise , tipc-discussion@lists.sourceforge.net, Daniel Vetter , Martin KaFai Lau , Karsten Graul , Joel Becker Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Wed, 21 Jun 2023, Joel Granados wrote: > On Wed, Jun 21, 2023 at 02:16:55PM +0300, Jani Nikula wrote: >> On Wed, 21 Jun 2023, Joel Granados wrote: >> > Remove the empty end element from all the arrays that are passed to the >> > register sysctl calls. In some files this means reducing the explicit >> > array size by one. Also make sure that we are using the size in >> > ctl_table_header instead of evaluating the .procname element. >> >> Where's the harm in removing the end elements driver by driver? This is >> an unwieldy patch to handle. > > I totally agree. Its a big one!!! but I'm concerned of breaking bisectibility: > * I could for example separate all the removes into separate commits and > then have a final commit that removes the check for the empty element. > But this will leave the tree in a state where the for loop will have > undefined behavior when it looks for the empty end element. It might > or might not work (probably not :) until the final commit where I fix > that. > > * I could also change the logic that looks for the final element, > commit that first and then remove the empty element one commit per > driver after that. But then for all the arrays that still have an > empty element, there would again be undefined behavior as it would > think that the last element is valid (when it is really the sentinel). > > Any ideas on how to get around these? First add size to the register calls, and allow the last one to be sentinel but do not require the sentinel. Start removing sentinels, adjusting the size passed in. Once enough sentinels have been removed, add warning if the final entry is a sentinel. Never really remove the check? (But surely you can rework the logic to not count the number of elements up front, only while iterating.) BR, Jani. >> >> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> > index f43950219ffc..e4d7372afb10 100644 >> > --- a/drivers/gpu/drm/i915/i915_perf.c >> > +++ b/drivers/gpu/drm/i915/i915_perf.c >> > @@ -4884,24 +4884,23 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data, >> > >> > static struct ctl_table oa_table[] = { >> > { >> > - .procname = "perf_stream_paranoid", >> > - .data = &i915_perf_stream_paranoid, >> > - .maxlen = sizeof(i915_perf_stream_paranoid), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = SYSCTL_ONE, >> > - }, >> > + .procname = "perf_stream_paranoid", >> > + .data = &i915_perf_stream_paranoid, >> > + .maxlen = sizeof(i915_perf_stream_paranoid), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = SYSCTL_ONE, >> > + }, >> > { >> > - .procname = "oa_max_sample_rate", >> > - .data = &i915_oa_max_sample_rate, >> > - .maxlen = sizeof(i915_oa_max_sample_rate), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = &oa_sample_rate_hard_limit, >> > - }, >> > - {} >> > + .procname = "oa_max_sample_rate", >> > + .data = &i915_oa_max_sample_rate, >> > + .maxlen = sizeof(i915_oa_max_sample_rate), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = &oa_sample_rate_hard_limit, >> > + } >> > }; >> >> The existing indentation is off, but fixing it doesn't really belong in >> this patch. > > Agreed. But I actually was trying to fix something that checkpatch > flagged. I'll change these back (which will cause this patch to be > flagged). > > An alternative solution would be to fix the indentation as part of the > preparation patches. Tell me what you think. > > Thx > >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center 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 BAEF7EB64DA for ; Thu, 22 Jun 2023 07:32:45 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9345010E4DE; Thu, 22 Jun 2023 07:32:30 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4956F10E474; Wed, 21 Jun 2023 13:17:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1687353447; x=1718889447; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=9eMWiG7JI6/9oPvFtdl/3iOGYIkGUexXwjKhm4v9hgQ=; b=VKdU+4JgbX4NpbUJ1pGQwx7rP4tCzihjtawrYes6gzCDNYtQiih+jwVe F873dwIsvhod0YARO9VKUhkj06aRarr7vuGmzoEiVQ5fk5wly5W6omK9t +iNA+hX6lctBvXKvd6x1QxXNWe7qto5MfagiH0EzUG6mn84V9O5UOzwoq F/aRtyuUwktofQl51JDVKYz5pF4rWhcdEX3XoC7ki2x3sTgM+O6J9yJog 5JEgyf/oEuKfTSadtzoz9HPLrnTtASM+FNLftPl81nPy05SF7GnHL/9Ot 22pUVV6zo5lB1BC+Z8d7QHj5e58COuQdxXBIR0/wXLEUCpqKND8y5XrMz w==; X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="446546927" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="446546927" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2023 06:16:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10747"; a="664658294" X-IronPort-AV: E=Sophos;i="6.00,260,1681196400"; d="scan'208";a="664658294" Received: from unknown (HELO localhost) ([10.237.66.162]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jun 2023 06:15:49 -0700 From: Jani Nikula To: Joel Granados Subject: Re: [PATCH 09/11] sysctl: Remove the end element in sysctl table arrays In-Reply-To: <20230621130614.s36w4u7dzmb5d5p3@localhost> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20230621091000.424843-1-j.granados@samsung.com> <20230621094817.433842-1-j.granados@samsung.com> <87o7l92hg8.fsf@intel.com> <20230621130614.s36w4u7dzmb5d5p3@localhost> Date: Wed, 21 Jun 2023 16:15:46 +0300 Message-ID: <878rcd2by5.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain X-Mailman-Approved-At: Thu, 22 Jun 2023 07:32:28 +0000 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Juri Lelli , Miaohe Lin , "Rafael J. Wysocki" , Catalin Marinas , dri-devel@lists.freedesktop.org, Ben Segall , linux-sctp@vger.kernel.org, ocfs2-devel@oss.oracle.com, Miquel Raynal , Alexander Gordeev , "K. Y. Srinivasan" , Stefan Schmidt , Wei Liu , Vincent Guittot , Michael Ellerman , bridge@lists.linux-foundation.org, James Morris , Christophe Leroy , Jozsef Kadlecsik , Eric Biggers , linux-cachefs@redhat.com, Mel Gorman , "Darrick J. Wong" , Waiman Long , Christian Borntraeger , Petr Mladek , Martin Schiller , Russ Weight , Tvrtko Ursulin , Boqun Feng , Nicholas Piggin , John Ogness , Alexander Viro , Andy Lutomirski , Remi Denis-Courmont , xen-devel@lists.xenproject.org, Thomas Gleixner , Trond Myklebust , Anton Altaparmakov , Christian Brauner , Will Drewry , Neil Horman , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, Ralf Baechle , Sergey Senozhatsky , mcgrof@kernel.org, Chuck Lever , netfilter-devel@vger.kernel.org, Masami Hiramatsu , Jiri Olsa , linux-fsdevel@vger.kernel.org, Matthieu Baerts , Andrew Morton , linux-trace-kernel@vger.kernel.org, linux-wpan@vger.kernel.org, Mark Rutland , linux-aio@kvack.org, "Jason A. Donenfeld" , linux-ia64@vger.kernel.org, Naoya Horiguchi , Dave Hansen , Clemens Ladisch , Phillip Potter , Song Liu , Eric Dumazet , keyrings@vger.kernel.org, John Stultz , Stanislav Fomichev , Jan Karcher , codalist@telemann.coda.cs.cmu.edu, linux-s390@vger.kernel.org, Valentin Schneider , Stefano Stabellini , Corey Minyard , Leon Romanovsky , Oleksandr Tyshchenko , Will Deacon , John Fastabend , Andrii Nakryiko , Anil S Keshavamurthy , Mat Martineau , Matthew Bobrowski , Julian Anastasov , coreteam@netfilter.org, Roopa Prabhu , Yonghong Song , Iurii Zaikin , Sven Schnelle , Vasily Gorbik , Mike Travis , Wenjia Zhang , Simon Horman , Xin Long , linux-arm-kernel@lists.infradead.org, fsverity@lists.linux.dev, Hao Luo , Theodore Ts'o , Stephen Boyd , Muchun Song , Florian Westphal , Robin Holt , "David S. Miller" , Jon Maloy , Jarkko Sakkinen , Eric Biederman , Anna Schumaker , Daniel Bristot de Oliveira , Mike Kravetz , Marcelo Ricardo Leitner , linux-hams@vger.kernel.org, Nikolay Aleksandrov , Alexei Starovoitov , Marc Dionne , Jiri Slaby , linux-afs@lists.infradead.org, Daniel Borkmann , linux-rdma@vger.kernel.org, Dexuan Cui , "Matthew Wilcox \(Oracle\)" , lvs-devel@vger.kernel.org, coda@cs.cmu.edu, Doug Gilbert , "Naveen N. Rao" , Gerald Schaefer , Paolo Abeni , Pablo Neira Ayuso , "Serge E. Hallyn" , Kees Cook , Arnd Bergmann , Haiyang Zhang , intel-gfx@lists.freedesktop.org, Steven Rostedt , linux-crypto@vger.kernel.org, Borislav Petkov , Rodrigo Vivi , openipmi-developer@lists.sourceforge.net, mptcp@lists.linux.dev, Jan Harkes , linux-nfs@vger.kernel.org, "Martin K. Petersen" , linux-mm@kvack.org, Jeff Layton , Andy Lutomirski , linux-xfs@vger.kernel.org, linux-ntfs-dev@lists.sourceforge.net, netdev@vger.kernel.org, Ying Xue , bpf@vger.kernel.org, Sudip Mukherjee , Dietmar Eggemann , Alexander Aring , Jan Kara , Steve Wahl , Peter Zijlstra , Balbir Singh , Amir Goldstein , KP Singh , David Howells , Joseph Qi , "H. Peter Anvin" , Steffen Klassert , rds-devel@oss.oracle.com, Herbert Xu , linux-scsi@vger.kernel.org, dccp@vger.kernel.org, Mark Fasheh , x86@kernel.org, Russell King , Jason Gunthorpe , Ingo Molnar , Jakub Kicinski , "James E.J. Bottomley" , Joerg Reuter , linux-hyperv@vger.kernel.org, Heiko Carstens , Santosh Shilimkar , apparmor@lists.ubuntu.com, linux-raid@vger.kernel.org, Paul Moore , Juergen Gross , John Johansen , linux-x25@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, David Ahern , kexec@lists.infradead.org, linux-security-module@vger.kernel.org, Benjamin LaHaise , tipc-discussion@lists.sourceforge.net, Martin KaFai Lau , Karsten Graul , Joel Becker Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Wed, 21 Jun 2023, Joel Granados wrote: > On Wed, Jun 21, 2023 at 02:16:55PM +0300, Jani Nikula wrote: >> On Wed, 21 Jun 2023, Joel Granados wrote: >> > Remove the empty end element from all the arrays that are passed to the >> > register sysctl calls. In some files this means reducing the explicit >> > array size by one. Also make sure that we are using the size in >> > ctl_table_header instead of evaluating the .procname element. >> >> Where's the harm in removing the end elements driver by driver? This is >> an unwieldy patch to handle. > > I totally agree. Its a big one!!! but I'm concerned of breaking bisectibility: > * I could for example separate all the removes into separate commits and > then have a final commit that removes the check for the empty element. > But this will leave the tree in a state where the for loop will have > undefined behavior when it looks for the empty end element. It might > or might not work (probably not :) until the final commit where I fix > that. > > * I could also change the logic that looks for the final element, > commit that first and then remove the empty element one commit per > driver after that. But then for all the arrays that still have an > empty element, there would again be undefined behavior as it would > think that the last element is valid (when it is really the sentinel). > > Any ideas on how to get around these? First add size to the register calls, and allow the last one to be sentinel but do not require the sentinel. Start removing sentinels, adjusting the size passed in. Once enough sentinels have been removed, add warning if the final entry is a sentinel. Never really remove the check? (But surely you can rework the logic to not count the number of elements up front, only while iterating.) BR, Jani. >> >> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c >> > index f43950219ffc..e4d7372afb10 100644 >> > --- a/drivers/gpu/drm/i915/i915_perf.c >> > +++ b/drivers/gpu/drm/i915/i915_perf.c >> > @@ -4884,24 +4884,23 @@ int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data, >> > >> > static struct ctl_table oa_table[] = { >> > { >> > - .procname = "perf_stream_paranoid", >> > - .data = &i915_perf_stream_paranoid, >> > - .maxlen = sizeof(i915_perf_stream_paranoid), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = SYSCTL_ONE, >> > - }, >> > + .procname = "perf_stream_paranoid", >> > + .data = &i915_perf_stream_paranoid, >> > + .maxlen = sizeof(i915_perf_stream_paranoid), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = SYSCTL_ONE, >> > + }, >> > { >> > - .procname = "oa_max_sample_rate", >> > - .data = &i915_oa_max_sample_rate, >> > - .maxlen = sizeof(i915_oa_max_sample_rate), >> > - .mode = 0644, >> > - .proc_handler = proc_dointvec_minmax, >> > - .extra1 = SYSCTL_ZERO, >> > - .extra2 = &oa_sample_rate_hard_limit, >> > - }, >> > - {} >> > + .procname = "oa_max_sample_rate", >> > + .data = &i915_oa_max_sample_rate, >> > + .maxlen = sizeof(i915_oa_max_sample_rate), >> > + .mode = 0644, >> > + .proc_handler = proc_dointvec_minmax, >> > + .extra1 = SYSCTL_ZERO, >> > + .extra2 = &oa_sample_rate_hard_limit, >> > + } >> > }; >> >> The existing indentation is off, but fixing it doesn't really belong in >> this patch. > > Agreed. But I actually was trying to fix something that checkpatch > flagged. I'll change these back (which will cause this patch to be > flagged). > > An alternative solution would be to fix the indentation as part of the > preparation patches. Tell me what you think. > > Thx > >> >> BR, >> Jani. >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center