Re: [PATCH v3 2/9] kexec_file: Generalize kexec_add_buffer.
From: Dave Young <hidden>
Date: 2016-07-05 00:55:48
Also in:
kexec, lkml
On 07/01/16 at 05:31pm, Thiago Jung Bauermann wrote:
Am Freitag, 01 Juli 2016, 17:02:23 schrieb Thiago Jung Bauermann:quoted
Am Freitag, 01 Juli 2016, 14:36:02 schrieb Dave Young:quoted
On 07/01/16 at 02:51pm, Thiago Jung Bauermann wrote:quoted
Am Donnerstag, 30 Juni 2016, 17:43:57 schrieb Dave Young:quoted
On 06/30/16 at 01:42pm, Thiago Jung Bauermann wrote:quoted
Am Donnerstag, 30 Juni 2016, 12:49:44 schrieb Thiago Jung Bauermann:I understand that this is all somewhat subjective, so if you still disagree with my points I can provide a patch set implementing the change above.I still feel it should be changed if more callbacks being introduced, though you can regard it is internal api, like above comment we do not need to assign them seperately, the member values can be assigned from the beginning.Ok, I'll implement the changes and submit a v4. Thanks for your review.Sorry for creating more email traffic, but it'll be better if I ask this before I change all other places in the code. Is the code below what you have in mind?
Thanks for the update, almost except a nitpick :)
quoted hunk ↗ jump to hunk
In particular, this version doesn't do the memset(&buf, 0, sizeof(buf)) that the previous code I sent earlier did. Is that ok?@@ -643,13 +632,14 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min, unsigned long max, int top_down) { struct purgatory_info *pi = &image->purgatory_info; - unsigned long align, buf_align, bss_align, buf_sz, bss_sz, bss_pad; - unsigned long memsz, entry, load_addr, curr_load_addr, bss_addr, offset; + unsigned long align, bss_align, bss_sz, bss_pad; + unsigned long entry, load_addr, curr_load_addr, bss_addr, offset; unsigned char *buf_addr, *src; int i, ret = 0, entry_sidx = -1; const Elf_Shdr *sechdrs_c; Elf_Shdr *sechdrs = NULL; - void *purgatory_buf = NULL; + struct kexec_buf buf = { .image = image, .buf_min = min, + .buf_max = max, .top_down = top_down }; /* * sechdrs_c points to section headers in purgatory and are read@@ -715,9 +705,9 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min, } /* Determine how much memory is needed to load relocatable object. */ - buf_align = 1; + buf.buf_align = 1; bss_align = 1; - buf_sz = 0; + buf.bufsz = 0; bss_sz = 0;
Above chunk can go to the initializatioin of struct kexec buf ealier.
quoted hunk ↗ jump to hunk
for (i = 0; i < pi->ehdr->e_shnum; i++) {@@ -726,10 +716,10 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min, align = sechdrs[i].sh_addralign; if (sechdrs[i].sh_type != SHT_NOBITS) { - if (buf_align < align) - buf_align = align; - buf_sz = ALIGN(buf_sz, align); - buf_sz += sechdrs[i].sh_size; + if (buf.buf_align < align) + buf.buf_align = align; + buf.bufsz = ALIGN(buf.bufsz, align); + buf.bufsz += sechdrs[i].sh_size; } else { /* bss section */ if (bss_align < align)@@ -741,32 +731,31 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min, /* Determine the bss padding required to align bss properly */ bss_pad = 0; - if (buf_sz & (bss_align - 1)) - bss_pad = bss_align - (buf_sz & (bss_align - 1)); + if (buf.bufsz & (bss_align - 1)) + bss_pad = bss_align - (buf.bufsz & (bss_align - 1)); - memsz = buf_sz + bss_pad + bss_sz; + buf.memsz = buf.bufsz + bss_pad + bss_sz; /* Allocate buffer for purgatory */ - purgatory_buf = vzalloc(buf_sz); - if (!purgatory_buf) { + buf.buffer = vzalloc(buf.bufsz); + if (!buf.buffer) { ret = -ENOMEM; goto out; } - if (buf_align < bss_align) - buf_align = bss_align; + if (buf.buf_align < bss_align) + buf.buf_align = bss_align; /* Add buffer to segment list */ - ret = kexec_add_buffer(image, purgatory_buf, buf_sz, memsz, - buf_align, min, max, top_down, - &pi->purgatory_load_addr); + ret = kexec_add_buffer(&buf); if (ret) goto out; + pi->purgatory_load_addr = buf.mem; /* Load SHF_ALLOC sections */ - buf_addr = purgatory_buf; + buf_addr = buf.buffer; load_addr = curr_load_addr = pi->purgatory_load_addr; - bss_addr = load_addr + buf_sz + bss_pad; + bss_addr = load_addr + buf.bufsz + bss_pad; for (i = 0; i < pi->ehdr->e_shnum; i++) { if (!(sechdrs[i].sh_flags & SHF_ALLOC))@@ -812,11 +801,11 @@ static int __kexec_load_purgatory(struct kimage *image, unsigned long min, * Used later to identify which section is purgatory and skip it * from checksumming. */ - pi->purgatory_buf = purgatory_buf; + pi->purgatory_buf = buf.buffer; return ret; out: vfree(sechdrs); - vfree(purgatory_buf); + vfree(buf.buffer); return ret; }[]'s Thiago Jung Bauermann IBM Linux Technology Center