All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: Hari Bathini <hbathini@linux.ibm.com>,
	lkml <linux-kernel@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Kexec-ml <kexec@lists.infradead.org>
Cc: Dave Young <dyoung@redhat.com>, Baoquan He <bhe@redhat.com>,
	Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
	Sourabh Jain <sourabhjain@linux.ibm.com>
Subject: Re: [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false
Date: Mon, 11 Sep 2023 17:13:08 +1000	[thread overview]
Message-ID: <8734zltc6z.fsf@mail.lhotse> (raw)
In-Reply-To: <20230905183604.568996-2-hbathini@linux.ibm.com>

Hari Bathini <hbathini@linux.ibm.com> writes:
> Currently, is_kdump_kernel() returns true when elfcorehdr_addr is set.
> While elfcorehdr_addr is set for kexec based kernel dump mechanism,
> alternate dump capturing methods like fadump [1] also set it to export
> the vmcore. Since, is_kdump_kernel() is used to restrict resources in
> crash dump capture kernel and such restrictions are not desirable for
> fadump, allow is_kdump_kernel() to be defined differently for fadump
> case. With that change, include is_fadump_active() check in functions
> is_vmcore_usable() & vmcore_unusable() to be able to export vmcore for
> fadump case too.
...
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index 0f3a656293b0..de8a9fabfb6f 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -50,6 +50,7 @@ void vmcore_cleanup(void);
>  #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
>  #endif
>  
> +#ifndef is_kdump_kernel
>  /*
>   * is_kdump_kernel() checks whether this kernel is booting after a panic of
>   * previous kernel or not. This is determined by checking if previous kernel
> @@ -64,6 +65,19 @@ static inline bool is_kdump_kernel(void)
>  {
>  	return elfcorehdr_addr != ELFCORE_ADDR_MAX;
>  }
> +#endif
> +
> +#ifndef is_fadump_active
> +/*
> + * If f/w assisted dump capturing mechanism (fadump), instead of kexec based
> + * dump capturing mechanism (kdump) is exporting the vmcore, then this function
> + * will be defined in arch specific code to return true, when appropriate.
> + */
> +static inline bool is_fadump_active(void)
> +{
> +	return false;
> +}
> +#endif
>  
>  /* is_vmcore_usable() checks if the kernel is booting after a panic and
>   * the vmcore region is usable.
> @@ -75,7 +89,8 @@ static inline bool is_kdump_kernel(void)
>  
>  static inline int is_vmcore_usable(void)
>  {
> -	return is_kdump_kernel() && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
> +	return (is_kdump_kernel() || is_fadump_active())
> +		&& elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
>  }
>  
>  /* vmcore_unusable() marks the vmcore as unusable,
> @@ -84,7 +99,7 @@ static inline int is_vmcore_usable(void)
>  
>  static inline void vmcore_unusable(void)
>  {
> -	if (is_kdump_kernel())
> +	if (is_kdump_kernel() || is_fadump_active())
>  		elfcorehdr_addr = ELFCORE_ADDR_ERR;
>  }

I think it would be cleaner to decouple is_vmcore_usable() and
vmcore_usable() from is_kdump_kernel().

ie, make them operate solely based on the value of elforehdr_addr:

static inline int is_vmcore_usable(void)
{
	elfcorehdr_addr != ELFCORE_ADDR_ERR && \
		elfcorehdr_addr != ELFCORE_ADDR_MAX;
}

static inline void vmcore_unusable(void)
{
	elfcorehdr_addr = ELFCORE_ADDR_ERR;
}


Then all we need on powerpc is a way to override is_kdump_kernel().

cheers

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

WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Hari Bathini <hbathini@linux.ibm.com>,
	lkml <linux-kernel@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Kexec-ml <kexec@lists.infradead.org>
Cc: Sourabh Jain <sourabhjain@linux.ibm.com>,
	Dave Young <dyoung@redhat.com>,
	Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
	Baoquan He <bhe@redhat.com>
Subject: Re: [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false
Date: Mon, 11 Sep 2023 17:13:08 +1000	[thread overview]
Message-ID: <8734zltc6z.fsf@mail.lhotse> (raw)
In-Reply-To: <20230905183604.568996-2-hbathini@linux.ibm.com>

Hari Bathini <hbathini@linux.ibm.com> writes:
> Currently, is_kdump_kernel() returns true when elfcorehdr_addr is set.
> While elfcorehdr_addr is set for kexec based kernel dump mechanism,
> alternate dump capturing methods like fadump [1] also set it to export
> the vmcore. Since, is_kdump_kernel() is used to restrict resources in
> crash dump capture kernel and such restrictions are not desirable for
> fadump, allow is_kdump_kernel() to be defined differently for fadump
> case. With that change, include is_fadump_active() check in functions
> is_vmcore_usable() & vmcore_unusable() to be able to export vmcore for
> fadump case too.
...
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index 0f3a656293b0..de8a9fabfb6f 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -50,6 +50,7 @@ void vmcore_cleanup(void);
>  #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
>  #endif
>  
> +#ifndef is_kdump_kernel
>  /*
>   * is_kdump_kernel() checks whether this kernel is booting after a panic of
>   * previous kernel or not. This is determined by checking if previous kernel
> @@ -64,6 +65,19 @@ static inline bool is_kdump_kernel(void)
>  {
>  	return elfcorehdr_addr != ELFCORE_ADDR_MAX;
>  }
> +#endif
> +
> +#ifndef is_fadump_active
> +/*
> + * If f/w assisted dump capturing mechanism (fadump), instead of kexec based
> + * dump capturing mechanism (kdump) is exporting the vmcore, then this function
> + * will be defined in arch specific code to return true, when appropriate.
> + */
> +static inline bool is_fadump_active(void)
> +{
> +	return false;
> +}
> +#endif
>  
>  /* is_vmcore_usable() checks if the kernel is booting after a panic and
>   * the vmcore region is usable.
> @@ -75,7 +89,8 @@ static inline bool is_kdump_kernel(void)
>  
>  static inline int is_vmcore_usable(void)
>  {
> -	return is_kdump_kernel() && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
> +	return (is_kdump_kernel() || is_fadump_active())
> +		&& elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
>  }
>  
>  /* vmcore_unusable() marks the vmcore as unusable,
> @@ -84,7 +99,7 @@ static inline int is_vmcore_usable(void)
>  
>  static inline void vmcore_unusable(void)
>  {
> -	if (is_kdump_kernel())
> +	if (is_kdump_kernel() || is_fadump_active())
>  		elfcorehdr_addr = ELFCORE_ADDR_ERR;
>  }

I think it would be cleaner to decouple is_vmcore_usable() and
vmcore_usable() from is_kdump_kernel().

ie, make them operate solely based on the value of elforehdr_addr:

static inline int is_vmcore_usable(void)
{
	elfcorehdr_addr != ELFCORE_ADDR_ERR && \
		elfcorehdr_addr != ELFCORE_ADDR_MAX;
}

static inline void vmcore_unusable(void)
{
	elfcorehdr_addr = ELFCORE_ADDR_ERR;
}


Then all we need on powerpc is a way to override is_kdump_kernel().

cheers

WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Hari Bathini <hbathini@linux.ibm.com>,
	lkml <linux-kernel@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Kexec-ml <kexec@lists.infradead.org>
Cc: Dave Young <dyoung@redhat.com>, Baoquan He <bhe@redhat.com>,
	Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
	Sourabh Jain <sourabhjain@linux.ibm.com>
Subject: Re: [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false
Date: Mon, 11 Sep 2023 17:13:08 +1000	[thread overview]
Message-ID: <8734zltc6z.fsf@mail.lhotse> (raw)
In-Reply-To: <20230905183604.568996-2-hbathini@linux.ibm.com>

Hari Bathini <hbathini@linux.ibm.com> writes:
> Currently, is_kdump_kernel() returns true when elfcorehdr_addr is set.
> While elfcorehdr_addr is set for kexec based kernel dump mechanism,
> alternate dump capturing methods like fadump [1] also set it to export
> the vmcore. Since, is_kdump_kernel() is used to restrict resources in
> crash dump capture kernel and such restrictions are not desirable for
> fadump, allow is_kdump_kernel() to be defined differently for fadump
> case. With that change, include is_fadump_active() check in functions
> is_vmcore_usable() & vmcore_unusable() to be able to export vmcore for
> fadump case too.
...
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index 0f3a656293b0..de8a9fabfb6f 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -50,6 +50,7 @@ void vmcore_cleanup(void);
>  #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
>  #endif
>  
> +#ifndef is_kdump_kernel
>  /*
>   * is_kdump_kernel() checks whether this kernel is booting after a panic of
>   * previous kernel or not. This is determined by checking if previous kernel
> @@ -64,6 +65,19 @@ static inline bool is_kdump_kernel(void)
>  {
>  	return elfcorehdr_addr != ELFCORE_ADDR_MAX;
>  }
> +#endif
> +
> +#ifndef is_fadump_active
> +/*
> + * If f/w assisted dump capturing mechanism (fadump), instead of kexec based
> + * dump capturing mechanism (kdump) is exporting the vmcore, then this function
> + * will be defined in arch specific code to return true, when appropriate.
> + */
> +static inline bool is_fadump_active(void)
> +{
> +	return false;
> +}
> +#endif
>  
>  /* is_vmcore_usable() checks if the kernel is booting after a panic and
>   * the vmcore region is usable.
> @@ -75,7 +89,8 @@ static inline bool is_kdump_kernel(void)
>  
>  static inline int is_vmcore_usable(void)
>  {
> -	return is_kdump_kernel() && elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
> +	return (is_kdump_kernel() || is_fadump_active())
> +		&& elfcorehdr_addr != ELFCORE_ADDR_ERR ? 1 : 0;
>  }
>  
>  /* vmcore_unusable() marks the vmcore as unusable,
> @@ -84,7 +99,7 @@ static inline int is_vmcore_usable(void)
>  
>  static inline void vmcore_unusable(void)
>  {
> -	if (is_kdump_kernel())
> +	if (is_kdump_kernel() || is_fadump_active())
>  		elfcorehdr_addr = ELFCORE_ADDR_ERR;
>  }

I think it would be cleaner to decouple is_vmcore_usable() and
vmcore_usable() from is_kdump_kernel().

ie, make them operate solely based on the value of elforehdr_addr:

static inline int is_vmcore_usable(void)
{
	elfcorehdr_addr != ELFCORE_ADDR_ERR && \
		elfcorehdr_addr != ELFCORE_ADDR_MAX;
}

static inline void vmcore_unusable(void)
{
	elfcorehdr_addr = ELFCORE_ADDR_ERR;
}


Then all we need on powerpc is a way to override is_kdump_kernel().

cheers

  reply	other threads:[~2023-09-11  7:13 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-05 18:36 [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Hari Bathini
2023-09-05 18:36 ` Hari Bathini
2023-09-05 18:36 ` Hari Bathini
2023-09-05 18:36 ` [PATCH v2 2/3] vmcore: allow fadump to export vmcore even if is_kdump_kernel() is false Hari Bathini
2023-09-05 18:36   ` Hari Bathini
2023-09-05 18:36   ` Hari Bathini
2023-09-11  7:13   ` Michael Ellerman [this message]
2023-09-11  7:13     ` Michael Ellerman
2023-09-11  7:13     ` Michael Ellerman
2023-09-11 10:31     ` Baoquan He
2023-09-11 10:31       ` Baoquan He
2023-09-11 10:31       ` Baoquan He
2023-09-12  8:31       ` Hari Bathini
2023-09-12  8:31         ` Hari Bathini
2023-09-12  8:31         ` Hari Bathini
2023-09-05 18:36 ` [PATCH v2 3/3] powerpc/fadump: make is_kdump_kernel() return false when fadump is active Hari Bathini
2023-09-05 18:36   ` Hari Bathini
2023-09-05 18:36   ` Hari Bathini
2023-09-07  5:42 ` [PATCH v2 1/3] powerpc/fadump: make is_fadump_active() visible for exporting vmcore Baoquan He
2023-09-07  5:42   ` Baoquan He
2023-09-07  5:42   ` Baoquan He
2023-09-08  5:49   ` Hari Bathini
2023-09-08  5:49     ` Hari Bathini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8734zltc6z.fsf@mail.lhotse \
    --to=mpe@ellerman.id.au \
    --cc=bhe@redhat.com \
    --cc=dyoung@redhat.com \
    --cc=hbathini@linux.ibm.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mahesh@linux.ibm.com \
    --cc=sourabhjain@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.