Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Eric W. Biederman" <ebiederm@xmission.com>
To:     Jan Kara <jack@suse.cz>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        kernel test robot <oliver.sang@intel.com>,
        Luis Chamberlain <mcgrof@kernel.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Amir Goldstein <amir73il@gmail.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Antti Palosaari <crope@iki.fi>, Arnd Bergmann <arnd@arndb.de>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Benjamin LaHaise <bcrl@kvack.org>,
        Clemens Ladisch <clemens@ladisch.de>,
        David Airlie <airlied@linux.ie>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Iurii Zaikin <yzaikin@google.com>,
        Jani Nikula <jani.nikula@linux.intel.com>,
        Joel Becker <jlbec@evilplan.org>,
        Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
        Joseph Qi <joseph.qi@linux.alibaba.com>,
        Julia Lawall <julia.lawall@inria.fr>,
        Kees Cook <keescook@chromium.org>,
        Lukas Middendorf <kernel@tuxforce.de>,
        Mark Fasheh <mark@fasheh.com>, Paul Turner <pjt@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Petr Mladek <pmladek@suse.com>,
        Phillip Potter <phil@philpotter.co.uk>,
        Qing Wang <wangqing@vivo.com>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Sebastian Reichel <sre@kernel.org>,
        Sergey Senozhatsky <senozhatsky@chromium.org>,
        Stephen Kitt <steve@sk2.org>,
        Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
        Xiaoming Ni <nixiaoming@huawei.com>,
        Douglas Gilbert <dgilbert@interlog.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        Jani Nikula <jani.nikula@intel.com>,
        John Ogness <john.ogness@linutronix.de>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Suren Baghdasaryan <surenb@google.com>,
        Theodore Ts'o <tytso@mit.edu>,
        Andrew Morton <akpm@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>, lkp@lists.01.org,
        kernel test robot <lkp@intel.com>
References: <20220128065310.GF8421@xsang-OptiPlex-9020>
        <CAHk-=wgT6wuBB=pK2bGNQbioNCdJHT8Jnvs+4oT25jX1JFyhgg@mail.gmail.com>
        <20220128120421.kzo5iduigr7k55bs@quack3.lan>
Date:   Fri, 28 Jan 2022 10:49:16 -0600
In-Reply-To: <20220128120421.kzo5iduigr7k55bs@quack3.lan> (Jan Kara's message
        of "Fri, 28 Jan 2022 13:04:21 +0100")
Message-ID: <87a6ffst4j.fsf@email.froward.int.ebiederm.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [ocfs2] c42ff46f97: sysctl_table_check_failed
Precedence: bulk

Jan Kara <jack@suse.cz> writes:

> On Fri 28-01-22 10:00:29, Linus Torvalds wrote:
>> On Fri, Jan 28, 2022 at 8:53 AM kernel test robot <oliver.sang@intel.com> wrote:
>> >
>> > commit: 46e33fd45a52bf03769906e64d8a8a1ab317777d ("ocfs2: simplify subdirectory
>> > registration with register_sysctl()")
>> 
>> Well, it's apparently commit c42ff46f97c1 ("ocfs2: simplify
>> subdirectory registration with register_sysctl()") in mainline now.
>> 
>> What worries me a bit is that the commit was auto-generated, and when
>> reading the commit message it reads as if it wasn't supposed to cause
>> any semantic changes at all.
>> 
>> Is the cause of this that 'nm' is supposed to be a directory, and
>> register_sysctl() doesn't handle directories?
>> 
>> I don't know this code at all, should it have been simplified even
>> further with something (TOTALLY UNTESTED) like the attached?
>
> Yep, I've tested the patch and it fixes the failure for me. Feel free to
> add:
>
> Tested-by: Jan Kara <jack@suse.cz>
>
> Also the change makes sense to me as far as I'm reading register_sysctl()
> so you can also add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>

Yes.  There is a register_sysctl_paths that can be used if you want/need
the embedded directories.  That probably would have been a better
choice for an automated conversion.

But since this there is only the single file in a single directory
register_sysctl() with the full path is perfectly fine in this case.

Reviewed-by: "Eric W. Biederman" <ebiederm@xmission.com>

>> diff --git a/fs/ocfs2/stackglue.c b/fs/ocfs2/stackglue.c
>> index 731558a6f27d..dd77b7aaabf5 100644
>> --- a/fs/ocfs2/stackglue.c
>> +++ b/fs/ocfs2/stackglue.c
>> @@ -661,17 +661,6 @@ static struct ctl_table ocfs2_nm_table[] = {
>>  	{ }
>>  };
>>  
>> -static struct ctl_table ocfs2_mod_table[] = {
>> -	{
>> -		.procname	= "nm",
>> -		.data		= NULL,
>> -		.maxlen		= 0,
>> -		.mode		= 0555,
>> -		.child		= ocfs2_nm_table
>> -	},
>> -	{ }
>> -};
>> -
>>  static struct ctl_table_header *ocfs2_table_header;
>>  
>>  /*
>> @@ -682,7 +671,7 @@ static int __init ocfs2_stack_glue_init(void)
>>  {
>>  	strcpy(cluster_stack_name, OCFS2_STACK_PLUGIN_O2CB);
>>  
>> -	ocfs2_table_header = register_sysctl("fs/ocfs2", ocfs2_mod_table);
>> +	ocfs2_table_header = register_sysctl("fs/ocfs2/nm", ocfs2_nm_table);
>>  	if (!ocfs2_table_header) {
>>  		printk(KERN_ERR
>>  		       "ocfs2 stack glue: unable to register sysctl\n");