linux-kernel - Re: [PATCH] Add alloc_pages_exact() and free_pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1214343218.12367.58.camel@nimitz>
Date:	Tue, 24 Jun 2008 14:33:38 -0700
From:	Dave Hansen <dave@...ux.vnet.ibm.com>
To:	Timur Tabi <timur@...escale.com>
Cc:	linux-kernel@...r.kernel.org, andi@...stfloor.org,
	randy.dunlap@...cle.com, corbet@....net,
	torvalds@...ux-foundation.org
Subject: Re: [PATCH] Add alloc_pages_exact() and free_pages_exact()

On Tue, 2008-06-24 at 11:40 -0500, Timur Tabi wrote:
> +void *alloc_pages_exact(size_t size, gfp_t gfp_mask)
> +{
> +       unsigned int order = get_order(size);
> +       unsigned long addr;
> +
> +       addr = __get_free_pages(gfp_mask, order);
> +       if (addr) {
> +               unsigned long alloc_end = addr + (PAGE_SIZE << order);
> +               unsigned long used = addr + PAGE_ALIGN(size);
> +
> +               split_page(virt_to_page(addr), order);
> +               while (used < alloc_end) {
> +                       free_page(used);
> +                       used += PAGE_SIZE;
> +               }
> +       }
> +
> +       return (void *)addr;
> +}

Hi Timur,

This looks like a really good idea.  It looks pretty good to me, no
functional problems.  My brain had a really hard time parsing that code
for some reason, though.  Could just be a lack of coffee.

I think the thing that confused me was trying to figure out if
'alloc_end' was the end of what we *did* allocate from
__get_free_pages() or if it was the *goal* allocation end.

'used' also seemed like a slightly strange variable name because it
points to the memory which is about to be freed and ends up *unused*.

I'll offer this up just in case you like it better.  For me, it is
easier to parse, and should do the exact same thing.  I also think it's
slightly nicer to do the arithmetic on 'struct page *' rather than
vaddrs in 'unsigned long'.  It is _slightly_ cheaper not having to do a
virt_to_page() on each free_page() call.  The same would go for the free
side as well.

All of the 'struct page *' arithmetic is OK since it is all done inside
one MAX_ORDER area.

void *alloc_pages_exact(size_t size, gfp_t gfp_mask)
{
	unsigned int order = get_order(size);
	void *alloc;
	struct page *surplus_start;
	struct page *surplus_end;
	struct page *page;

	size = PAGE_ALIGN(size);

	alloc = (void *)__get_free_pages(gfp_mask, order);
	if (!alloc)
		return NULL;

	/* Turn the big allocation into a bunch of single pages */
	split_page(virt_to_page(alloc), order);

	surplus_start = virt_to_page(alloc + size);
	surplus_end = surplus_start + (1 << order);

	for (page = surplus_start; page < surplus_end; page++)
		__free_page(page);

	return alloc;
}

-- Dave

--
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/