All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges
@ 2017-08-17 12:31 Hari Bathini
  2017-08-18  4:56 ` Alistair Popple
  2017-08-28  3:12 ` Pingfan Liu
  0 siblings, 2 replies; 5+ messages in thread
From: Hari Bathini @ 2017-08-17 12:31 UTC (permalink / raw)
  To: Simon Horman, Kexec-ml
  Cc: Ankit Kumar, Anshuman Khandual, Ananth N Mavinakayanahalli,
	Alistair Popple

Accelerator devices like GPU and FPGA cards contain onboard memory. This
onboard memory is represented as a memory only NUMA node, integrating it
with core memory subsystem. Since, the link through which these devices
are integrated to core memory goes down after a system crash and they are
meant for user workloads, avoid adding coherent device memory regions to
crash memory ranges. Without this change, makedumpfile tool tries to save
unaccessible coherent device memory regions, crashing the system.

Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
---
 kexec/arch/ppc64/crashdump-ppc64.c |   64 +++++++++++++++++++++++++++++++++++-
 kexec/arch/ppc64/kexec-ppc64.h     |    1 +
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/kexec/arch/ppc64/crashdump-ppc64.c b/kexec/arch/ppc64/crashdump-ppc64.c
index 13995bf..7ea3983 100644
--- a/kexec/arch/ppc64/crashdump-ppc64.c
+++ b/kexec/arch/ppc64/crashdump-ppc64.c
@@ -181,6 +181,53 @@ static int get_dyn_reconf_crash_memory_ranges(void)
 	return 0;
 }
 
+/*
+ * For a given memory node, check if it is mapped to system RAM or
+ * to onboard memory on accelerator device like GPU card or such.
+ */
+static int is_coherent_device_mem(const char *fname)
+{
+	char fpath[PATH_LEN];
+	char buf[32];
+	DIR *dmem;
+	FILE *file;
+	struct dirent *mentry;
+	int cnt, ret = 0;
+
+	strcpy(fpath, fname);
+	if ((dmem = opendir(fpath)) == NULL) {
+		perror(fpath);
+		return -1;
+	}
+
+	while ((mentry = readdir(dmem)) != NULL) {
+		if (strcmp(mentry->d_name, "compatible"))
+			continue;
+
+		strcat(fpath, "/compatible");
+		if ((file = fopen(fpath, "r")) == NULL) {
+			perror(fpath);
+			ret = -1;
+			break;
+		}
+		if ((cnt = fread(buf, 1, 32, file)) < 0) {
+			perror(fpath);
+			fclose(file);
+			ret = -1;
+			break;
+		}
+		if (!strncmp(buf, "ibm,coherent-device-memory", 26)) {
+			ret = 1;
+			break;
+		}
+		fclose(file);
+	}
+
+	closedir(dmem);
+	return ret;
+}
+
+
 /* Reads the appropriate file and retrieves the SYSTEM RAM regions for whom to
  * create Elf headers. Keeping it separate from get_memory_ranges() as
  * requirements are different in the case of normal kexec and crashdumps.
@@ -196,12 +243,12 @@ static int get_crash_memory_ranges(struct memory_range **range, int *ranges)
 {
 
 	char device_tree[256] = "/proc/device-tree/";
-	char fname[256];
+	char fname[PATH_LEN];
 	char buf[MAXBYTES];
 	DIR *dir, *dmem;
 	FILE *file;
 	struct dirent *dentry, *mentry;
-	int n, crash_rng_len = 0;
+	int n, ret, crash_rng_len = 0;
 	unsigned long long start, end;
 	int page_size;
 
@@ -240,6 +287,19 @@ static int get_crash_memory_ranges(struct memory_range **range, int *ranges)
 			continue;
 		strcpy(fname, device_tree);
 		strcat(fname, dentry->d_name);
+
+		ret = is_coherent_device_mem(fname);
+		if (ret == -1) {
+			closedir(dir);
+			goto err;
+		} else if (ret == 1) {
+			/*
+			 * Avoid adding this memory region as it is not
+			 * mapped to system RAM.
+			 */
+			continue;
+		}
+
 		if ((dmem = opendir(fname)) == NULL) {
 			perror(fname);
 			closedir(dir);
diff --git a/kexec/arch/ppc64/kexec-ppc64.h b/kexec/arch/ppc64/kexec-ppc64.h
index 633ae77..434b4bf 100644
--- a/kexec/arch/ppc64/kexec-ppc64.h
+++ b/kexec/arch/ppc64/kexec-ppc64.h
@@ -1,6 +1,7 @@
 #ifndef KEXEC_PPC64_H
 #define KEXEC_PPC64_H
 
+#define PATH_LEN 256
 #define MAXBYTES 128
 #define MAX_LINE 160
 #define CORE_TYPE_ELF32 1


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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges
  2017-08-17 12:31 [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges Hari Bathini
@ 2017-08-18  4:56 ` Alistair Popple
  2017-08-28  3:12 ` Pingfan Liu
  1 sibling, 0 replies; 5+ messages in thread
From: Alistair Popple @ 2017-08-18  4:56 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Ankit Kumar, Kexec-ml, Ananth N Mavinakayanahalli, Simon Horman,
	Anshuman Khandual

Hi,

I haven't reviewed the code in depth but skipping memory regions marked
ibm,coherent-device-memory is the right thing to do as the links to this memory
will usually go down prior during a kexec.

Also device-drivers typically put this memory into ZONE_MOVABLE which means
there will be no kernel allocations present in this memory.

Regards,

Alistair

On Thu, 17 Aug 2017 06:01:51 PM Hari Bathini wrote:
> Accelerator devices like GPU and FPGA cards contain onboard memory. This
> onboard memory is represented as a memory only NUMA node, integrating it
> with core memory subsystem. Since, the link through which these devices
> are integrated to core memory goes down after a system crash and they are
> meant for user workloads, avoid adding coherent device memory regions to
> crash memory ranges. Without this change, makedumpfile tool tries to save
> unaccessible coherent device memory regions, crashing the system.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> ---
>  kexec/arch/ppc64/crashdump-ppc64.c |   64 +++++++++++++++++++++++++++++++++++-
>  kexec/arch/ppc64/kexec-ppc64.h     |    1 +
>  2 files changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/kexec/arch/ppc64/crashdump-ppc64.c b/kexec/arch/ppc64/crashdump-ppc64.c
> index 13995bf..7ea3983 100644
> --- a/kexec/arch/ppc64/crashdump-ppc64.c
> +++ b/kexec/arch/ppc64/crashdump-ppc64.c
> @@ -181,6 +181,53 @@ static int get_dyn_reconf_crash_memory_ranges(void)
>  	return 0;
>  }
>  
> +/*
> + * For a given memory node, check if it is mapped to system RAM or
> + * to onboard memory on accelerator device like GPU card or such.
> + */
> +static int is_coherent_device_mem(const char *fname)
> +{
> +	char fpath[PATH_LEN];
> +	char buf[32];
> +	DIR *dmem;
> +	FILE *file;
> +	struct dirent *mentry;
> +	int cnt, ret = 0;
> +
> +	strcpy(fpath, fname);
> +	if ((dmem = opendir(fpath)) == NULL) {
> +		perror(fpath);
> +		return -1;
> +	}
> +
> +	while ((mentry = readdir(dmem)) != NULL) {
> +		if (strcmp(mentry->d_name, "compatible"))
> +			continue;
> +
> +		strcat(fpath, "/compatible");
> +		if ((file = fopen(fpath, "r")) == NULL) {
> +			perror(fpath);
> +			ret = -1;
> +			break;
> +		}
> +		if ((cnt = fread(buf, 1, 32, file)) < 0) {
> +			perror(fpath);
> +			fclose(file);
> +			ret = -1;
> +			break;
> +		}
> +		if (!strncmp(buf, "ibm,coherent-device-memory", 26)) {
> +			ret = 1;
> +			break;
> +		}
> +		fclose(file);
> +	}
> +
> +	closedir(dmem);
> +	return ret;
> +}
> +
> +
>  /* Reads the appropriate file and retrieves the SYSTEM RAM regions for whom to
>   * create Elf headers. Keeping it separate from get_memory_ranges() as
>   * requirements are different in the case of normal kexec and crashdumps.
> @@ -196,12 +243,12 @@ static int get_crash_memory_ranges(struct memory_range **range, int *ranges)
>  {
>  
>  	char device_tree[256] = "/proc/device-tree/";
> -	char fname[256];
> +	char fname[PATH_LEN];
>  	char buf[MAXBYTES];
>  	DIR *dir, *dmem;
>  	FILE *file;
>  	struct dirent *dentry, *mentry;
> -	int n, crash_rng_len = 0;
> +	int n, ret, crash_rng_len = 0;
>  	unsigned long long start, end;
>  	int page_size;
>  
> @@ -240,6 +287,19 @@ static int get_crash_memory_ranges(struct memory_range **range, int *ranges)
>  			continue;
>  		strcpy(fname, device_tree);
>  		strcat(fname, dentry->d_name);
> +
> +		ret = is_coherent_device_mem(fname);
> +		if (ret == -1) {
> +			closedir(dir);
> +			goto err;
> +		} else if (ret == 1) {
> +			/*
> +			 * Avoid adding this memory region as it is not
> +			 * mapped to system RAM.
> +			 */
> +			continue;
> +		}
> +
>  		if ((dmem = opendir(fname)) == NULL) {
>  			perror(fname);
>  			closedir(dir);
> diff --git a/kexec/arch/ppc64/kexec-ppc64.h b/kexec/arch/ppc64/kexec-ppc64.h
> index 633ae77..434b4bf 100644
> --- a/kexec/arch/ppc64/kexec-ppc64.h
> +++ b/kexec/arch/ppc64/kexec-ppc64.h
> @@ -1,6 +1,7 @@
>  #ifndef KEXEC_PPC64_H
>  #define KEXEC_PPC64_H
>  
> +#define PATH_LEN 256
>  #define MAXBYTES 128
>  #define MAX_LINE 160
>  #define CORE_TYPE_ELF32 1
> 


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges
  2017-08-17 12:31 [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges Hari Bathini
  2017-08-18  4:56 ` Alistair Popple
@ 2017-08-28  3:12 ` Pingfan Liu
  2017-08-28 13:37   ` Simon Horman
  1 sibling, 1 reply; 5+ messages in thread
From: Pingfan Liu @ 2017-08-28  3:12 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Alistair Popple, Ankit Kumar, Kexec-ml,
	Ananth N Mavinakayanahalli, Simon Horman, Anshuman Khandual





----- Original Message -----
> From: "Hari Bathini" <hbathini@linux.vnet.ibm.com>
> To: "Simon Horman" <horms@verge.net.au>, "Kexec-ml" <kexec@lists.infradead.org>
> Cc: "Ankit Kumar" <ankit@linux.vnet.ibm.com>, "Anshuman Khandual" <khandual@linux.vnet.ibm.com>, "Ananth N
> Mavinakayanahalli" <ananth@linux.vnet.ibm.com>, "Alistair Popple" <alistair@popple.id.au>
> Sent: Thursday, August 17, 2017 8:31:51 PM
> Subject: [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges
> 
> Accelerator devices like GPU and FPGA cards contain onboard memory. This
> onboard memory is represented as a memory only NUMA node, integrating it
> with core memory subsystem. Since, the link through which these devices
> are integrated to core memory goes down after a system crash and they are
> meant for user workloads, avoid adding coherent device memory regions to
> crash memory ranges. Without this change, makedumpfile tool tries to save
> unaccessible coherent device memory regions, crashing the system.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> ---
>  kexec/arch/ppc64/crashdump-ppc64.c |   64
>  +++++++++++++++++++++++++++++++++++-
>  kexec/arch/ppc64/kexec-ppc64.h     |    1 +
>  2 files changed, 63 insertions(+), 2 deletions(-)
> 
> diff --git a/kexec/arch/ppc64/crashdump-ppc64.c
> b/kexec/arch/ppc64/crashdump-ppc64.c
> index 13995bf..7ea3983 100644
> --- a/kexec/arch/ppc64/crashdump-ppc64.c
> +++ b/kexec/arch/ppc64/crashdump-ppc64.c
> @@ -181,6 +181,53 @@ static int get_dyn_reconf_crash_memory_ranges(void)
>  	return 0;
>  }
>  
> +/*
> + * For a given memory node, check if it is mapped to system RAM or
> + * to onboard memory on accelerator device like GPU card or such.
> + */
> +static int is_coherent_device_mem(const char *fname)
> +{
> +	char fpath[PATH_LEN];
> +	char buf[32];
> +	DIR *dmem;
> +	FILE *file;
> +	struct dirent *mentry;
> +	int cnt, ret = 0;
> +
> +	strcpy(fpath, fname);
> +	if ((dmem = opendir(fpath)) == NULL) {
> +		perror(fpath);
> +		return -1;
> +	}
> +
> +	while ((mentry = readdir(dmem)) != NULL) {
> +		if (strcmp(mentry->d_name, "compatible"))
> +			continue;
> +
> +		strcat(fpath, "/compatible");
> +		if ((file = fopen(fpath, "r")) == NULL) {
> +			perror(fpath);
> +			ret = -1;
> +			break;
> +		}
> +		if ((cnt = fread(buf, 1, 32, file)) < 0) {
> +			perror(fpath);
> +			fclose(file);
> +			ret = -1;
> +			break;
> +		}
> +		if (!strncmp(buf, "ibm,coherent-device-memory", 26)) {
> +			ret = 1;
> +			break;
> +		}
> +		fclose(file);
> +	}
> +
> +	closedir(dmem);
> +	return ret;
> +}
> +
> +
>  /* Reads the appropriate file and retrieves the SYSTEM RAM regions for whom
>  to
>   * create Elf headers. Keeping it separate from get_memory_ranges() as
>   * requirements are different in the case of normal kexec and crashdumps.
> @@ -196,12 +243,12 @@ static int get_crash_memory_ranges(struct memory_range
> **range, int *ranges)
>  {
>  
>  	char device_tree[256] = "/proc/device-tree/";
> -	char fname[256];
> +	char fname[PATH_LEN];
>  	char buf[MAXBYTES];
>  	DIR *dir, *dmem;
>  	FILE *file;
>  	struct dirent *dentry, *mentry;
> -	int n, crash_rng_len = 0;
> +	int n, ret, crash_rng_len = 0;
>  	unsigned long long start, end;
>  	int page_size;
>  
> @@ -240,6 +287,19 @@ static int get_crash_memory_ranges(struct memory_range
> **range, int *ranges)
>  			continue;
>  		strcpy(fname, device_tree);
>  		strcat(fname, dentry->d_name);
> +
> +		ret = is_coherent_device_mem(fname);
> +		if (ret == -1) {
> +			closedir(dir);
> +			goto err;
> +		} else if (ret == 1) {
> +			/*
> +			 * Avoid adding this memory region as it is not
> +			 * mapped to system RAM.
> +			 */
> +			continue;
> +		}
> +
>  		if ((dmem = opendir(fname)) == NULL) {
>  			perror(fname);
>  			closedir(dir);
> diff --git a/kexec/arch/ppc64/kexec-ppc64.h b/kexec/arch/ppc64/kexec-ppc64.h
> index 633ae77..434b4bf 100644
> --- a/kexec/arch/ppc64/kexec-ppc64.h
> +++ b/kexec/arch/ppc64/kexec-ppc64.h
> @@ -1,6 +1,7 @@
>  #ifndef KEXEC_PPC64_H
>  #define KEXEC_PPC64_H
>  
> +#define PATH_LEN 256
>  #define MAXBYTES 128
>  #define MAX_LINE 160
>  #define CORE_TYPE_ELF32 1
> 
Tested-by: Pingfan Liu <piliu@redhat.com>

> 
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges
  2017-08-28  3:12 ` Pingfan Liu
@ 2017-08-28 13:37   ` Simon Horman
  2017-08-29 17:47     ` Hari Bathini
  0 siblings, 1 reply; 5+ messages in thread
From: Simon Horman @ 2017-08-28 13:37 UTC (permalink / raw)
  To: Pingfan Liu
  Cc: Alistair Popple, Ankit Kumar, Kexec-ml,
	Ananth N Mavinakayanahalli, Hari Bathini, Anshuman Khandual

On Sun, Aug 27, 2017 at 11:12:37PM -0400, Pingfan Liu wrote:
> 
> 
> 
> 
> ----- Original Message -----
> > From: "Hari Bathini" <hbathini@linux.vnet.ibm.com>
> > To: "Simon Horman" <horms@verge.net.au>, "Kexec-ml" <kexec@lists.infradead.org>
> > Cc: "Ankit Kumar" <ankit@linux.vnet.ibm.com>, "Anshuman Khandual" <khandual@linux.vnet.ibm.com>, "Ananth N
> > Mavinakayanahalli" <ananth@linux.vnet.ibm.com>, "Alistair Popple" <alistair@popple.id.au>
> > Sent: Thursday, August 17, 2017 8:31:51 PM
> > Subject: [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges
> > 
> > Accelerator devices like GPU and FPGA cards contain onboard memory. This
> > onboard memory is represented as a memory only NUMA node, integrating it
> > with core memory subsystem. Since, the link through which these devices
> > are integrated to core memory goes down after a system crash and they are
> > meant for user workloads, avoid adding coherent device memory regions to
> > crash memory ranges. Without this change, makedumpfile tool tries to save
> > unaccessible coherent device memory regions, crashing the system.
> > 
> > Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> > ---
> >  kexec/arch/ppc64/crashdump-ppc64.c |   64
> >  +++++++++++++++++++++++++++++++++++-
> >  kexec/arch/ppc64/kexec-ppc64.h     |    1 +
> >  2 files changed, 63 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kexec/arch/ppc64/crashdump-ppc64.c
> > b/kexec/arch/ppc64/crashdump-ppc64.c
> > index 13995bf..7ea3983 100644
> > --- a/kexec/arch/ppc64/crashdump-ppc64.c
> > +++ b/kexec/arch/ppc64/crashdump-ppc64.c
> > @@ -181,6 +181,53 @@ static int get_dyn_reconf_crash_memory_ranges(void)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * For a given memory node, check if it is mapped to system RAM or
> > + * to onboard memory on accelerator device like GPU card or such.
> > + */
> > +static int is_coherent_device_mem(const char *fname)
> > +{
> > +	char fpath[PATH_LEN];
> > +	char buf[32];
> > +	DIR *dmem;
> > +	FILE *file;
> > +	struct dirent *mentry;
> > +	int cnt, ret = 0;
> > +
> > +	strcpy(fpath, fname);
> > +	if ((dmem = opendir(fpath)) == NULL) {
> > +		perror(fpath);
> > +		return -1;
> > +	}
> > +
> > +	while ((mentry = readdir(dmem)) != NULL) {
> > +		if (strcmp(mentry->d_name, "compatible"))
> > +			continue;
> > +
> > +		strcat(fpath, "/compatible");
> > +		if ((file = fopen(fpath, "r")) == NULL) {
> > +			perror(fpath);
> > +			ret = -1;
> > +			break;
> > +		}
> > +		if ((cnt = fread(buf, 1, 32, file)) < 0) {
> > +			perror(fpath);
> > +			fclose(file);
> > +			ret = -1;
> > +			break;
> > +		}
> > +		if (!strncmp(buf, "ibm,coherent-device-memory", 26)) {
> > +			ret = 1;
> > +			break;

This seems to leak file.

> > +		}
> > +		fclose(file);
> > +	}
> > +
> > +	closedir(dmem);
> > +	return ret;
> > +}
> > +
> > +
> >  /* Reads the appropriate file and retrieves the SYSTEM RAM regions for whom
> >  to
> >   * create Elf headers. Keeping it separate from get_memory_ranges() as
> >   * requirements are different in the case of normal kexec and crashdumps.
> > @@ -196,12 +243,12 @@ static int get_crash_memory_ranges(struct memory_range
> > **range, int *ranges)
> >  {
> >  
> >  	char device_tree[256] = "/proc/device-tree/";
> > -	char fname[256];
> > +	char fname[PATH_LEN];
> >  	char buf[MAXBYTES];
> >  	DIR *dir, *dmem;
> >  	FILE *file;
> >  	struct dirent *dentry, *mentry;
> > -	int n, crash_rng_len = 0;
> > +	int n, ret, crash_rng_len = 0;
> >  	unsigned long long start, end;
> >  	int page_size;
> >  
> > @@ -240,6 +287,19 @@ static int get_crash_memory_ranges(struct memory_range
> > **range, int *ranges)
> >  			continue;
> >  		strcpy(fname, device_tree);
> >  		strcat(fname, dentry->d_name);
> > +
> > +		ret = is_coherent_device_mem(fname);
> > +		if (ret == -1) {
> > +			closedir(dir);
> > +			goto err;
> > +		} else if (ret == 1) {
> > +			/*
> > +			 * Avoid adding this memory region as it is not
> > +			 * mapped to system RAM.
> > +			 */
> > +			continue;
> > +		}
> > +
> >  		if ((dmem = opendir(fname)) == NULL) {
> >  			perror(fname);
> >  			closedir(dir);
> > diff --git a/kexec/arch/ppc64/kexec-ppc64.h b/kexec/arch/ppc64/kexec-ppc64.h
> > index 633ae77..434b4bf 100644
> > --- a/kexec/arch/ppc64/kexec-ppc64.h
> > +++ b/kexec/arch/ppc64/kexec-ppc64.h
> > @@ -1,6 +1,7 @@
> >  #ifndef KEXEC_PPC64_H
> >  #define KEXEC_PPC64_H
> >  
> > +#define PATH_LEN 256
> >  #define MAXBYTES 128
> >  #define MAX_LINE 160
> >  #define CORE_TYPE_ELF32 1
> > 
> Tested-by: Pingfan Liu <piliu@redhat.com>

The above not withstanding I have applied this patch with Pingfan's tag.
Please post a follow-up patch as appropriate.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges
  2017-08-28 13:37   ` Simon Horman
@ 2017-08-29 17:47     ` Hari Bathini
  0 siblings, 0 replies; 5+ messages in thread
From: Hari Bathini @ 2017-08-29 17:47 UTC (permalink / raw)
  To: Simon Horman, Pingfan Liu
  Cc: Alistair Popple, Ankit Kumar, Kexec-ml,
	Ananth N Mavinakayanahalli, Anshuman Khandual



On Monday 28 August 2017 07:07 PM, Simon Horman wrote:
> On Sun, Aug 27, 2017 at 11:12:37PM -0400, Pingfan Liu wrote:
>>
>>
>>
>> ----- Original Message -----
>>> From: "Hari Bathini" <hbathini@linux.vnet.ibm.com>
>>> To: "Simon Horman" <horms@verge.net.au>, "Kexec-ml" <kexec@lists.infradead.org>
>>> Cc: "Ankit Kumar" <ankit@linux.vnet.ibm.com>, "Anshuman Khandual" <khandual@linux.vnet.ibm.com>, "Ananth N
>>> Mavinakayanahalli" <ananth@linux.vnet.ibm.com>, "Alistair Popple" <alistair@popple.id.au>
>>> Sent: Thursday, August 17, 2017 8:31:51 PM
>>> Subject: [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges
>>>
>>> Accelerator devices like GPU and FPGA cards contain onboard memory. This
>>> onboard memory is represented as a memory only NUMA node, integrating it
>>> with core memory subsystem. Since, the link through which these devices
>>> are integrated to core memory goes down after a system crash and they are
>>> meant for user workloads, avoid adding coherent device memory regions to
>>> crash memory ranges. Without this change, makedumpfile tool tries to save
>>> unaccessible coherent device memory regions, crashing the system.
>>>
>>> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
>>> ---
>>>   kexec/arch/ppc64/crashdump-ppc64.c |   64
>>>   +++++++++++++++++++++++++++++++++++-
>>>   kexec/arch/ppc64/kexec-ppc64.h     |    1 +
>>>   2 files changed, 63 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kexec/arch/ppc64/crashdump-ppc64.c
>>> b/kexec/arch/ppc64/crashdump-ppc64.c
>>> index 13995bf..7ea3983 100644
>>> --- a/kexec/arch/ppc64/crashdump-ppc64.c
>>> +++ b/kexec/arch/ppc64/crashdump-ppc64.c
>>> @@ -181,6 +181,53 @@ static int get_dyn_reconf_crash_memory_ranges(void)
>>>   	return 0;
>>>   }
>>>   
>>> +/*
>>> + * For a given memory node, check if it is mapped to system RAM or
>>> + * to onboard memory on accelerator device like GPU card or such.
>>> + */
>>> +static int is_coherent_device_mem(const char *fname)
>>> +{
>>> +	char fpath[PATH_LEN];
>>> +	char buf[32];
>>> +	DIR *dmem;
>>> +	FILE *file;
>>> +	struct dirent *mentry;
>>> +	int cnt, ret = 0;
>>> +
>>> +	strcpy(fpath, fname);
>>> +	if ((dmem = opendir(fpath)) == NULL) {
>>> +		perror(fpath);
>>> +		return -1;
>>> +	}
>>> +
>>> +	while ((mentry = readdir(dmem)) != NULL) {
>>> +		if (strcmp(mentry->d_name, "compatible"))
>>> +			continue;
>>> +
>>> +		strcat(fpath, "/compatible");
>>> +		if ((file = fopen(fpath, "r")) == NULL) {
>>> +			perror(fpath);
>>> +			ret = -1;
>>> +			break;
>>> +		}
>>> +		if ((cnt = fread(buf, 1, 32, file)) < 0) {
>>> +			perror(fpath);
>>> +			fclose(file);
>>> +			ret = -1;
>>> +			break;
>>> +		}
>>> +		if (!strncmp(buf, "ibm,coherent-device-memory", 26)) {
>>> +			ret = 1;
>>> +			break;
> This seems to leak file.
>
>>> +		}
>>> +		fclose(file);
>>> +	}
>>> +
>>> +	closedir(dmem);
>>> +	return ret;
>>> +}
>>> +
>>> +
>>>   /* Reads the appropriate file and retrieves the SYSTEM RAM regions for whom
>>>   to
>>>    * create Elf headers. Keeping it separate from get_memory_ranges() as
>>>    * requirements are different in the case of normal kexec and crashdumps.
>>> @@ -196,12 +243,12 @@ static int get_crash_memory_ranges(struct memory_range
>>> **range, int *ranges)
>>>   {
>>>   
>>>   	char device_tree[256] = "/proc/device-tree/";
>>> -	char fname[256];
>>> +	char fname[PATH_LEN];
>>>   	char buf[MAXBYTES];
>>>   	DIR *dir, *dmem;
>>>   	FILE *file;
>>>   	struct dirent *dentry, *mentry;
>>> -	int n, crash_rng_len = 0;
>>> +	int n, ret, crash_rng_len = 0;
>>>   	unsigned long long start, end;
>>>   	int page_size;
>>>   
>>> @@ -240,6 +287,19 @@ static int get_crash_memory_ranges(struct memory_range
>>> **range, int *ranges)
>>>   			continue;
>>>   		strcpy(fname, device_tree);
>>>   		strcat(fname, dentry->d_name);
>>> +
>>> +		ret = is_coherent_device_mem(fname);
>>> +		if (ret == -1) {
>>> +			closedir(dir);
>>> +			goto err;
>>> +		} else if (ret == 1) {
>>> +			/*
>>> +			 * Avoid adding this memory region as it is not
>>> +			 * mapped to system RAM.
>>> +			 */
>>> +			continue;
>>> +		}
>>> +
>>>   		if ((dmem = opendir(fname)) == NULL) {
>>>   			perror(fname);
>>>   			closedir(dir);
>>> diff --git a/kexec/arch/ppc64/kexec-ppc64.h b/kexec/arch/ppc64/kexec-ppc64.h
>>> index 633ae77..434b4bf 100644
>>> --- a/kexec/arch/ppc64/kexec-ppc64.h
>>> +++ b/kexec/arch/ppc64/kexec-ppc64.h
>>> @@ -1,6 +1,7 @@
>>>   #ifndef KEXEC_PPC64_H
>>>   #define KEXEC_PPC64_H
>>>   
>>> +#define PATH_LEN 256
>>>   #define MAXBYTES 128
>>>   #define MAX_LINE 160
>>>   #define CORE_TYPE_ELF32 1
>>>
>> Tested-by: Pingfan Liu <piliu@redhat.com>
> The above not withstanding I have applied this patch with Pingfan's tag.
> Please post a follow-up patch as appropriate.

Thanks, Simon.
Posted the follow-up patch at 
http://lists.infradead.org/pipermail/kexec/2017-August/019439.html

- Hari


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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-08-29 17:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-17 12:31 [PATCH] kexec-tools: ppc64: avoid adding coherent memory regions to crash memory ranges Hari Bathini
2017-08-18  4:56 ` Alistair Popple
2017-08-28  3:12 ` Pingfan Liu
2017-08-28 13:37   ` Simon Horman
2017-08-29 17:47     ` Hari Bathini

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.