[PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Uwe Kleine-König]

With this knob you can enable pstore recording early enough to debug
hangs happening during the boot process before userspace is up enough to
enable it via sysfs.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 fs/pstore/ftrace.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c
index 5939595f0115..92d35ee83c95 100644
--- a/fs/pstore/ftrace.c
+++ b/fs/pstore/ftrace.c
@@ -117,13 +117,26 @@ static const struct file_operations pstore_knob_fops = {
 
 static struct dentry *pstore_ftrace_dir;
 
+static unsigned int record_ftrace_init;
+module_param(record_ftrace_init, uint, 0400);
+
 void pstore_register_ftrace(void)
 {
+	int ret;
+
 	if (!psinfo->write)
 		return;
 
 	pstore_ftrace_dir = debugfs_create_dir("pstore", NULL);
 
+	if (record_ftrace_init) {
+		ret = register_ftrace_function(&pstore_ftrace_ops);
+		if (ret)
+			return;
+
+		pstore_ftrace_enabled = 1;
+	}
+
 	debugfs_create_file("record_ftrace", 0600, pstore_ftrace_dir, NULL,
 			    &pstore_knob_fops);
 }
-- 
2.29.2

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Steven Rostedt]

On Thu, 10 Jun 2021 10:21:34 +0200
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> With this knob you can enable pstore recording early enough to debug
> hangs happening during the boot process before userspace is up enough to
> enable it via sysfs.
> 

Is this still needed? It's still in my internal patchwork, and I haven't
seen any responses.

-- Steve


> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  fs/pstore/ftrace.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c
> index 5939595f0115..92d35ee83c95 100644
> --- a/fs/pstore/ftrace.c
> +++ b/fs/pstore/ftrace.c
> @@ -117,13 +117,26 @@ static const struct file_operations pstore_knob_fops = {
>  
>  static struct dentry *pstore_ftrace_dir;
>  
> +static unsigned int record_ftrace_init;
> +module_param(record_ftrace_init, uint, 0400);
> +
>  void pstore_register_ftrace(void)
>  {
> +	int ret;
> +
>  	if (!psinfo->write)
>  		return;
>  
>  	pstore_ftrace_dir = debugfs_create_dir("pstore", NULL);
>  
> +	if (record_ftrace_init) {
> +		ret = register_ftrace_function(&pstore_ftrace_ops);
> +		if (ret)
> +			return;
> +
> +		pstore_ftrace_enabled = 1;
> +	}
> +
>  	debugfs_create_file("record_ftrace", 0600, pstore_ftrace_dir, NULL,
>  			    &pstore_knob_fops);
>  }

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

Hello Steven,

On Tue, Nov 16, 2021 at 12:06:03PM -0500, Steven Rostedt wrote:
> On Thu, 10 Jun 2021 10:21:34 +0200
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > With this knob you can enable pstore recording early enough to debug
> > hangs happening during the boot process before userspace is up enough to
> > enable it via sysfs.
> 
> Is this still needed? It's still in my internal patchwork, and I haven't
> seen any responses.

I didn't see any responses either, and unless someone else implemented
something similar somewhere else, it's still needed.

The change was actually useful to debug a clk problem, where the machine
freezed when a certain driver was loaded.

Thanks for keeping track of this change. I would only have noticed it
missing next time when I have to debug a similar problem.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Steven Rostedt]

On Tue, 16 Nov 2021 18:28:35 +0100
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:

> > Is this still needed? It's still in my internal patchwork, and I haven't
> > seen any responses.  
> 
> I didn't see any responses either, and unless someone else implemented
> something similar somewhere else, it's still needed.
> 
> The change was actually useful to debug a clk problem, where the machine
> freezed when a certain driver was loaded.

Perhaps I should ask this. Would anyone object if I just take this change
through my tree for the next merge window?

-- Steve


> 
> Thanks for keeping track of this change. I would only have noticed it
> missing next time when I have to debug a similar problem.

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

Hello Steven,

On Tue, Nov 16, 2021 at 12:37:05PM -0500, Steven Rostedt wrote:
> On Tue, 16 Nov 2021 18:28:35 +0100
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > > Is this still needed? It's still in my internal patchwork, and I haven't
> > > seen any responses.  
> > 
> > I didn't see any responses either, and unless someone else implemented
> > something similar somewhere else, it's still needed.
> > 
> > The change was actually useful to debug a clk problem, where the machine
> > freezed when a certain driver was loaded.
> 
> Perhaps I should ask this. Would anyone object if I just take this change
> through my tree for the next merge window?

