Horms wrote:
Hi Sachin,
I took a quick look over this. And it seems to be fine.
I've put some minor comments inline.
*/
-static struct memory_range crash_memory_range[CRASH_MAX_MEMORY_RANGES];
+static struct memory_range *crash_memory_range;
I think that crash_memory_range needs to be initialised to NULL here.
Otherwise the err check in get_crash_memory_ranges() may be bogus.
Hmm .. since all these variables are static globals they will be
initialized to NULL. But i think for clarity sake we should
explicitly initialize them to NULL. Have changed it.
+ fprintf(stderr, "Memory allocation for crash memory range
failed\n");
Can you make this <= 80col wide? I think it is just a little over.
Done.
I think that memory_range, base_memory_range and exclude_range all
need to be initialised to NULL here. Otherwise the checks
in cleanup_memory_ranges() may be bogus.
Hmm ok done.
+
+static void cleanup_memory_ranges()
+{
+ if (memory_range)
+ free(memory_range);
+ if (base_memory_range)
+ free(base_memory_range);
+ if (exclude_range)
+ free(exclude_range);
+ if (usablemem_rgns.ranges)
+ free(usablemem_rgns.ranges);
+}
Should this function set any freed pointers to NULL ?
I guess it should not be necessary to set them to NULL. This is an exit
path and the program will end.
+
+/*
+ * Allocate memory for various data structures used to hold
+ * values of different memory ranges
+ */
+static int alloc_memory_ranges()
+{
+ memory_range = (struct memory_range *) malloc(
+ (sizeof(struct memory_range) * max_memory_ranges));
+ base_memory_range = (struct memory_range *) malloc(
+ (sizeof(struct memory_range) * max_memory_ranges));
+ exclude_range = (struct exclude_range *) malloc(
+ (sizeof(struct exclude_range) * max_memory_ranges));
+ usablemem_rgns.ranges = (struct exclude_range *) malloc(
+ (sizeof(struct exclude_range) * max_memory_ranges));
+ if ((memory_range == NULL) ||
+ (base_memory_range == NULL) ||
+ (exclude_range == NULL) ||
+ (usablemem_rgns.ranges == NULL)) {
I personally prefer the following
if (!memory_range || !base_memory_range || !exclude_range ||
!usablemem_rgns.ranges)
Actually, I really prefer doing the error check after each malloc(),
but thats just me.
Done.
+ fprintf(stderr, "Memory allocation for memory range structure
failed\n");
Can you make this <= 80col wide? I think it is just a little over.
Done
+extern mem_rgns_t usablemem_rgns;
usablemem_rgns.ranges needs to be initialised to NULL somehow.
Perhaps you could just change the declaration (wherever that is)
to mem_rgns_t usablemem_rgns = { 0, NULL }; I think that would work.
Done.
Here is the modified patch. Thanks for the review.
I have tested this on few PPC64 boxes and haven't found any problem
with it.
* kexec tools for ppc64 uses static arrays [of length MAX_MEMORY_RANGES] to
* store data related to memory regions. This used to eat up lot's of memory.
* There were some instances where more memory regions existed, which made
* kexec tools unusable on those machines. The following patch gets rid of
* MAX_MEMORY_RANGES macro and uses dynamic memory allocation for the
* above mentioned structures.
Signed-off-by : Sachin Sant <[EMAIL PROTECTED]>
---
* kexec tools for ppc64 uses static arrays [of length MAX_MEMORY_RANGES] to
* store data related to memory regions. This used to eat up lot's of memory.
* There were some instances where more memory regions existed, which made
* kexec tools unusable on those machines. The following patch gets rid of
* MAX_MEMORY_RANGES macro and uses dynamic memory allocation for the
* above mentioned structures.
Signed-off-by : Sachin Sant <[EMAIL PROTECTED]>
---
diff -Naurp hormsgittree/kexec/arch/ppc64/crashdump-ppc64.c hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/crashdump-ppc64.c
--- hormsgittree/kexec/arch/ppc64/crashdump-ppc64.c 2006-12-05 17:08:18.000000000 +0530
+++ hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/crashdump-ppc64.c 2006-12-14 14:39:51.000000000 +0530
@@ -62,13 +62,16 @@ extern struct arch_options_t arch_option
/* Stores a sorted list of RAM memory ranges for which to create elf headers.
* A separate program header is created for backup region
*/
-static struct memory_range crash_memory_range[CRASH_MAX_MEMORY_RANGES];
+static struct memory_range *crash_memory_range = NULL;
+
+/* Define a variable to replace the CRASH_MAX_MEMORY_RANGES macro */
+static int crash_max_memory_ranges;
/*
* Used to save various memory ranges/regions needed for the captured
* kernel to boot. (lime memmap= option in other archs)
*/
-mem_rgns_t usablemem_rgns = {0, };
+mem_rgns_t usablemem_rgns = {0, NULL};
/*
* To store the memory size of the first kernel and this value will be
@@ -105,6 +108,15 @@ static int get_crash_memory_ranges(struc
int i, n;
unsigned long long start, end, cstart, cend;
+ crash_max_memory_ranges = max_memory_ranges + 6;
+
+ crash_memory_range = (struct memory_range *) malloc(
+ (sizeof(struct memory_range) * (crash_max_memory_ranges)));
+ if (!crash_memory_range) {
+ fprintf(stderr, "Allocation for crash memory range failed\n");
+ return -1;
+ }
+
/* create a separate program header for the backup region */
crash_memory_range[0].start = BACKUP_SRC_START;
crash_memory_range[0].end = BACKUP_SRC_END;
@@ -113,7 +125,7 @@ static int get_crash_memory_ranges(struc
if ((dir = opendir(device_tree)) == NULL) {
perror(device_tree);
- return -1;
+ goto err;
}
while ((dentry = readdir(dir)) != NULL) {
if (strncmp(dentry->d_name, "memory@", 7))
@@ -123,7 +135,7 @@ static int get_crash_memory_ranges(struc
if ((dmem = opendir(fname)) == NULL) {
perror(fname);
closedir(dir);
- return -1;
+ goto err;
}
while ((mentry = readdir(dmem)) != NULL) {
if (strcmp(mentry->d_name, "reg"))
@@ -133,21 +145,21 @@ static int get_crash_memory_ranges(struc
perror(fname);
closedir(dmem);
closedir(dir);
- return -1;
+ goto err;
}
if ((n = fread(buf, 1, MAXBYTES, file)) < 0) {
perror(fname);
fclose(file);
closedir(dmem);
closedir(dir);
- return -1;
+ goto err;
}
- if (memory_ranges >= MAX_MEMORY_RANGES) {
+ if (memory_ranges >= max_memory_ranges) {
/* No space to insert another element. */
fprintf(stderr,
"Error: Number of crash memory ranges"
" excedeed the max limit\n");
- return -1;
+ goto err;
}
start = ((unsigned long long *)buf)[0];
@@ -228,6 +240,11 @@ static int get_crash_memory_ranges(struc
}
#endif
return 0;
+
+err:
+ if (crash_memory_range)
+ free(crash_memory_range);
+ return -1;
}
/* Converts unsigned long to ascii string. */
diff -Naurp hormsgittree/kexec/arch/ppc64/crashdump-ppc64.h hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/crashdump-ppc64.h
--- hormsgittree/kexec/arch/ppc64/crashdump-ppc64.h 2006-12-05 17:08:18.000000000 +0530
+++ hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/crashdump-ppc64.h 2006-12-13 11:26:40.000000000 +0530
@@ -13,8 +13,6 @@ void add_usable_mem_rgns(unsigned long l
#define __pa(x) ((unsigned long)(x)-PAGE_OFFSET)
#define MAXMEM (-KERNELBASE-VMALLOCBASE)
-#define CRASH_MAX_MEMORY_RANGES (MAX_MEMORY_RANGES + 6)
-
#define COMMAND_LINE_SIZE 512 /* from kernel */
/* Backup Region, First 64K of System RAM. */
#define BACKUP_SRC_START 0x0000
diff -Naurp hormsgittree/kexec/arch/ppc64/fs2dt.c hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/fs2dt.c
--- hormsgittree/kexec/arch/ppc64/fs2dt.c 2006-12-13 11:23:25.000000000 +0530
+++ hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/fs2dt.c 2006-12-13 17:29:26.000000000 +0530
@@ -36,6 +36,7 @@
#define NAMESPACE 16384 /* max bytes for property names */
#define TREEWORDS 65536 /* max 32 bit words for property values */
#define MEMRESERVE 256 /* max number of reserved memory blocks */
+#define MAX_MEMORY_RANGES 1024
static char pathname[MAXPATH], *pathstart;
static char propnames[NAMESPACE] = { 0 };
diff -Naurp hormsgittree/kexec/arch/ppc64/kexec-ppc64.c hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/kexec-ppc64.c
--- hormsgittree/kexec/arch/ppc64/kexec-ppc64.c 2006-12-13 11:25:48.000000000 +0530
+++ hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/kexec-ppc64.c 2006-12-14 14:48:45.000000000 +0530
@@ -20,6 +20,7 @@
#include <stddef.h>
#include <stdio.h>
+#include <stdlib.h>
#include <errno.h>
#include <stdint.h>
#include <string.h>
@@ -34,17 +35,92 @@
#include "crashdump-ppc64.h"
#include <arch/options.h>
-static struct exclude_range exclude_range[MAX_MEMORY_RANGES];
+static struct exclude_range *exclude_range = NULL;
+static struct memory_range *memory_range = NULL;
+static struct memory_range *base_memory_range = NULL;
static unsigned long long rmo_top;
-static struct memory_range memory_range[MAX_MEMORY_RANGES];
-static struct memory_range base_memory_range[MAX_MEMORY_RANGES];
unsigned long long memory_max = 0;
static int nr_memory_ranges, nr_exclude_ranges;
unsigned long long crash_base, crash_size;
unsigned int rtas_base, rtas_size;
+int max_memory_ranges;
static int sort_base_ranges();
+
+static void cleanup_memory_ranges()
+{
+ if (memory_range)
+ free(memory_range);
+ if (base_memory_range)
+ free(base_memory_range);
+ if (exclude_range)
+ free(exclude_range);
+ if (usablemem_rgns.ranges)
+ free(usablemem_rgns.ranges);
+}
+
+/*
+ * Allocate memory for various data structures used to hold
+ * values of different memory ranges
+ */
+static int alloc_memory_ranges()
+{
+ memory_range = (struct memory_range *) malloc(
+ (sizeof(struct memory_range) * max_memory_ranges));
+ if (!memory_range)
+ goto err1;
+
+ base_memory_range = (struct memory_range *) malloc(
+ (sizeof(struct memory_range) * max_memory_ranges));
+ if (!base_memory_range)
+ goto err1;
+
+ exclude_range = (struct exclude_range *) malloc(
+ (sizeof(struct exclude_range) * max_memory_ranges));
+ if (!exclude_range)
+ goto err1;
+
+ usablemem_rgns.ranges = (struct exclude_range *) malloc(
+ (sizeof(struct exclude_range) * max_memory_ranges));
+ if (!(usablemem_rgns.ranges))
+ goto err1;
+
+ return 0;
+
+err1:
+ fprintf(stderr, "memory range structure allocation failure\n");
+ cleanup_memory_ranges();
+ return -1;
+
+}
+
+/*
+ * Count the memory@ nodes under /proc/device-tree and populate the
+ * max_memory_ranges variable. This variable replaces MAX_MEMORY_RANGES
+ * macro used earlier.
+ */
+static int count_memory_ranges()
+{
+ char device_tree[256] = "/proc/device-tree/";
+ struct dirent *dentry;
+ DIR *dir;
+
+ if ((dir = opendir(device_tree)) == NULL) {
+ perror(device_tree);
+ return -1;
+ }
+
+ while ((dentry = readdir(dir)) != NULL) {
+ if (strncmp(dentry->d_name, "memory@", 7))
+ continue;
+ max_memory_ranges++;
+ }
+ closedir(dir);
+
+ return 0;
+}
+
/* Get base memory ranges */
static int get_base_ranges()
{
@@ -88,7 +164,7 @@ static int get_base_ranges()
closedir(dir);
return -1;
}
- if (local_memory_ranges >= MAX_MEMORY_RANGES) {
+ if (local_memory_ranges >= max_memory_ranges) {
fclose(file);
break;
}
@@ -447,8 +523,10 @@ int setup_memory_ranges(unsigned long ke
* nodes. Build list of ranges to be excluded from valid memory
*/
- get_base_ranges();
- get_devtree_details(kexec_flags);
+ if (get_base_ranges())
+ goto out;
+ if (get_devtree_details(kexec_flags))
+ goto out;
for (i = 0; i < nr_exclude_ranges; i++) {
/* If first exclude range does not start with 0, include the
@@ -511,12 +589,21 @@ int setup_memory_ranges(unsigned long ke
fprintf(stderr, "setup_memory_ranges memory_range[%d] start:%lx, end:%lx\n", k, memory_range[k].start, memory_range[k].end);
#endif
return 0;
+
+out:
+ cleanup_memory_ranges();
+ return -1;
}
/* Return a list of valid memory ranges */
int get_memory_ranges(struct memory_range **range, int *ranges,
unsigned long kexec_flags)
{
+ if (count_memory_ranges())
+ return -1;
+ if (alloc_memory_ranges())
+ return -1;
+
setup_memory_ranges(kexec_flags);
*range = memory_range;
*ranges = nr_memory_ranges;
diff -Naurp hormsgittree/kexec/arch/ppc64/kexec-ppc64.h hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/kexec-ppc64.h
--- hormsgittree/kexec/arch/ppc64/kexec-ppc64.h 2006-12-13 11:25:48.000000000 +0530
+++ hormsgittree-dyn-mem-alloc/kexec/arch/ppc64/kexec-ppc64.h 2006-12-13 11:43:00.000000000 +0530
@@ -1,8 +1,6 @@
#ifndef KEXEC_PPC64_H
#define KEXEC_PPC64_H
-#define MAX_MEMORY_RANGES 1024 /* TO FIX - needs to be dynamically set */
-
#define MAXBYTES 128
#define MAX_LINE 160
#define CORE_TYPE_ELF32 1
@@ -17,6 +15,8 @@ void elf_ppc64_usage(void);
void reserve(unsigned long long where, unsigned long long length);
extern unsigned long initrd_base, initrd_size;
+extern int max_memory_ranges;
+
/* boot block version 2 as defined by the linux kernel */
struct bootblock {
unsigned magic,
@@ -39,7 +39,9 @@ struct exclude_range {
typedef struct mem_rgns {
unsigned int size;
- struct exclude_range ranges[MAX_MEMORY_RANGES];
+ struct exclude_range *ranges;
} mem_rgns_t;
+extern mem_rgns_t usablemem_rgns;
+
#endif /* KEXEC_PPC64_H */
_______________________________________________
fastboot mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/fastboot
