[PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[Petr Tesarik]

Hello all,

this patch series makes it possible to save a kernel crash dump when the
kernel command line includes "crash_kexec_post_notifiers". There might
be other approaches, but mine has the advantage that no new sysctl is
required, and the behaviour is the same whether panic notifiers are run
or not: If you load a crash kernel with kexec, it will be used, otherwise
the hypervisor facility is used (using a hypercall).

Feedback welcome!

Petr T

---

Petr Tesarik (2):
      Add a kexec_crash_loaded() function
      Allow kdump with crash_kexec_post_notifiers


 arch/x86/xen/enlighten.c |    3 ++-
 include/linux/kexec.h    |    2 ++
 kernel/kexec_core.c      |    5 +++++
 kernel/ksysfs.c          |    2 +-
 4 files changed, 10 insertions(+), 2 deletions(-)

--
Signature

[PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[Petr Tesarik]

Hello all,

this patch series makes it possible to save a kernel crash dump when the
kernel command line includes "crash_kexec_post_notifiers". There might
be other approaches, but mine has the advantage that no new sysctl is
required, and the behaviour is the same whether panic notifiers are run
or not: If you load a crash kernel with kexec, it will be used, otherwise
the hypervisor facility is used (using a hypercall).

Feedback welcome!

Petr T

---

Petr Tesarik (2):
      Add a kexec_crash_loaded() function
      Allow kdump with crash_kexec_post_notifiers


 arch/x86/xen/enlighten.c |    3 ++-
 include/linux/kexec.h    |    2 ++
 kernel/kexec_core.c      |    5 +++++
 kernel/ksysfs.c          |    2 +-
 4 files changed, 10 insertions(+), 2 deletions(-)

--
Signature

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 1/2] Add a kexec_crash_loaded() function

[Petr Tesarik]

Provide a wrapper function to be used by kernel code to check whether
a crash kernel is loaded. It returns the same value that can be seen
in /sys/kernel/kexec_crash_loaded by userspace programs.

I'm exporting the function, because it will be used by Xen, and it is
possible to compile Xen modules separately to enable the use of PV
drivers with unmodified bare-metal kernels.

Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
 include/linux/kexec.h |    2 ++
 kernel/kexec_core.c   |    6 ++++++
 kernel/ksysfs.c       |    2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index e8acb2b..d461d02 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -228,6 +228,7 @@ extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
+int kexec_crash_loaded(void);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
 void crash_save_vmcoreinfo(void);
 void arch_crash_save_vmcoreinfo(void);
@@ -324,6 +325,7 @@ struct task_struct;
 static inline void __crash_kexec(struct pt_regs *regs) { }
 static inline void crash_kexec(struct pt_regs *regs) { }
 static inline int kexec_should_crash(struct task_struct *p) { return 0; }
+static inline int kexec_crash_loaded(void) { return 0; }
 #define kexec_in_progress false
 #endif /* CONFIG_KEXEC_CORE */
 
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 56b3ed0..a303dce 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
 	return 0;
 }
 
+int kexec_crash_loaded(void)
+{
+	return !!kexec_crash_image;
+}
+EXPORT_SYMBOL_GPL(kexec_crash_loaded);
+
 /*
  * When kexec transitions to the new kernel there is a one-to-one
  * mapping between physical and virtual addresses.  On processors
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index 152da4a..bf9fc9d 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -101,7 +101,7 @@ KERNEL_ATTR_RO(kexec_loaded);
 static ssize_t kexec_crash_loaded_show(struct kobject *kobj,
 				       struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n", !!kexec_crash_image);
+	return sprintf(buf, "%d\n", kexec_crash_loaded());
 }
 KERNEL_ATTR_RO(kexec_crash_loaded);
 

[PATCH 1/2] Add a kexec_crash_loaded() function

[Petr Tesarik]

Provide a wrapper function to be used by kernel code to check whether
a crash kernel is loaded. It returns the same value that can be seen
in /sys/kernel/kexec_crash_loaded by userspace programs.

I'm exporting the function, because it will be used by Xen, and it is
possible to compile Xen modules separately to enable the use of PV
drivers with unmodified bare-metal kernels.

Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
 include/linux/kexec.h |    2 ++
 kernel/kexec_core.c   |    6 ++++++
 kernel/ksysfs.c       |    2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index e8acb2b..d461d02 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -228,6 +228,7 @@ extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
+int kexec_crash_loaded(void);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
 void crash_save_vmcoreinfo(void);
 void arch_crash_save_vmcoreinfo(void);
@@ -324,6 +325,7 @@ struct task_struct;
 static inline void __crash_kexec(struct pt_regs *regs) { }
 static inline void crash_kexec(struct pt_regs *regs) { }
 static inline int kexec_should_crash(struct task_struct *p) { return 0; }
+static inline int kexec_crash_loaded(void) { return 0; }
 #define kexec_in_progress false
 #endif /* CONFIG_KEXEC_CORE */
 
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 56b3ed0..a303dce 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
 	return 0;
 }
 
