On Mon, 4 Jul 2011 23:04:39 +0900
Minchan Kim <minchan.kim@xxxxxxxxx> wrote:
> This patch defines new APIs to put back new page into old page's position as LRU order.
> for LRU churning of compaction.
>
> The idea I suggested in LSF/MM is simple.
>
> ...
>
> +static bool same_lru(struct page *page, struct page *prev)
> +{
> + bool ret = false;
> + if (!prev || !PageLRU(prev))
Both parts of this test need explanations so readers can understand why
they are here.
> + goto out;
> +
> + if (unlikely(PageUnevictable(prev)))
As does this.
> + goto out;
> +
> + if (page_lru_base_type(page) != page_lru_base_type(prev))
> + goto out;
This (and testing for PageLRU) is the only part of this function whcih
is sufficiently obvious to leave undocumented.
> + ret = true;
> +out:
> + return ret;
> +}
> +
> +void putback_ilru_pages(struct inorder_lru *l)
> +{
> + struct zone *zone;
> + struct page *page, *page2, *prev;
> +
> + list_for_each_ilru_entry_safe(page, page2, l, ilru) {
> + ilru_list_del(page, l);
> + dec_zone_page_state(page, NR_ISOLATED_ANON +
> + page_is_file_cache(page));
> + zone = page_zone(page);
> + spin_lock_irq(&zone->lru_lock);
> + prev = page->ilru.prev_page;
> + if (same_lru(page, prev)) {
> + putback_page_to_lru(page, prev);
> + spin_unlock_irq(&zone->lru_lock);
> + put_page(page);
> + } else {
> + spin_unlock_irq(&zone->lru_lock);
> + putback_lru_page(page);
> + }
> + }
> +}
This function takes lru_lock at lest once per page, up to twice per
page. The spinlocking frequency here could be optimised tremendously.
The trick of hanging onto zone->lru_lock is the zone didn't change gets
hard if we want to do a put_page() inside the loop.
We have functions "putback_page_to_lru()" and "putback_lru_page()".
Ugh. Can we think of better naming?
Does this function even need to exist if CONFIG_MIGRATION=n?
> +/*
> * Restore a potential migration pte to a working pte entry
> */
>
> ...
>
> +void __put_ilru_pages(struct page *page, struct page *newpage,
> + struct inorder_lru *prev_lru, struct inorder_lru *ihead)
The function name leaves me wondering where we put the pages, and
there's no documentation telling me.
> +{
> + struct page *prev_page;
> + struct zone *zone;
> + prev_page = page->ilru.prev_page;
> + /*
> + * A page that has been migrated has all references
> + * removed and will be freed. A page that has not been
> + * migrated will have kepts its references and be
> + * restored.
> + */
> + ilru_list_del(page, prev_lru);
> + dec_zone_page_state(page, NR_ISOLATED_ANON +
> + page_is_file_cache(page));
> +
> + /*
> + * Move the new page to the LRU. If migration was not successful
> + * then this will free the page.
> + */
> + zone = page_zone(newpage);
> + spin_lock_irq(&zone->lru_lock);
> + if (same_lru(page, prev_page)) {
> + putback_page_to_lru(newpage, prev_page);
> + spin_unlock_irq(&zone->lru_lock);
> + /*
> + * The newpage replaced LRU position of old page and
> + * old one would be freed. So let's adjust prev_page of pages
> + * remained in inorder_lru list.
> + */
> + adjust_ilru_prev_page(ihead, page, newpage);
> + put_page(newpage);
> + } else {
> + spin_unlock_irq(&zone->lru_lock);
> + putback_lru_page(newpage);
> + }
The same spinlocking frequency issue.
> + putback_lru_page(page);
> +}
> +
>
> ...
>
> +int migrate_ilru_pages(struct inorder_lru *ihead, new_page_t get_new_page,
> + unsigned long private, bool offlining, bool sync)
> +{
> + int retry = 1;
> + int nr_failed = 0;
> + int pass = 0;
> + struct page *page, *page2;
> + struct inorder_lru *prev;
> + int swapwrite = current->flags & PF_SWAPWRITE;
> + int rc;
> +
> + if (!swapwrite)
> + current->flags |= PF_SWAPWRITE;
> +
> + for (pass = 0; pass < 10 && retry; pass++) {
That ten-passes thing was too ugly to live, and now it's breeding. Argh.
> + retry = 0;
> + prev = ihead;
> + list_for_each_ilru_entry_safe(page, page2, ihead, ilru) {
> + cond_resched();
> +
> + rc = unmap_and_move_ilru(get_new_page, private,
> + page, pass > 2, offlining,
> + sync, prev, ihead);
> +
> + switch (rc) {
> + case -ENOMEM:
> + goto out;
> + case -EAGAIN:
> + retry++;
> + prev = &page->ilru;
> + break;
> + case 0:
> + break;
> + default:
> + /* Permanent failure */
> + nr_failed++;
> + break;
> + }
> + }
> + }
> + rc = 0;
> +out:
> + if (!swapwrite)
> + current->flags &= ~PF_SWAPWRITE;
> +
> + if (rc)
> + return rc;
> +
> + return nr_failed + retry;
> +}
> +
>
> ...
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. 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>