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: netdev@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