On 05/13/2010 07:20 PM, Benjamin Herrenschmidt wrote: > On Thu, 2010-05-13 at 17:19 -0700, Yinghai Lu wrote: >> size is returned according free range. >> Will be used to find free ranges for early_memtest and memory corruption check > > Please provide a better explanation of what these functions do. It's > very unclear from the code (which looks like it could be a lot simpler), > and the name of the function is totally obscure as well. this just line by line translation from early_res version to lmb changes please focus on lmb core at this point. > > We have asked you how many times to improve on your changeset comments > at the -very-least- ? Explain what functions do and why they do it, and > when I say explain, I don't mean 2 lines of rot13. I mean actual > sentences that a human being can read and have a chance to understand. > > Also, I would appreciate if you picked up the habit of adding docbook > doco for any API function you add, even if it's in the x86 "internal" > file. > > Cheers, > Ben. > >> Do not mess it up with mm/lmb.c yet. >> >> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx> >> --- >> arch/x86/include/asm/lmb.h | 8 ++++ >> arch/x86/mm/Makefile | 2 + >> arch/x86/mm/lmb.c | 88 ++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 98 insertions(+), 0 deletions(-) >> create mode 100644 arch/x86/include/asm/lmb.h >> create mode 100644 arch/x86/mm/lmb.c >> >> diff --git a/arch/x86/include/asm/lmb.h b/arch/x86/include/asm/lmb.h >> new file mode 100644 >> index 0000000..aa3a66e >> --- /dev/null >> +++ b/arch/x86/include/asm/lmb.h >> @@ -0,0 +1,8 @@ >> +#ifndef _X86_LMB_H >> +#define _X86_LMB_H >> + >> +#define ARCH_DISCARD_LMB >> + >> +u64 lmb_find_area_size(u64 start, u64 *sizep, u64 align); >> + >> +#endif >> diff --git a/arch/x86/mm/Makefile b/arch/x86/mm/Makefile >> index a4c7683..8ab0505 100644 >> --- a/arch/x86/mm/Makefile >> +++ b/arch/x86/mm/Makefile >> @@ -26,4 +26,6 @@ obj-$(CONFIG_NUMA) += numa.o numa_$(BITS).o >> obj-$(CONFIG_K8_NUMA) += k8topology_64.o >> obj-$(CONFIG_ACPI_NUMA) += srat_$(BITS).o >> >> +obj-$(CONFIG_HAVE_LMB) += lmb.o >> + >> obj-$(CONFIG_MEMTEST) += memtest.o >> diff --git a/arch/x86/mm/lmb.c b/arch/x86/mm/lmb.c >> new file mode 100644 >> index 0000000..9d26eed >> --- /dev/null >> +++ b/arch/x86/mm/lmb.c >> @@ -0,0 +1,88 @@ >> +#include <linux/kernel.h> >> +#include <linux/types.h> >> +#include <linux/init.h> >> +#include <linux/bitops.h> >> +#include <linux/lmb.h> >> +#include <linux/bootmem.h> >> +#include <linux/mm.h> >> +#include <linux/range.h> >> + >> +/* Check for already reserved areas */ >> +static inline bool __init bad_addr_size(u64 *addrp, u64 *sizep, u64 align) >> +{ >> + int i; >> + u64 addr = *addrp, last; >> + u64 size = *sizep; >> + bool changed = false; >> +again: >> + last = addr + size; >> + for (i = 0; i < lmb.reserved.cnt && lmb.reserved.regions[i].size; i++) { >> + struct lmb_region *r = &lmb.reserved.regions[i]; >> + if (last > r->base && addr < r->base) { >> + size = r->base - addr; >> + changed = true; >> + goto again; >> + } >> + if (last > (r->base + r->size) && addr < (r->base + r->size)) { >> + addr = round_up(r->base + r->size, align); >> + size = last - addr; >> + changed = true; >> + goto again; >> + } >> + if (last <= (r->base + r->size) && addr >= r->base) { >> + (*sizep)++; >> + return false; >> + } >> + } >> + if (changed) { >> + *addrp = addr; >> + *sizep = size; >> + } >> + return changed; >> +} >> + >> +static u64 __init __lmb_find_area_size(u64 ei_start, u64 ei_last, u64 start, >> + u64 *sizep, u64 align) >> +{ >> + u64 addr, last; >> + >> + addr = round_up(ei_start, align); >> + if (addr < start) >> + addr = round_up(start, align); >> + if (addr >= ei_last) >> + goto out; >> + *sizep = ei_last - addr; >> + while (bad_addr_size(&addr, sizep, align) && addr + *sizep <= ei_last) >> + ; >> + last = addr + *sizep; >> + if (last > ei_last) >> + goto out; >> + >> + return addr; >> + >> +out: >> + return LMB_ERROR; >> +} >> + >> +/* >> + * Find next free range after *start >> + */ >> +u64 __init lmb_find_area_size(u64 start, u64 *sizep, u64 align) >> +{ >> + int i; >> + >> + for (i = 0; i < lmb.memory.cnt; i++) { >> + u64 ei_start = lmb.memory.regions[i].base; >> + u64 ei_last = ei_start + lmb.memory.regions[i].size; >> + u64 addr; >> + >> + addr = __lmb_find_area_size(ei_start, ei_last, start, >> + sizep, align); >> + >> + if (addr != LMB_ERROR) >> + return addr; >> + } >> + >> + return LMB_ERROR; >> +} >> + > -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html