[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4BECEDAA.9070009@oracle.com>
Date: Thu, 13 May 2010 23:28:58 -0700
From: Yinghai <yinghai.lu@...cle.com>
To: Benjamin Herrenschmidt <benh@...nel.crashing.org>
CC: Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>,
"H. Peter Anvin" <hpa@...or.com>,
Andrew Morton <akpm@...ux-foundation.org>,
David Miller <davem@...emloft.net>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Johannes Weiner <hannes@...xchg.org>,
linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org
Subject: Re: [PATCH 05/35] x86, lmb: Add lmb_find_area_size()
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@...nel.org>
>> ---
>> 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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists