Re: [RFC][PATCH 3/3] alloc contig pages with migration.

From: KAMEZAWA Hiroyuki
Date: Sun Oct 17 2010 - 20:41:05 EST

Next message: KAMEZAWA Hiroyuki: "Re: [PATCH v2 02/11] memcg: document cgroup dirty memory interfaces"
Previous message: KAMEZAWA Hiroyuki: "Re: [RFC][PATCH 2/3] find a contiguous range."
In reply to: Minchan Kim: "Re: [RFC][PATCH 3/3] alloc contig pages with migration."
Next in thread: Minchan Kim: "Re: [RFC][PATCH 3/3] alloc contig pages with migration."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Sun, 17 Oct 2010 13:05:22 +0900
Minchan Kim <minchan.kim@xxxxxxxxx> wrote:

> On Wed, Oct 13, 2010 at 12:18 PM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@xxxxxxxxxxxxxx> wrote:
> > From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> >
> > Add an function to allocate contigous memory larger than MAX_ORDER.
> > The main difference between usual page allocater is that this uses
> > memory offline techiqueue (Isoalte pages and migrate remaining pages.).
> >
> > I think this is not 100% solution because we can't avoid fragmentation,
> > but we have kernelcore= boot option and can create MOVABLE zone. That
> > helps us to allow allocate a contigous range on demand.
> >
> > Maybe drivers can alloc contig pages by bootmem or hiding some memory
> > from the kernel at boot. But if contig pages are necessary only in some
> > situation, kernelcore= boot option and using page migration is a choice.
> >
> > Anyway, to allocate a contiguous chunk larger than MAX_ORDER, we need to
> > add an overlay allocator on buddy allocator. This can be a 1st step.
> >
> > Note:
> > This function is heavy if there are tons of memory requesters. So, maybe
> > not good for 1GB pages for x86's usual use. It will requires some other
> > tricks than migration.
>
> I got found many typos but I don't pointed out each by each. :)
> Please, correct typos in next version.
>
Sorry.

> >
> > TODO:
> > Â- allows the caller to specify the migration target pages.
> > Â- reduce the number of lru_add_drain_all()..etc...system wide heavy calls.
> > Â- Pass gfp_t for some purpose...
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> > ---
> > Âinclude/linux/page-isolation.h | Â Â8 ++
> > Âmm/page_alloc.c Â Â Â Â Â Â Â Â| Â 29 ++++++++
> > Âmm/page_isolation.c Â Â Â Â Â Â| Â136 +++++++++++++++++++++++++++++++++++++++++
> > Â3 files changed, 173 insertions(+)
> >
> > Index: mmotm-1008/mm/page_isolation.c
> > ===================================================================
> > --- mmotm-1008.orig/mm/page_isolation.c
> > +++ mmotm-1008/mm/page_isolation.c
> > @@ -7,6 +7,7 @@
> > Â#include <linux/mm.h>
> > Â#include <linux/page-isolation.h>
> > Â#include <linux/pageblock-flags.h>
> > +#include <linux/swap.h>
> > Â#include <linux/memcontrol.h>
> > Â#include <linux/migrate.h>
> > Â#include <linux/memory_hotplug.h>
> > @@ -384,3 +385,138 @@ retry:
> > Â Â Â Â}
> > Â Â Â Âreturn 0;
> > Â}
> > +
> > +/**
> > + * alloc_contig_pages - allocate a contigous physical pages
> > + * @hint: Â Â Âthe base address of searching free space(in pfn)
> > + * @size: Â Â Âsize of requested area (in # of pages)
>
> Could you add _range_ which can be specified by user into your TODO list?
> Maybe some embedded system have a requirement to allocate contiguous
> pages in some bank.
> so guys try to allocate pages in some base address and if it fails, he
> can try to next offset in same bank.
> But it's very annoying. So let's add feature that user can specify
> _range_ where user want to allocate.

Add [start, end) to the argument.

>
> > + * @node: Â Â Â the node from which memory is allocated. "-1" means anywhere.
> > + * @no_search: if true, "hint" is not a hint, requirement.
>
> As I said previous, how about "strict" or "ALLOC_FIXED" like MAP_FIXED?
>

If "range" is an argument, ALLOC_FIXED is not necessary. I'll add "range".

> > + *
> > + * Search an area of @size in the physical memory map and checks wheter
>
> Typo
> whether
>
> > + * we can create a contigous free space. If it seems possible, try to
> > + * create contigous space with page migration. If no_search==true, we just try
> > + * to allocate [hint, hint+size) range of pages as contigous block.
> > + *
> > + * Returns a page of the beginning of contiguous block. At failure, NULL
> > + * is returned. Each page in the area is set to page_count() = 1. Because
>
> Why do you mention page_count() = 1?
> Do users of this function have to know it?

A user can free any page within the range for his purpose.

> > + * this function does page migration, this function is very heavy and
>
> Nitpick.
> page migration is implementation, too. Do we need to mention it in here?
> We might add page reclaim/or new feature in future or page migration
> might be very light function although it is not a easy. :)
> Let's not show the implementation for users.
>

ok.

> > + * sleeps some time. Caller must be aware that "NULL returned" is not a
> > + * special case.
>
> I think this information is enough to users.
>
> > + *
> > + * Now, returned range is aligned to MAX_ORDER. (So "hint" must be aligned
> > + * if no_search==true.)
>
> Couldn't we add handling of this exception?
> If (hint != MAX_ORDER_ALIGH(hint) && no_search == true)
> return 0 or WARN_ON?
>

I'll add "alignment" argument. (for 1G hugepage.)
and add the check.

> > + */
> > +
> > +#define MIGRATION_RETRY Â Â Â Â(5)
> > +struct page *alloc_contig_pages(unsigned long hint, unsigned long size,
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â int node, bool no_search)
> > +{
> > + Â Â Â unsigned long base, found, end, pages, start;
> > + Â Â Â struct page *ret = NULL;
> > + Â Â Â int migration_failed;
> > + Â Â Â struct zone *zone;
> > +
> > + Â Â Â hint = MAX_ORDER_ALIGN(hint);
> > + Â Â Â /*
> > + Â Â Â Â* request size should be aligned to pageblock_order..but use
> > + Â Â Â Â* MAX_ORDER here for avoiding messy checks.
> > + Â Â Â Â*/
> > + Â Â Â pages = MAX_ORDER_ALIGN(size);
> > + Â Â Â found = 0;
> > +retry:
> > + Â Â Â for_each_populated_zone(zone) {
> > + Â Â Â Â Â Â Â unsigned long zone_end_pfn;
> > +
> > + Â Â Â Â Â Â Â if (node >= 0 && node != zone_to_nid(zone))
> > + Â Â Â Â Â Â Â Â Â Â Â continue;
> > + Â Â Â Â Â Â Â if (zone->present_pages < pages)
> > + Â Â Â Â Â Â Â Â Â Â Â continue;
> > + Â Â Â Â Â Â Â base = MAX_ORDER_ALIGN(zone->zone_start_pfn);
> > + Â Â Â Â Â Â Â base = max(base, hint);
> > + Â Â Â Â Â Â Â zone_end_pfn = zone->zone_start_pfn + zone->spanned_pages;
> > + Â Â Â Â Â Â Â if (base + pages > zone_end_pfn)
> > + Â Â Â Â Â Â Â Â Â Â Â continue;
> > + Â Â Â Â Â Â Â found = find_contig_block(base, zone_end_pfn, pages, no_search);
> > + Â Â Â Â Â Â Â /* Next try will see the next block. */
> > + Â Â Â Â Â Â Â hint = base + MAX_ORDER_NR_PAGES;
> > + Â Â Â Â Â Â Â if (found)
> > + Â Â Â Â Â Â Â Â Â Â Â break;
> > + Â Â Â }
> > +
> > + Â Â Â if (!found)
> > + Â Â Â Â Â Â Â return NULL;
> > +
> > + Â Â Â if (no_search && found != hint)
>
> You increased hint before "break".
> So if the no_search is true, this condition (found != hint) is always true.
>
Ah...yes.

>
> > + Â Â Â Â Â Â Â return NULL;
> > +
> > + Â Â Â /*
> > + Â Â Â Â* Ok, here, we have contiguous pageblock marked as "isolated"
> > + Â Â Â Â* try migration.
> > + Â Â Â Â*
> > + Â Â Â Â* FIXME: permanent migration_failure detection logic is required.
> > + Â Â Â Â*/
> > + Â Â Â lru_add_drain_all();
> > + Â Â Â flush_scheduled_work();
> > + Â Â Â drain_all_pages();
> > +
> > + Â Â Â end = found + pages;
> > + Â Â Â /*
> > + Â Â Â Â* scan_lru_pages() finds the next PG_lru page in the range
> > + Â Â Â Â* scan_lru_pages() returns 0 when it reaches the end.
> > + Â Â Â Â*/
> > + Â Â Â for (start = scan_lru_pages(found, end), migration_failed = 0;
> > + Â Â Â Â Â Âstart && start < end;
> > + Â Â Â Â Â Âstart = scan_lru_pages(start, end)) {
> > + Â Â Â Â Â Â Â if (do_migrate_range(start, end)) {
> > + Â Â Â Â Â Â Â Â Â Â Â /* it's better to try another block ? */
> > + Â Â Â Â Â Â Â Â Â Â Â if (++migration_failed >= MIGRATION_RETRY)
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
> > + Â Â Â Â Â Â Â Â Â Â Â /* take a rest and synchronize LRU etc. */
> > + Â Â Â Â Â Â Â Â Â Â Â lru_add_drain_all();
> > + Â Â Â Â Â Â Â Â Â Â Â flush_scheduled_work();
> > + Â Â Â Â Â Â Â Â Â Â Â cond_resched();
> > + Â Â Â Â Â Â Â Â Â Â Â drain_all_pages();
> > + Â Â Â Â Â Â Â } else /* reset migration_failure counter */
> > + Â Â Â Â Â Â Â Â Â Â Â migration_failed = 0;
> > + Â Â Â }
> > +
> > + Â Â Â lru_add_drain_all();
> > + Â Â Â flush_scheduled_work();
> > + Â Â Â drain_all_pages();
>
> Hmm.. as you mentioned, It would be better to remove many flush lru/per-cpu.
> But in embedded system, it couldn't be a big overhead.
>

I'll drop flush_scheduled_work().

> > + Â Â Â /* Check all pages are isolated */
> > + Â Â Â if (test_pages_isolated(found, end)) {
> > + Â Â Â Â Â Â Â undo_isolate_page_range(found, pages);
> > + Â Â Â Â Â Â Â /*
> > + Â Â Â Â Â Â Â Â* We failed at [start...end) migration.
> > + Â Â Â Â Â Â Â Â* FIXME: there may be better restaring point.
> > + Â Â Â Â Â Â Â Â*/
> > + Â Â Â Â Â Â Â hint = MAX_ORDER_ALIGN(end + 1);
> > + Â Â Â Â Â Â Â goto retry; /* goto next chunk */
> > + Â Â Â }
> > + Â Â Â /*
> > + Â Â Â Â* Ok, here, [found...found+pages) memory are isolated.
> > + Â Â Â Â* All pages in the range will be moved into the list with
> > + Â Â Â Â* page_count(page)=1.
> > + Â Â Â Â*/
> > + Â Â Â ret = pfn_to_page(found);
> > + Â Â Â alloc_contig_freed_pages(found, found + pages);
> > + Â Â Â /* unset ISOLATE */
> > + Â Â Â undo_isolate_page_range(found, pages);
> > + Â Â Â /* Free unnecessary pages in tail */
> > + Â Â Â for (start = found + size; start < found + pages; start++)
> > + Â Â Â Â Â Â Â __free_page(pfn_to_page(start));
> > + Â Â Â return ret;
> > +
> > +}
>
> Thanks for the good patches, Kame.
>

Thank you for advices.

-Kame

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

Next message: KAMEZAWA Hiroyuki: "Re: [PATCH v2 02/11] memcg: document cgroup dirty memory interfaces"
Previous message: KAMEZAWA Hiroyuki: "Re: [RFC][PATCH 2/3] find a contiguous range."
In reply to: Minchan Kim: "Re: [RFC][PATCH 3/3] alloc contig pages with migration."
Next in thread: Minchan Kim: "Re: [RFC][PATCH 3/3] alloc contig pages with migration."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]