Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <f2fe220f-70c9-7b95-a9cb-4709752e4bdc@igalia.com>
Date:   Sat, 2 Apr 2022 11:01:51 -0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [RFC 0/1] Add sysctl entry for controlling
 crash_kexec_post_notifiers
Content-Language: en-US
To:     Alejandro Jimenez <alejandro.j.jimenez@oracle.com>, corbet@lwn.net,
        mcgrof@kernel.org, keescook@chromium.org, yzaikin@google.com,
        linux-fsdevel@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     akpm@linux-foundation.org, linux@rasmusvillemoes.dk,
        ebiggers@google.com, peterz@infradead.org, ying.huang@intel.com,
        mchehab+huawei@kernel.org, Jason@zx2c4.com, daniel@iogearbox.net,
        robh@kernel.org, wangqing@vivo.com, prestwoj@gmail.com,
        dsahern@kernel.org, stephen.s.brennan@oracle.com
References: <20220401202300.12660-1-alejandro.j.jimenez@oracle.com>
From:   "Guilherme G. Piccoli" <gpiccoli@igalia.com>
In-Reply-To: <20220401202300.12660-1-alejandro.j.jimenez@oracle.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 01/04/2022 17:22, Alejandro Jimenez wrote:
> I noticed that in contrast to other kernel core parameters (e.g. kernel.panic,
> kernel.panic_on_warn, kernel.panic_print) crash_kexec_post_notifiers is not
> available as a sysctl tunable. I am aware that because it is a kernel core
> parameter, there is already an entry under:
> 
>   /sys/module/kernel/parameters/crash_kexec_post_notifiers
> 
> and that allows us to read/modify it at runtime. However, I thought it should
> also be available via sysctl, since users that want to read/set this value at
> runtime might look there first.
> 
> I believe there is an ongoing effort to clean up kernel/sysctl.c, but it wasn't
> clear to me if this entry (and perhaps the other panic related entries too)
> should be placed on kernel/panic.c. I wanted to verify first that this change
> would be welcomed before doing additional refactoring work.
> 
> I'd appreciate any comments or suggestions.
> 
> Thank you,
> Alejandro

Hi Alejandro, thanks for you patch. I have a "selfish" concern though,
I'll expose it here.

I'm working a panic refactor, in order to split the panic notifiers in
more lists - good summary of this discussion at [0].
I'm in the half of the patches, hopefully next 2 weeks I have something
ready to submit (I'll be out next week).

As part of this effort, I plan to have a more fine-grained control of
this parameter, and it's going to be a sysctl, but not
"crash_kexec_post_notifiers" - this one should be kept I guess due to
retro-compatibility, but it'd be a layer on top oh the new one.
With that said, unless you have urgent needs for this patch to be
reviewed/merged , I'd like to ask you to wait the series and I can loop
you there, so you may review/comment and see if it fits your use case.

Thanks,


Guilherme


[0] https://lore.kernel.org/lkml/YfPxvzSzDLjO5ldp@alley/