Re: [PATCH 05/12] mm: alloc_contig_range() added

From: Michal Nazarewicz
Date: Thu Mar 31 2011 - 17:18:13 EST


On Thu, 31 Mar 2011 22:28:42 +0200, Dave Hansen <dave@xxxxxxxxxxxxxxxxxx> wrote:

On Thu, 2011-03-31 at 18:26 +0200, Michal Nazarewicz wrote:
> On Thu, 2011-03-31 at 15:16 +0200, Marek Szyprowski wrote:
>> + ret = 0;
>> + while (!PageBuddy(pfn_to_page(start & (~0UL << ret))))
>> + if (WARN_ON(++ret >= MAX_ORDER))
>> + return -EINVAL;

On Thu, 31 Mar 2011 18:02:41 +0200, Dave Hansen wrote:
> Holy cow, that's dense. Is there really no more straightforward way to
> do that?

Which part exactly is dense? What would be qualify as a more
straightforward way?

I'm still not 100% sure what it's trying to do. It looks like it
attempts to check all of "start"'s buddy pages.

No. I'm going up through parents. This is because even though start
falls in a free block (ie. one that page allocator tracks), the actual
page that is in buddy system is larger then start and this loop looks
for beginning of that page.

int order;
for (order = 0; order <= MAX_ORDER; order++) {
unsigned long buddy_pfn = find_buddy(start, order);
struct page *buddy = pfn_to_page(buddy_pfn);
if (PageBuddy(buddy)
break;
WARN();
return -EINVAL;
}

The WARN() and return would have to be outside of the loop and, as I
described, instead of find_buddy() something like find_parent() would
have to be used.

I'm wondering also if you can share some code with __rmqueue().

Doubtful since start does not (have to) point to a page that is tracked
by page allocator but a page inside such a page.

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: mnazarewicz@xxxxxxxxxx>-----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/