I don't know.

(To be interpreted as in
https://math.stackexchange.com/questions/1884607/does-everyone-want-coffee-propositional-logic-problem :-)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Kees Cook]

On Tue, Nov 16, 2021 at 12:37:05PM -0500, Steven Rostedt wrote:
> On Tue, 16 Nov 2021 18:28:35 +0100
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> 
> > > Is this still needed? It's still in my internal patchwork, and I haven't
> > > seen any responses.  
> > 
> > I didn't see any responses either, and unless someone else implemented
> > something similar somewhere else, it's still needed.
> > 
> > The change was actually useful to debug a clk problem, where the machine
> > freezed when a certain driver was loaded.
> 
> Perhaps I should ask this. Would anyone object if I just take this change
> through my tree for the next merge window?

If you can Ack it, I'll take it via the pstore tree; I've got a few
other things that will likely need to go in for the next merge window
too.

-Kees

-- 
Kees Cook

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Kees Cook]

On Thu, 10 Jun 2021 10:21:34 +0200, Uwe Kleine-König wrote:
> With this knob you can enable pstore recording early enough to debug
> hangs happening during the boot process before userspace is up enough to
> enable it via sysfs.
> 
> 

I refactored this patch a fair bit so it would use a common enable/disable
routine, but otherwise the original intent remains. :)

Applied to for-next/pstore, thanks!

[1/1] pstore/ftrace: add a kernel parameter to start pstore recording
      https://git.kernel.org/kees/c/8d74118c9441

-- 
Kees Cook

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Kees Cook]

On Thu, Nov 18, 2021 at 10:08:38AM -0800, Kees Cook wrote:
> On Thu, 10 Jun 2021 10:21:34 +0200, Uwe Kleine-König wrote:
> > With this knob you can enable pstore recording early enough to debug
> > hangs happening during the boot process before userspace is up enough to
> > enable it via sysfs.
> > 
> > 
> 
> I refactored this patch a fair bit so it would use a common enable/disable
> routine, but otherwise the original intent remains. :)
> 
> Applied to for-next/pstore, thanks!
> 
> [1/1] pstore/ftrace: add a kernel parameter to start pstore recording
>       https://git.kernel.org/kees/c/8d74118c9441

Hrm, sha shenanigans. This is actually:

	https://git.kernel.org/kees/c/a5d05b07961a

-- 
Kees Cook

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]

Hello Kees,

On Thu, Nov 18, 2021 at 10:31:26AM -0800, Kees Cook wrote:
> On Thu, Nov 18, 2021 at 10:08:38AM -0800, Kees Cook wrote:
> > On Thu, 10 Jun 2021 10:21:34 +0200, Uwe Kleine-König wrote:
> > > With this knob you can enable pstore recording early enough to debug
> > > hangs happening during the boot process before userspace is up enough to
> > > enable it via sysfs.
> > > 
> > > 
> > 
> > I refactored this patch a fair bit so it would use a common enable/disable
> > routine, but otherwise the original intent remains. :)
> > 
> > Applied to for-next/pstore, thanks!
> > 
> > [1/1] pstore/ftrace: add a kernel parameter to start pstore recording
> >       https://git.kernel.org/kees/c/8d74118c9441
> 
> Hrm, sha shenanigans. This is actually:
> 
> 	https://git.kernel.org/kees/c/a5d05b07961a

\o/ Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] pstore/ftrace: add a kernel parameter to start pstore recording

[Steven Rostedt]

On Tue, 16 Nov 2021 13:00:07 -0800
Kees Cook <keescook@chromium.org> wrote:

> On Tue, Nov 16, 2021 at 12:37:05PM -0500, Steven Rostedt wrote:
> > On Tue, 16 Nov 2021 18:28:35 +0100
> > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> >   
> > > > Is this still needed? It's still in my internal patchwork, and I haven't
> > > > seen any responses.    
> > > 
> > > I didn't see any responses either, and unless someone else implemented
> > > something similar somewhere else, it's still needed.
> > > 
> > > The change was actually useful to debug a clk problem, where the machine
> > > freezed when a certain driver was loaded.  
> > 
> > Perhaps I should ask this. Would anyone object if I just take this change
> > through my tree for the next merge window?  
> 
> If you can Ack it, I'll take it via the pstore tree; I've got a few
> other things that will likely need to go in for the next merge window
> too.
>

I've must have missed this.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve