Skip to contentSkip to conversation

kexec: remove duplicated backup_src_start field from struct crash_elf_info

PATCH Applied Amerigo Wang 3 participants 9 messages Apr 27, 2011 – Apr 29, 2011 1 message not in the archive

Subsystems (2)
Live update kexec handover (KHO)
lore
Share & tools
lore

Amerigo Wang

author
#0 lore raw

PATCH 1/1 kexec: remove duplicated backup_src_start field from struct crash_elf_info

Vivek pointed out that we have duplicated ->backup_src_start in struct crash_elf_info and struct kexec_info.

This patch removes the ->backup_src_start and ->backup_src_end from struct crash_elf_info.

I tested it on both i686 and ppc64, and used a test case from Dave Anderson to confirm the backup region is correct on i686.

Signed-off-by WANG Cong <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Simon Horman <[email protected]>

---

kexec/arch/i386/crashdump-x86.c +5 -8
index 01f470d..98cda72 100644
@@ -721,16 +721,15 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
 	struct memory_range *mem_range, *memmap_p;
 	struct crash_elf_info elf_info;
 	unsigned kexec_arch;
+	unsigned long tmp_backup_end = 0;
 
 	memset(&elf_info, 0x0, sizeof(elf_info));
 
 	/* Constant parts of the elf_info */
 	memset(&elf_info, 0, sizeof(elf_info));
 	elf_info.data             = ELFDATA2LSB;
-	get_backup_area(&elf_info.backup_src_start, &elf_info.backup_src_end);
-	info->backup_src_start = elf_info.backup_src_start;
-	info->backup_src_size = elf_info.backup_src_end
-				- elf_info.backup_src_start + 1;
+	get_backup_area(&info->backup_src_start, &tmp_backup_end);
+	info->backup_src_size = tmp_backup_end - info->backup_src_start + 1;
 
 	/* Get the architecture of the running kernel */
 	kexec_arch = info->kexec_flags & KEXEC_ARCH_MASK;

@@ -785,15 +784,13 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
 	sz = (sizeof(struct memory_range) * (KEXEC_MAX_SEGMENTS + 1));
 	memmap_p = xmalloc(sz);
 	memset(memmap_p, 0, sz);
-	add_memmap(memmap_p, elf_info.backup_src_start,
-		   elf_info.backup_src_end - elf_info.backup_src_start + 1);
+	add_memmap(memmap_p, info->backup_src_start, info->backup_src_size);
 	sz = crash_reserved_mem.end - crash_reserved_mem.start +1;
 	add_memmap(memmap_p, crash_reserved_mem.start, sz);
 
 	/* Create a backup region segment to store backup data*/
 	if (!(info->kexec_flags & KEXEC_PRESERVE_CONTEXT)) {
-		sz = (elf_info.backup_src_end - elf_info.backup_src_start + align)
-		     & ~(align - 1);
+		sz = (info->backup_src_size + align) & ~(align - 1);
 		tmp = xmalloc(sz);
 		memset(tmp, 0, sz);
 		info->backup_start = add_buffer(info, tmp, sz, sz, align,
kexec/arch/mips/crashdump-mips.c +0 -4
index aa50109..4fd6c30 100644
@@ -329,8 +329,6 @@ static struct crash_elf_info elf_info64 = {
 	class: ELFCLASS64,
 	data : ELFDATA2MSB,
 	machine : EM_MIPS,
-	backup_src_start : BACKUP_SRC_START,
-	backup_src_end : BACKUP_SRC_END,
 	page_offset : PAGE_OFFSET,
 	lowmem_limit : MAXMEM,
 };

@@ -339,8 +337,6 @@ static struct crash_elf_info elf_info32 = {
 	class: ELFCLASS32,
 	data : ELFDATA2MSB,
 	machine : EM_MIPS,
-	backup_src_start : BACKUP_SRC_START,
-	backup_src_end : BACKUP_SRC_END,
 	page_offset : PAGE_OFFSET,
 	lowmem_limit : MAXMEM,
 };
kexec/arch/ppc/crashdump-powerpc.c +0 -4
index 7bfad20..eb82122 100644
@@ -21,8 +21,6 @@ static struct crash_elf_info elf_info64 = {
 class: ELFCLASS64,
 data: ELFDATA2MSB,
 machine: EM_PPC64,
-backup_src_start: BACKUP_SRC_START,
-backup_src_end: BACKUP_SRC_END,
 page_offset: PAGE_OFFSET,
 lowmem_limit: MAXMEM,
 };

@@ -35,8 +33,6 @@ machine: EM_PPC64,
 #else
 machine: EM_PPC,
 #endif
-backup_src_start: BACKUP_SRC_START,
-backup_src_end: BACKUP_SRC_END,
 page_offset: PAGE_OFFSET,
 lowmem_limit: MAXMEM,
 };
kexec/arch/ppc64/crashdump-ppc64.c +0 -4
index 0176bc7..6a66f2a 100644
@@ -40,8 +40,6 @@ static struct crash_elf_info elf_info64 =
 	class: ELFCLASS64,
 	data: ELFDATA2MSB,
 	machine: EM_PPC64,
-	backup_src_start: BACKUP_SRC_START,
-	backup_src_end: BACKUP_SRC_END,
 	page_offset: PAGE_OFFSET,
 	lowmem_limit: MAXMEM,
 };

@@ -51,8 +49,6 @@ static struct crash_elf_info elf_info32 =
 	class: ELFCLASS32,
 	data: ELFDATA2MSB,
 	machine: EM_PPC64,
-	backup_src_start: BACKUP_SRC_START,
-	backup_src_end: BACKUP_SRC_END,
 	page_offset: PAGE_OFFSET,
 	lowmem_limit: MAXMEM,
 };
kexec/crashdump-elf.c +2 -2
index 954d670..8d82db9 100644
@@ -227,8 +227,8 @@ int FUNC(struct kexec_info *info,
 		phdr->p_flags	= PF_R|PF_W|PF_X;
 		phdr->p_offset	= mstart;
 
-		if (mstart == elf_info->backup_src_start
-		    && mend == elf_info->backup_src_end)
+		if (mstart == info->backup_src_start
+		    && (mend - mstart + 1) == info->backup_src_size)
 			phdr->p_offset	= info->backup_start;
 
 		/* We already prepared the header for kernel text. Map
kexec/crashdump.h +0 -3
index 5a597eb..0f7c2ea 100644
@@ -23,9 +23,6 @@ struct crash_elf_info {
 	unsigned long data;
 	unsigned long machine;
 
-	unsigned long backup_src_start;
-	unsigned long backup_src_end;
-
 	unsigned long long page_offset;
 	unsigned long long kern_vaddr_start;
 	unsigned long long kern_paddr_start;

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Applied
#1 lore raw
Quoted 6 lines · Amerigo Wang

Vivek pointed out that we have duplicated ->backup_src_start in struct crash_elf_info and struct kexec_info.

This patch removes the ->backup_src_start and ->backup_src_end from struct crash_elf_info.

Quoted 6 lines

I tested it on both i686 and ppc64, and used a test case from Dave Anderson to confirm the backup region is correct on i686.

Signed-off-by WANG Cong <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Simon Horman <[email protected]>

Thanks, applied.

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#2 lore raw
In reply to Amerigo Wang
Quoted 83 lines · Amerigo Wang

Vivek pointed out that we have duplicated ->backup_src_start in struct crash_elf_info and struct kexec_info.

This patch removes the ->backup_src_start and ->backup_src_end from struct crash_elf_info.

I tested it on both i686 and ppc64, and used a test case from Dave Anderson to confirm the backup region is correct on i686.

Signed-off-by WANG Cong <[email protected]>
Cc Vivek Goyal <[email protected]>
Cc Simon Horman <[email protected]>

---

kexec/arch/i386/crashdump-x86.c +5 -8
index 01f470d..98cda72 100644
@@ -721,16 +721,15 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
 	struct memory_range *mem_range, *memmap_p;
 	struct crash_elf_info elf_info;
 	unsigned kexec_arch;
+	unsigned long tmp_backup_end = 0;
 
 	memset(&elf_info, 0x0, sizeof(elf_info));
 
 	/* Constant parts of the elf_info */
 	memset(&elf_info, 0, sizeof(elf_info));
 	elf_info.data             = ELFDATA2LSB;
-	get_backup_area(&elf_info.backup_src_start, &elf_info.backup_src_end);
-	info->backup_src_start = elf_info.backup_src_start;
-	info->backup_src_size = elf_info.backup_src_end
-				- elf_info.backup_src_start + 1;
+	get_backup_area(&info->backup_src_start, &tmp_backup_end);
+	info->backup_src_size = tmp_backup_end - info->backup_src_start + 1;
 
 	/* Get the architecture of the running kernel */
 	kexec_arch = info->kexec_flags & KEXEC_ARCH_MASK;

@@ -785,15 +784,13 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
 	sz = (sizeof(struct memory_range) * (KEXEC_MAX_SEGMENTS + 1));
 	memmap_p = xmalloc(sz);
 	memset(memmap_p, 0, sz);
-	add_memmap(memmap_p, elf_info.backup_src_start,
-		   elf_info.backup_src_end - elf_info.backup_src_start + 1);
+	add_memmap(memmap_p, info->backup_src_start, info->backup_src_size);
 	sz = crash_reserved_mem.end - crash_reserved_mem.start +1;
 	add_memmap(memmap_p, crash_reserved_mem.start, sz);
 
 	/* Create a backup region segment to store backup data*/
 	if (!(info->kexec_flags & KEXEC_PRESERVE_CONTEXT)) {
-		sz = (elf_info.backup_src_end - elf_info.backup_src_start + align)
-		     & ~(align - 1);
+		sz = (info->backup_src_size + align) & ~(align - 1);
 		tmp = xmalloc(sz);
 		memset(tmp, 0, sz);
 		info->backup_start = add_buffer(info, tmp, sz, sz, align,
kexec/arch/mips/crashdump-mips.c +0 -4
index aa50109..4fd6c30 100644
@@ -329,8 +329,6 @@ static struct crash_elf_info elf_info64 = {
 	class: ELFCLASS64,
 	data : ELFDATA2MSB,
 	machine : EM_MIPS,
-	backup_src_start : BACKUP_SRC_START,
-	backup_src_end : BACKUP_SRC_END,
 	page_offset : PAGE_OFFSET,
 	lowmem_limit : MAXMEM,
 };

@@ -339,8 +337,6 @@ static struct crash_elf_info elf_info32 = {
 	class: ELFCLASS32,
 	data : ELFDATA2MSB,
 	machine : EM_MIPS,
-	backup_src_start : BACKUP_SRC_START,
-	backup_src_end : BACKUP_SRC_END,
 	page_offset : PAGE_OFFSET,
 	lowmem_limit : MAXMEM,
 };
kexec/arch/ppc/crashdump-powerpc.c +0 -0
index 7bfad20..eb82122 100644

kexec/arch/ppc/crashdump-powerpc.c static struct crash_elf_info elf_info64 = { · line 25

Quoted 5 lines
kexec/arch/ppc/crashdump-powerpc.c +0 -2
@@ -21,8 +21,6 @@ static struct crash_elf_info elf_info64 = {
 class: ELFCLASS64,
 data: ELFDATA2MSB,
 machine: EM_PPC64,
-backup_src_start: BACKUP_SRC_START,
-backup_src_end: BACKUP_SRC_END,

Amerigo,

Now who will do following initialization for ppc?

info->backup_src_start = BACKUP_SRC_START; info->backup_sz = BACKUP_SRC_END - BACKUP_SRC_START + 1;

I think I have concern for mips too.

Thanks
Vivek

Quoted 54 lines
kexec/arch/ppc/crashdump-powerpc.c +0 -2
 page_offset: PAGE_OFFSET,
 lowmem_limit: MAXMEM,
 };

@@ -35,8 +33,6 @@ machine: EM_PPC64,
 #else
 machine: EM_PPC,
 #endif
-backup_src_start: BACKUP_SRC_START,
-backup_src_end: BACKUP_SRC_END,
 page_offset: PAGE_OFFSET,
 lowmem_limit: MAXMEM,
 };
kexec/arch/ppc64/crashdump-ppc64.c +0 -4
index 0176bc7..6a66f2a 100644
@@ -40,8 +40,6 @@ static struct crash_elf_info elf_info64 =
 	class: ELFCLASS64,
 	data: ELFDATA2MSB,
 	machine: EM_PPC64,
-	backup_src_start: BACKUP_SRC_START,
-	backup_src_end: BACKUP_SRC_END,
 	page_offset: PAGE_OFFSET,
 	lowmem_limit: MAXMEM,
 };

@@ -51,8 +49,6 @@ static struct crash_elf_info elf_info32 =
 	class: ELFCLASS32,
 	data: ELFDATA2MSB,
 	machine: EM_PPC64,
-	backup_src_start: BACKUP_SRC_START,
-	backup_src_end: BACKUP_SRC_END,
 	page_offset: PAGE_OFFSET,
 	lowmem_limit: MAXMEM,
 };
kexec/crashdump-elf.c +2 -2
index 954d670..8d82db9 100644
@@ -227,8 +227,8 @@ int FUNC(struct kexec_info *info,
 		phdr->p_flags	= PF_R|PF_W|PF_X;
 		phdr->p_offset	= mstart;
 
-		if (mstart == elf_info->backup_src_start
-		    && mend == elf_info->backup_src_end)
+		if (mstart == info->backup_src_start
+		    && (mend - mstart + 1) == info->backup_src_size)
 			phdr->p_offset	= info->backup_start;
 
 		/* We already prepared the header for kernel text. Map
kexec/crashdump.h +0 -0
index 5a597eb..0f7c2ea 100644

kexec/crashdump.h struct crash_elf_info { · line 28

Quoted 9 lines
kexec/crashdump.h +0 -3
@@ -23,9 +23,6 @@ struct crash_elf_info {
 	unsigned long data;
 	unsigned long machine;
 
-	unsigned long backup_src_start;
-	unsigned long backup_src_end;
-
 	unsigned long long page_offset;
 	unsigned long long kern_vaddr_start;
 	unsigned long long kern_paddr_start;

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw
Quoted 8 lines · Cong Wang
Quoted 7 lines · Vivek Goyal

Amerigo,

Now who will do following initialization for ppc?

info->backup_src_start = BACKUP_SRC_START; info->backup_sz = BACKUP_SRC_END - BACKUP_SRC_START + 1;

Quoted 8 lines
Quoted 1 line

I think I have concern for mips too.

Right, we need to initialize them in load_crashdump_segments().

How about the attached patch? It is untested.

Signed-off-by WANG Cong <[email protected]>

Ok, this one looks better.

Vivek

Quoted 31 lines
kexec/arch/mips/crashdump-mips.c +2 -0
index 4fd6c30..e90b362 100644
@@ -374,6 +374,8 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
 	if (get_crash_memory_ranges(&mem_range, &nr_ranges) < 0)
 		return -1;
 
+	info->backup_src_start = BACKUP_SRC_START;
+	info->backup_src_size = BACKUP_SRC_SIZE;
 	/* Create a backup region segment to store backup data*/
 	sz = (BACKUP_SRC_SIZE + align - 1) & ~(align - 1);
 	tmp = xmalloc(sz);
kexec/arch/ppc/crashdump-powerpc.c +2 -0
index 371137c..1dd6485 100644
@@ -328,6 +328,8 @@ int load_crashdump_segments(struct kexec_info *info, char *mod_cmdline,
 	if (get_crash_memory_ranges(&mem_range, &nr_ranges) < 0)
 		return -1;
 
+	info->backup_src_start = BACKUP_SRC_START;
+	info->backup_src_size = BACKUP_SRC_SIZE;
 	/* Create a backup region segment to store backup data*/
 	sz = (BACKUP_SRC_SIZE + align - 1) & ~(align - 1);
 	tmp = xmalloc(sz);
kexec/arch/ppc64/crashdump-ppc64.c +0 -0
index 6a66f2a..d52b438 100644

kexec/arch/ppc64/crashdump-ppc64.c int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline, · line 397

Quoted 8 lines
kexec/arch/ppc64/crashdump-ppc64.c +2 -0
@@ -390,6 +390,8 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline,
 	if (get_crash_memory_ranges(&mem_range, &nr_ranges) < 0)
 		return -1;
 
+	info->backup_src_start = BACKUP_SRC_START;
+	info->backup_src_size = BACKUP_SRC_SIZE;
 	/* Create a backup region segment to store backup data*/
 	sz = (BACKUP_SRC_SIZE + align - 1) & ~(align - 1);
 	tmp = xmalloc(sz);

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#5 lore raw
Quoted 11 lines · Vivek Goyal
Quoted 10 lines · Cong Wang
Quoted 8 lines · Vivek Goyal

Amerigo,

Now who will do following initialization for ppc?

info->backup_src_start = BACKUP_SRC_START; info->backup_sz = BACKUP_SRC_END - BACKUP_SRC_START + 1;

I think I have concern for mips too.

Quoted 8 lines
Quoted 6 lines

Right, we need to initialize them in load_crashdump_segments().

How about the attached patch? It is untested.

Signed-off-by WANG Cong <[email protected]>

Ok, this one looks better.

Vivek, can I take that as an Acked-by?

Cong, can you clarify if I need to revert any of your recent patches or not?

Thanks

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Acked-by
#6 lore raw
Quoted 14 lines · Simon Horman
Quoted 13 lines · Vivek Goyal
Quoted 12 lines · Cong Wang
Quoted 8 lines · Vivek Goyal

Amerigo,

Now who will do following initialization for ppc?

info->backup_src_start = BACKUP_SRC_START; info->backup_sz = BACKUP_SRC_END - BACKUP_SRC_START + 1;

I think I have concern for mips too.

Right, we need to initialize them in load_crashdump_segments().

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

How about the attached patch? It is untested.

Signed-off-by WANG Cong <[email protected]>

Ok, this one looks better.

Vivek, can I take that as an Acked-by?

Simon, yes please.

Acked-by Vivek Goyal <[email protected]>

Vivek

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#7 lore raw
Quoted 20 lines · Vivek Goyal
Quoted 19 lines · Simon Horman
Quoted 18 lines · Vivek Goyal
Quoted 16 lines · Cong Wang
Quoted 8 lines · Vivek Goyal

Amerigo,

Now who will do following initialization for ppc?

info->backup_src_start = BACKUP_SRC_START; info->backup_sz = BACKUP_SRC_END - BACKUP_SRC_START + 1;

I think I have concern for mips too.

Right, we need to initialize them in load_crashdump_segments().

How about the attached patch? It is untested.

Signed-off-by WANG Cong <[email protected]>
Quoted 7 lines
Quoted 3 lines
Quoted 1 line

Ok, this one looks better.

Vivek, can I take that as an Acked-by?

Simon, yes please.

Acked-by Vivek Goyal <[email protected]>

Thanks

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

author
#8 lore raw
In reply to Simon Horman
Quoted 17 lines · Simon Horman
Quoted 16 lines · Vivek Goyal
Quoted 14 lines · Cong Wang
Quoted 8 lines · Vivek Goyal

Amerigo,

Now who will do following initialization for ppc?

info->backup_src_start = BACKUP_SRC_START; info->backup_sz = BACKUP_SRC_END - BACKUP_SRC_START + 1;

I think I have concern for mips too.

Right, we need to initialize them in load_crashdump_segments().

How about the attached patch? It is untested.

Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by WANG Cong<[email protected]>

Ok, this one looks better.

Vivek, can I take that as an Acked-by?

Cong, can you clarify if I need to revert any of your recent patches or not?

No, that attached patch is right to fix the first one, so you don't need to revert anything.

Thanks!

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#9 lore raw
Quoted 22 lines · Cong Wang
Quoted 21 lines · Simon Horman
Quoted 19 lines · Vivek Goyal
Quoted 8 lines · Cong Wang
Quoted 7 lines · Vivek Goyal

Amerigo,

Now who will do following initialization for ppc?

info->backup_src_start = BACKUP_SRC_START; info->backup_sz = BACKUP_SRC_END - BACKUP_SRC_START + 1;

Quoted 8 lines
Quoted 1 line

I think I have concern for mips too.

Right, we need to initialize them in load_crashdump_segments().

How about the attached patch? It is untested.

Signed-off-by WANG Cong<[email protected]>

Ok, this one looks better.

Quoted 7 lines
Quoted 4 lines

Vivek, can I take that as an Acked-by?

Cong, can you clarify if I need to revert any of your recent patches or not?

No, that attached patch is right to fix the first one, so you don't need to revert anything.

Thanks, pushed.

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec