On Mon, 2011-04-11 at 15:22 -0700, Andrew Morton wrote:
> > +/* 'struct page' version */
> > +struct page *__alloc_pages_exact(gfp_t gfp_mask, size_t size);
> > +void __free_pages_exact(struct page *page, size_t size);
>
> The declarations use "size", but the definitions use "nr_pages".
> "nr_pages" is way better.
I'll fix that.
> Should it really be size_t? size_t's units are "bytes", usually.
Yeah, the nr_pages one should probably be an unsigned long.
> > -void *get_free_pages_exact(gfp_t gfp_mask, size_t size)
> > +struct page *__alloc_pages_exact(gfp_t gfp_mask, size_t nr_pages)
>
> Most allocation functions are of the form foo(size, gfp_t), but this
> one has the args reversed. Was there a reason for that?
I'm trying to make this a clone of alloc_pages(), which does:
#define alloc_pages(gfp_mask, order)
It needs a note in the changelog on why I did it.
> > {
> > - unsigned int order = get_order(size);
> > - unsigned long addr;
> > + unsigned int order = get_order(nr_pages * PAGE_SIZE);
> > + struct page *page;
> >
> > - addr = __get_free_pages(gfp_mask, order);
> > - if (addr) {
> > - unsigned long alloc_end = addr + (PAGE_SIZE << order);
> > - unsigned long used = addr + PAGE_ALIGN(size);
> > + page = alloc_pages(gfp_mask, order);
> > + if (page) {
> > + struct page *alloc_end = page + (1 << order);
> > + struct page *used = page + nr_pages;
> >
> > - split_page(virt_to_page((void *)addr), order);
> > + split_page(page, order);
> > while (used < alloc_end) {
> > - free_page(used);
> > - used += PAGE_SIZE;
> > + __free_page(used);
> > + used++;
> > }
> > }
> >
> > - return (void *)addr;
> > + return page;
> > +}
> > +EXPORT_SYMBOL(__alloc_pages_exact);
> > +
> > +/**
> > + * __free_pages_exact - release memory allocated via __alloc_pages_exact()
> > + * @virt: the value returned by get_free_pages_exact.
> > + * @nr_pages: size in pages, same value as passed to __alloc_pages_exact().
> > + *
> > + * Release the memory allocated by a previous call to __alloc_pages_exact().
> > + */
> > +void __free_pages_exact(struct page *page, size_t nr_pages)
> > +{
> > + struct page *end = page + nr_pages;
> > +
> > + while (page < end) {
>
> Hand-optimised. Old school. Doesn't trust the compiler :)
Hey, ask the dude who put free_pages_exact() in there! :)
> > + __free_page(page);
> > + page++;
> > + }
> > +}
> > +EXPORT_SYMBOL(__free_pages_exact);
>
> Really, this function duplicates release_pages(). release_pages() is
> big and fat and complex and is a crime against uniprocessor but it does
> make some effort to reduce the spinlocking frequency and in many
> situations, release_pages() will cause vastly less locked bus traffic
> than your __free_pages_exact(). And who knows, smart use of
> release_pages()'s "cold" hint may provide some benefits.
Seems like a decent enough thing to try. I'll give it a shot and make
sure it's OK to use.
-- Dave
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>