+int kexec_crash_loaded(void)
+{
+	return !!kexec_crash_image;
+}
+EXPORT_SYMBOL_GPL(kexec_crash_loaded);
+
 /*
  * When kexec transitions to the new kernel there is a one-to-one
  * mapping between physical and virtual addresses.  On processors
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index 152da4a..bf9fc9d 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -101,7 +101,7 @@ KERNEL_ATTR_RO(kexec_loaded);
 static ssize_t kexec_crash_loaded_show(struct kobject *kobj,
 				       struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n", !!kexec_crash_image);
+	return sprintf(buf, "%d\n", kexec_crash_loaded());
 }
 KERNEL_ATTR_RO(kexec_crash_loaded);
 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 1/2] Add a kexec_crash_loaded() function

[Petr Tesarik]

Provide a wrapper function to be used by kernel code to check whether
a crash kernel is loaded. It returns the same value that can be seen
in /sys/kernel/kexec_crash_loaded by userspace programs.

I'm exporting the function, because it will be used by Xen, and it is
possible to compile Xen modules separately to enable the use of PV
drivers with unmodified bare-metal kernels.

Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
 include/linux/kexec.h |    2 ++
 kernel/kexec_core.c   |    6 ++++++
 kernel/ksysfs.c       |    2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index e8acb2b..d461d02 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -228,6 +228,7 @@ extern void *kexec_purgatory_get_symbol_addr(struct kimage *image,
 extern void __crash_kexec(struct pt_regs *);
 extern void crash_kexec(struct pt_regs *);
 int kexec_should_crash(struct task_struct *);
+int kexec_crash_loaded(void);
 void crash_save_cpu(struct pt_regs *regs, int cpu);
 void crash_save_vmcoreinfo(void);
 void arch_crash_save_vmcoreinfo(void);
@@ -324,6 +325,7 @@ struct task_struct;
 static inline void __crash_kexec(struct pt_regs *regs) { }
 static inline void crash_kexec(struct pt_regs *regs) { }
 static inline int kexec_should_crash(struct task_struct *p) { return 0; }
+static inline int kexec_crash_loaded(void) { return 0; }
 #define kexec_in_progress false
 #endif /* CONFIG_KEXEC_CORE */
 
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 56b3ed0..a303dce 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
 	return 0;
 }
 
+int kexec_crash_loaded(void)
+{
+	return !!kexec_crash_image;
+}
+EXPORT_SYMBOL_GPL(kexec_crash_loaded);
+
 /*
  * When kexec transitions to the new kernel there is a one-to-one
  * mapping between physical and virtual addresses.  On processors
diff --git a/kernel/ksysfs.c b/kernel/ksysfs.c
index 152da4a..bf9fc9d 100644
--- a/kernel/ksysfs.c
+++ b/kernel/ksysfs.c
@@ -101,7 +101,7 @@ KERNEL_ATTR_RO(kexec_loaded);
 static ssize_t kexec_crash_loaded_show(struct kobject *kobj,
 				       struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "%d\n", !!kexec_crash_image);
+	return sprintf(buf, "%d\n", kexec_crash_loaded());
 }
 KERNEL_ATTR_RO(kexec_crash_loaded);
 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Petr Tesarik]

If a crash kernel is loaded, do not crash the running domain. This is
needed if the kernel is loaded with crash_kexec_post_notifiers, because
panic notifiers are run before __crash_kexec() in that case, and this
Xen hook prevents its being called later.

Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
 arch/x86/xen/enlighten.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 760789a..6e3e7c6 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1325,7 +1325,8 @@ static void xen_crash_shutdown(struct pt_regs *regs)
 static int
 xen_panic_event(struct notifier_block *this, unsigned long event, void *ptr)
 {
-	xen_reboot(SHUTDOWN_crash);
+	if (!kexec_crash_loaded())
+		xen_reboot(SHUTDOWN_crash);
 	return NOTIFY_DONE;
 }
 

[PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Petr Tesarik]

If a crash kernel is loaded, do not crash the running domain. This is
needed if the kernel is loaded with crash_kexec_post_notifiers, because
panic notifiers are run before __crash_kexec() in that case, and this
Xen hook prevents its being called later.

Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
 arch/x86/xen/enlighten.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 760789a..6e3e7c6 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1325,7 +1325,8 @@ static void xen_crash_shutdown(struct pt_regs *regs)
 static int
 xen_panic_event(struct notifier_block *this, unsigned long event, void *ptr)
 {
-	xen_reboot(SHUTDOWN_crash);
+	if (!kexec_crash_loaded())
+		xen_reboot(SHUTDOWN_crash);
 	return NOTIFY_DONE;
 }
 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Petr Tesarik]

If a crash kernel is loaded, do not crash the running domain. This is
needed if the kernel is loaded with crash_kexec_post_notifiers, because
panic notifiers are run before __crash_kexec() in that case, and this
Xen hook prevents its being called later.

Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
 arch/x86/xen/enlighten.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 760789a..6e3e7c6 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1325,7 +1325,8 @@ static void xen_crash_shutdown(struct pt_regs *regs)
 static int
 xen_panic_event(struct notifier_block *this, unsigned long event, void *ptr)
 {
-	xen_reboot(SHUTDOWN_crash);
+	if (!kexec_crash_loaded())
+		xen_reboot(SHUTDOWN_crash);
 	return NOTIFY_DONE;
 }
 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[Petr Tesarik]

On Wed, 13 Jul 2016 14:19:50 +0200
Petr Tesarik <ptesarik@suse.com> wrote:

> Hello all,
> 
> this patch series makes it possible to save a kernel crash dump when the
> kernel command line includes "crash_kexec_post_notifiers".

Oh ... I forgot to add: This only applies to running Linux under Xen.
If you run on bare metal, you can always save the dump already, as you
certainly know.

Petr T

> There might
> be other approaches, but mine has the advantage that no new sysctl is
> required, and the behaviour is the same whether panic notifiers are run
> or not: If you load a crash kernel with kexec, it will be used, otherwise
> the hypervisor facility is used (using a hypercall).
> 
> Feedback welcome!
> 
> Petr T
> 
> ---
> 
> Petr Tesarik (2):
>       Add a kexec_crash_loaded() function
>       Allow kdump with crash_kexec_post_notifiers
> 
> 
>  arch/x86/xen/enlighten.c |    3 ++-
>  include/linux/kexec.h    |    2 ++
>  kernel/kexec_core.c      |    5 +++++
>  kernel/ksysfs.c          |    2 +-
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> --
> Signature
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[Petr Tesarik]

On Wed, 13 Jul 2016 14:19:50 +0200
Petr Tesarik <ptesarik@suse.com> wrote:

> Hello all,
> 
> this patch series makes it possible to save a kernel crash dump when the
> kernel command line includes "crash_kexec_post_notifiers".

Oh ... I forgot to add: This only applies to running Linux under Xen.
If you run on bare metal, you can always save the dump already, as you
certainly know.

Petr T

> There might
> be other approaches, but mine has the advantage that no new sysctl is
> required, and the behaviour is the same whether panic notifiers are run
> or not: If you load a crash kernel with kexec, it will be used, otherwise
> the hypervisor facility is used (using a hypercall).
> 
> Feedback welcome!
> 
> Petr T
> 
> ---
> 
> Petr Tesarik (2):
>       Add a kexec_crash_loaded() function
>       Allow kdump with crash_kexec_post_notifiers
> 
> 
>  arch/x86/xen/enlighten.c |    3 ++-
>  include/linux/kexec.h    |    2 ++
>  kernel/kexec_core.c      |    5 +++++
>  kernel/ksysfs.c          |    2 +-
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> --
> Signature
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[Petr Tesarik]

On Wed, 13 Jul 2016 14:19:50 +0200
Petr Tesarik <ptesarik@suse.com> wrote:

> Hello all,
> 
> this patch series makes it possible to save a kernel crash dump when the
> kernel command line includes "crash_kexec_post_notifiers".

Oh ... I forgot to add: This only applies to running Linux under Xen.
If you run on bare metal, you can always save the dump already, as you
certainly know.

Petr T

> There might
> be other approaches, but mine has the advantage that no new sysctl is
> required, and the behaviour is the same whether panic notifiers are run
> or not: If you load a crash kernel with kexec, it will be used, otherwise
> the hypervisor facility is used (using a hypercall).
> 
> Feedback welcome!
> 
> Petr T
> 
> ---
> 
> Petr Tesarik (2):
>       Add a kexec_crash_loaded() function
>       Allow kdump with crash_kexec_post_notifiers
> 
> 
>  arch/x86/xen/enlighten.c |    3 ++-
>  include/linux/kexec.h    |    2 ++
>  kernel/kexec_core.c      |    5 +++++
>  kernel/ksysfs.c          |    2 +-
>  4 files changed, 10 insertions(+), 2 deletions(-)
> 
> --
> Signature
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/2] Add a kexec_crash_loaded() function

[Josh Triplett]

On Wed, Jul 13, 2016 at 02:19:55PM +0200, Petr Tesarik wrote:
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
>  	return 0;
>  }
>  
> +int kexec_crash_loaded(void)
> +{
> +	return !!kexec_crash_image;
> +}

Nit: this function should return bool rather than int, since it
effectively returns true/false.

Re: [PATCH 1/2] Add a kexec_crash_loaded() function

[Josh Triplett]

On Wed, Jul 13, 2016 at 02:19:55PM +0200, Petr Tesarik wrote:
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
>  	return 0;
>  }
>  
> +int kexec_crash_loaded(void)
> +{
> +	return !!kexec_crash_image;
> +}

Nit: this function should return bool rather than int, since it
effectively returns true/false.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] Add a kexec_crash_loaded() function

[Josh Triplett]

On Wed, Jul 13, 2016 at 02:19:55PM +0200, Petr Tesarik wrote:
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
>  	return 0;
>  }
>  
> +int kexec_crash_loaded(void)
> +{
> +	return !!kexec_crash_image;
> +}

Nit: this function should return bool rather than int, since it
effectively returns true/false.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [Xen-devel] [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[David Vrabel]

On 13/07/16 13:20, Petr Tesarik wrote:
> If a crash kernel is loaded, do not crash the running domain. This is
> needed if the kernel is loaded with crash_kexec_post_notifiers, because
> panic notifiers are run before __crash_kexec() in that case, and this
> Xen hook prevents its being called later.

Prioritising the in-kernel kexec image over the hypervisor one seems
sensible behaviour to me.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[David Vrabel]

On 13/07/16 13:20, Petr Tesarik wrote:
> If a crash kernel is loaded, do not crash the running domain. This is
> needed if the kernel is loaded with crash_kexec_post_notifiers, because
> panic notifiers are run before __crash_kexec() in that case, and this
> Xen hook prevents its being called later.

Prioritising the in-kernel kexec image over the hypervisor one seems
sensible behaviour to me.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[David Vrabel]

On 13/07/16 13:20, Petr Tesarik wrote:
> If a crash kernel is loaded, do not crash the running domain. This is
> needed if the kernel is loaded with crash_kexec_post_notifiers, because
> panic notifiers are run before __crash_kexec() in that case, and this
> Xen hook prevents its being called later.

Prioritising the in-kernel kexec image over the hypervisor one seems
sensible behaviour to me.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/2] Add a kexec_crash_loaded() function

[Petr Tesarik]

On Wed, 13 Jul 2016 05:52:33 -0700
Josh Triplett <josh@joshtriplett.org> wrote:

> On Wed, Jul 13, 2016 at 02:19:55PM +0200, Petr Tesarik wrote:
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
> >  	return 0;
> >  }
> >  
> > +int kexec_crash_loaded(void)
> > +{
> > +	return !!kexec_crash_image;
> > +}
> 
> Nit: this function should return bool rather than int, since it
> effectively returns true/false.

I thought about this for a moment. Since the return value should also
be used for the corresponding sysctl, which is an integer, I wasn't
sure if bool is the correct type, especially since other boolean
functions in kexec.h also return int... But that could be legacy.

I've got no problem changing the type to 'bool' if that's what it takes.

Petr T

Re: [PATCH 1/2] Add a kexec_crash_loaded() function

[Petr Tesarik]

On Wed, 13 Jul 2016 05:52:33 -0700
Josh Triplett <josh@joshtriplett.org> wrote:

> On Wed, Jul 13, 2016 at 02:19:55PM +0200, Petr Tesarik wrote:
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
> >  	return 0;
> >  }
> >  
> > +int kexec_crash_loaded(void)
> > +{
> > +	return !!kexec_crash_image;
> > +}
> 
> Nit: this function should return bool rather than int, since it
> effectively returns true/false.

I thought about this for a moment. Since the return value should also
be used for the corresponding sysctl, which is an integer, I wasn't
sure if bool is the correct type, especially since other boolean
functions in kexec.h also return int... But that could be legacy.

I've got no problem changing the type to 'bool' if that's what it takes.

Petr T

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/2] Add a kexec_crash_loaded() function

[Petr Tesarik]

On Wed, 13 Jul 2016 05:52:33 -0700
Josh Triplett <josh@joshtriplett.org> wrote:

> On Wed, Jul 13, 2016 at 02:19:55PM +0200, Petr Tesarik wrote:
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -95,6 +95,12 @@ int kexec_should_crash(struct task_struct *p)
> >  	return 0;
> >  }
> >  
> > +int kexec_crash_loaded(void)
> > +{
> > +	return !!kexec_crash_image;
> > +}
> 
> Nit: this function should return bool rather than int, since it
> effectively returns true/false.

I thought about this for a moment. Since the return value should also
be used for the corresponding sysctl, which is an integer, I wasn't
sure if bool is the correct type, especially since other boolean
functions in kexec.h also return int... But that could be legacy.

I've got no problem changing the type to 'bool' if that's what it takes.

Petr T

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [Xen-devel] [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Jan Beulich]

>>> On 13.07.16 at 14:53, <david.vrabel@citrix.com> wrote:
> On 13/07/16 13:20, Petr Tesarik wrote:
>> If a crash kernel is loaded, do not crash the running domain. This is
>> needed if the kernel is loaded with crash_kexec_post_notifiers, because
>> panic notifiers are run before __crash_kexec() in that case, and this
>> Xen hook prevents its being called later.
> 
> Prioritising the in-kernel kexec image over the hypervisor one seems
> sensible behaviour to me.

For HVM guests certainly; does loading of an in-kernel crash kernel
properly fail for PV guests (and namely PV Dom0), or does such a
setup work nowadays?

Jan

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Jan Beulich]

>>> On 13.07.16 at 14:53, <david.vrabel@citrix.com> wrote:
> On 13/07/16 13:20, Petr Tesarik wrote:
>> If a crash kernel is loaded, do not crash the running domain. This is
>> needed if the kernel is loaded with crash_kexec_post_notifiers, because
>> panic notifiers are run before __crash_kexec() in that case, and this
>> Xen hook prevents its being called later.
> 
> Prioritising the in-kernel kexec image over the hypervisor one seems
> sensible behaviour to me.

For HVM guests certainly; does loading of an in-kernel crash kernel
properly fail for PV guests (and namely PV Dom0), or does such a
setup work nowadays?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Jan Beulich]

>>> On 13.07.16 at 14:53, <david.vrabel@citrix.com> wrote:
> On 13/07/16 13:20, Petr Tesarik wrote:
>> If a crash kernel is loaded, do not crash the running domain. This is
>> needed if the kernel is loaded with crash_kexec_post_notifiers, because
>> panic notifiers are run before __crash_kexec() in that case, and this
>> Xen hook prevents its being called later.
> 
> Prioritising the in-kernel kexec image over the hypervisor one seems
> sensible behaviour to me.

For HVM guests certainly; does loading of an in-kernel crash kernel
properly fail for PV guests (and namely PV Dom0), or does such a
setup work nowadays?

Jan


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Petr Tesarik]

On Mon, 1 Aug 2016 13:55:01 +0200
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 13.07.16 at 14:53, <david.vrabel@citrix.com> wrote:
> > On 13/07/16 13:20, Petr Tesarik wrote:
> >> If a crash kernel is loaded, do not crash the running domain. This is
> >> needed if the kernel is loaded with crash_kexec_post_notifiers, because
> >> panic notifiers are run before __crash_kexec() in that case, and this
> >> Xen hook prevents its being called later.
> > 
> > Prioritising the in-kernel kexec image over the hypervisor one seems
> > sensible behaviour to me.
> 
> For HVM guests certainly; does loading of an in-kernel crash kernel
> properly fail for PV guests (and namely PV Dom0), or does such a
> setup work nowadays?

This is a good question, but I don't think it is relevant to this
patch. It does not change anything unless the kernel is booted with
crash_kexec_post_notifiers.

I fully understand that Dom0 kernels want to load the panic kernel in
the hypervisor and crash the complete machine, rather than just Dom0,
but if you want that behaviour, simply pass the "crashkernel="
parameter only to the Xen hypervisor and not to the Dom0 kernel.

Did I miss something?

Petr T

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Jan Beulich]

(re-adding xen-devel)

>>> On 01.08.16 at 15:02, <PTesarik@suse.com> wrote:
> On Mon, 1 Aug 2016 13:55:01 +0200
> "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>> >>> On 13.07.16 at 14:53, <david.vrabel@citrix.com> wrote:
>> > On 13/07/16 13:20, Petr Tesarik wrote:
>> >> If a crash kernel is loaded, do not crash the running domain. This is
>> >> needed if the kernel is loaded with crash_kexec_post_notifiers, because
>> >> panic notifiers are run before __crash_kexec() in that case, and this
>> >> Xen hook prevents its being called later.
>> > 
>> > Prioritising the in-kernel kexec image over the hypervisor one seems
>> > sensible behaviour to me.
>> 
>> For HVM guests certainly; does loading of an in-kernel crash kernel
>> properly fail for PV guests (and namely PV Dom0), or does such a
>> setup work nowadays?
> 
> This is a good question, but I don't think it is relevant to this
> patch. It does not change anything unless the kernel is booted with
> crash_kexec_post_notifiers.
> 
> I fully understand that Dom0 kernels want to load the panic kernel in
> the hypervisor and crash the complete machine, rather than just Dom0,
> but if you want that behaviour, simply pass the "crashkernel="
> parameter only to the Xen hypervisor and not to the Dom0 kernel.
> 
> Did I miss something?

For one there are still many people who, for varying reasons, add
"crashkernel=" also to Dom0's command line.

And then trying to invoke a locally loaded crash kernel which won't
work is bad - you really want the domain to be reported crashed
instead in such a case, to avoid further obfuscating whatever issue
had been encountered originally.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Petr Tesarik]

On Mon, 1 Aug 2016 15:47:58 +0200
"Jan Beulich" <JBeulich@suse.com> wrote:

> (re-adding xen-devel)
> 
> >>> On 01.08.16 at 15:02, <PTesarik@suse.com> wrote:
> > On Mon, 1 Aug 2016 13:55:01 +0200
> > "Jan Beulich" <JBeulich@suse.com> wrote:
> > 
> >> >>> On 13.07.16 at 14:53, <david.vrabel@citrix.com> wrote:
> >> > On 13/07/16 13:20, Petr Tesarik wrote:
> >> >> If a crash kernel is loaded, do not crash the running domain. This is
> >> >> needed if the kernel is loaded with crash_kexec_post_notifiers, because
> >> >> panic notifiers are run before __crash_kexec() in that case, and this
> >> >> Xen hook prevents its being called later.
> >> > 
> >> > Prioritising the in-kernel kexec image over the hypervisor one seems
> >> > sensible behaviour to me.
> >> 
> >> For HVM guests certainly; does loading of an in-kernel crash kernel
> >> properly fail for PV guests (and namely PV Dom0), or does such a
> >> setup work nowadays?
> > 
> > This is a good question, but I don't think it is relevant to this
> > patch. It does not change anything unless the kernel is booted with
> > crash_kexec_post_notifiers.
> > 
> > I fully understand that Dom0 kernels want to load the panic kernel in
> > the hypervisor and crash the complete machine, rather than just Dom0,
> > but if you want that behaviour, simply pass the "crashkernel="
> > parameter only to the Xen hypervisor and not to the Dom0 kernel.
> > 
> > Did I miss something?
> 
> For one there are still many people who, for varying reasons, add
> "crashkernel=" also to Dom0's command line.

Is there a valid use case for it?

FWIW the legacy Xen implementation (as found in SLES) simply ignores
the 'crashkernel=' kernel parameter. The code is not even compiled in.

> And then trying to invoke a locally loaded crash kernel which won't
> work is bad

Without actually knowing whether a PV kernel can kexec another PV
kernel, this discussion is somewhat moot...

But let me repeat: if PV kexec works, then it has always had priority
over the hypercall. If it doesn't work, then it has always been broken.
For the latter case, I agree that the kernel should not even allow to
load the kexec image, but that's unrelated to my patch.

Has anyone here tried booting up a PV domain and performing kexec(2)?
I can't test it with my SLES12 installation, because the kexec(8)
user-space utility is patched in SLES to load the hypervisor kexec
image with a hypercall if Xen is detected.

Petr T

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Jan Beulich]

>>> On 01.08.16 at 16:15, <PTesarik@suse.com> wrote:
> On Mon, 1 Aug 2016 15:47:58 +0200
> "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>> (re-adding xen-devel)
>> 
>> >>> On 01.08.16 at 15:02, <PTesarik@suse.com> wrote:
>> > On Mon, 1 Aug 2016 13:55:01 +0200
>> > "Jan Beulich" <JBeulich@suse.com> wrote:
>> > 
>> >> >>> On 13.07.16 at 14:53, <david.vrabel@citrix.com> wrote:
>> >> > On 13/07/16 13:20, Petr Tesarik wrote:
>> >> >> If a crash kernel is loaded, do not crash the running domain. This is
>> >> >> needed if the kernel is loaded with crash_kexec_post_notifiers, because
>> >> >> panic notifiers are run before __crash_kexec() in that case, and this
>> >> >> Xen hook prevents its being called later.
>> >> > 
>> >> > Prioritising the in-kernel kexec image over the hypervisor one seems
>> >> > sensible behaviour to me.
>> >> 
>> >> For HVM guests certainly; does loading of an in-kernel crash kernel
>> >> properly fail for PV guests (and namely PV Dom0), or does such a
>> >> setup work nowadays?
>> > 
>> > This is a good question, but I don't think it is relevant to this
>> > patch. It does not change anything unless the kernel is booted with
>> > crash_kexec_post_notifiers.
>> > 
>> > I fully understand that Dom0 kernels want to load the panic kernel in
>> > the hypervisor and crash the complete machine, rather than just Dom0,
>> > but if you want that behaviour, simply pass the "crashkernel="
>> > parameter only to the Xen hypervisor and not to the Dom0 kernel.
>> > 
>> > Did I miss something?
>> 
>> For one there are still many people who, for varying reasons, add
>> "crashkernel=" also to Dom0's command line.
> 
> Is there a valid use case for it?

No, but people doing so shouldn't end up being in more trouble than
they already are with their crashed guest. I.e. ...

> FWIW the legacy Xen implementation (as found in SLES) simply ignores
> the 'crashkernel=' kernel parameter. The code is not even compiled in.

... along those lines the pointlessly specified option should have
no effect.

>> And then trying to invoke a locally loaded crash kernel which won't
>> work is bad
> 
> Without actually knowing whether a PV kernel can kexec another PV
> kernel, this discussion is somewhat moot...
> 
> But let me repeat: if PV kexec works, then it has always had priority
> over the hypercall. If it doesn't work, then it has always been broken.
> For the latter case, I agree that the kernel should not even allow to
> load the kexec image, but that's unrelated to my patch.

It's not, afaict: without your patch, the hypercall to report the guest
crashed would be made unconditionally, without even an attempt to
load that secondary kernel.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Konrad Rzeszutek Wilk]

On Mon, Aug 01, 2016 at 04:15:10PM +0200, Petr Tesarik wrote:
> On Mon, 1 Aug 2016 15:47:58 +0200
> "Jan Beulich" <JBeulich@suse.com> wrote:
> 
> > (re-adding xen-devel)
> > 
> > >>> On 01.08.16 at 15:02, <PTesarik@suse.com> wrote:
> > > On Mon, 1 Aug 2016 13:55:01 +0200
> > > "Jan Beulich" <JBeulich@suse.com> wrote:
> > > 
> > >> >>> On 13.07.16 at 14:53, <david.vrabel@citrix.com> wrote:
> > >> > On 13/07/16 13:20, Petr Tesarik wrote:
> > >> >> If a crash kernel is loaded, do not crash the running domain. This is
> > >> >> needed if the kernel is loaded with crash_kexec_post_notifiers, because
> > >> >> panic notifiers are run before __crash_kexec() in that case, and this
> > >> >> Xen hook prevents its being called later.
> > >> > 
> > >> > Prioritising the in-kernel kexec image over the hypervisor one seems
> > >> > sensible behaviour to me.
> > >> 
> > >> For HVM guests certainly; does loading of an in-kernel crash kernel
> > >> properly fail for PV guests (and namely PV Dom0), or does such a
> > >> setup work nowadays?
> > > 
> > > This is a good question, but I don't think it is relevant to this
> > > patch. It does not change anything unless the kernel is booted with
> > > crash_kexec_post_notifiers.
> > > 
> > > I fully understand that Dom0 kernels want to load the panic kernel in
> > > the hypervisor and crash the complete machine, rather than just Dom0,
> > > but if you want that behaviour, simply pass the "crashkernel="
> > > parameter only to the Xen hypervisor and not to the Dom0 kernel.
> > > 
> > > Did I miss something?
> > 
> > For one there are still many people who, for varying reasons, add
> > "crashkernel=" also to Dom0's command line.
> 
> Is there a valid use case for it?
> 
> FWIW the legacy Xen implementation (as found in SLES) simply ignores
> the 'crashkernel=' kernel parameter. The code is not even compiled in.

I wonder if Xen should do that - as in 'fix' the bootparams to not have it.
Or perhaps Linux code during bootup can sanitze this..

> 
> > And then trying to invoke a locally loaded crash kernel which won't
> > work is bad
> 
> Without actually knowing whether a PV kernel can kexec another PV
> kernel, this discussion is somewhat moot...
> 
> But let me repeat: if PV kexec works, then it has always had priority
> over the hypercall. If it doesn't work, then it has always been broken.
> For the latter case, I agree that the kernel should not even allow to
> load the kexec image, but that's unrelated to my patch.
> 
> Has anyone here tried booting up a PV domain and performing kexec(2)?

Yes. Daniel (CC-ed) did it. He had it working but only for one CPU
and then Greg KH picked up the patchset .. and not sure what happend.

The underlaying issue was the PV guest could not re-initialize the grants,
events, etc - but now with Vitaly's 'reset' hypercall it would be possible.
Except the 'reset' hypercall is only for HVM guests.

> I can't test it with my SLES12 installation, because the kexec(8)
> user-space utility is patched in SLES to load the hypervisor kexec
> image with a hypercall if Xen is detected.

Also the upstream one would do this.
> 
> Petr T
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Petr Tesarik]

On Mon, 1 Aug 2016 16:22:18 +0200
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 01.08.16 at 16:15, <PTesarik@suse.com> wrote:
>[...]
> > But let me repeat: if PV kexec works, then it has always had priority
> > over the hypercall. If it doesn't work, then it has always been broken.
> > For the latter case, I agree that the kernel should not even allow to
> > load the kexec image, but that's unrelated to my patch.
> 
> It's not, afaict: without your patch, the hypercall to report the guest
> crashed would be made unconditionally, without even an attempt to
> load that secondary kernel.

That's a misunderstanding then. Without my patch and without
'crash_kexec_post_notifiers' on the kernel command line, panic() _will_
attempt kexec, see the beginning of panic():

        if (!crash_kexec_post_notifiers) {
                printk_nmi_flush_on_panic();
                __crash_kexec(NULL);
        }

That's the very first thing done, even before running the panic
handlers on panic_notifier_list.

Petr T

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/2] Allow kdump with crash_kexec_post_notifiers

[Daniel Kiper]

On Mon, Aug 01, 2016 at 10:25:22AM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 01, 2016 at 04:15:10PM +0200, Petr Tesarik wrote:
> > On Mon, 1 Aug 2016 15:47:58 +0200

[...]

> I wonder if Xen should do that - as in 'fix' the bootparams to not have it.
> Or perhaps Linux code during bootup can sanitze this..

It should but I am not sure it does. Or at least it should display warning
that dom0 crashkernel argument does not make sense and is not supported.

> > > And then trying to invoke a locally loaded crash kernel which won't
> > > work is bad
> >
> > Without actually knowing whether a PV kernel can kexec another PV
> > kernel, this discussion is somewhat moot...
> >
> > But let me repeat: if PV kexec works, then it has always had priority
> > over the hypercall. If it doesn't work, then it has always been broken.
> > For the latter case, I agree that the kernel should not even allow to
> > load the kexec image, but that's unrelated to my patch.
> >
> > Has anyone here tried booting up a PV domain and performing kexec(2)?
>
> Yes. Daniel (CC-ed) did it. He had it working but only for one CPU
> and then Greg KH picked up the patchset .. and not sure what happend.

Greg had it in https://git.kernel.org/ some time ago, however, it looks
that it vanished. Anyway, IIRC, the problem was that kdump must be started
with one processor and this is not easy due to various constraints. Later,
PV guest have to be restarted to regain full functionality.

> The underlaying issue was the PV guest could not re-initialize the grants,
> events, etc - but now with Vitaly's 'reset' hypercall it would be possible.
> Except the 'reset' hypercall is only for HVM guests.

I think that it is for PVHVM. Hence, making it work on PV should not be
very difficult. I hope...

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 0/2] Allow crash dumps with crash_kexec_post_notifiers

[Petr Tesarik]

Hello all,

this patch series makes it possible to save a kernel crash dump when the
kernel command line includes "crash_kexec_post_notifiers". There might
be other approaches, but mine has the advantage that no new sysctl is
required, and the behaviour is the same whether panic notifiers are run
or not: If you load a crash kernel with kexec, it will be used, otherwise
the hypervisor facility is used (using a hypercall).

Feedback welcome!

Petr T

---

Petr Tesarik (2):
      Add a kexec_crash_loaded() function
      Allow kdump with crash_kexec_post_notifiers


 arch/x86/xen/enlighten.c |    3 ++-
 include/linux/kexec.h    |    2 ++
 kernel/kexec_core.c      |    5 +++++
 kernel/ksysfs.c          |    2 +-
 4 files changed, 10 insertions(+), 2 deletions(-)

--
Signature

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel