linux-kernel - Re: [RFC 2/2] cma: support MIGRATE

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: <20120815233323.GB15225@bbox>
Date:	Thu, 16 Aug 2012 08:33:23 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Rik van Riel <riel@...hat.com>
Cc:	Marek Szyprowski <m.szyprowski@...sung.com>,
	Mel Gorman <mgorman@...e.de>,
	Kamezawa Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [RFC 2/2] cma: support MIGRATE_DISCARD

Hi Rik,

On Wed, Aug 15, 2012 at 02:58:01PM -0400, Rik van Riel wrote:
> On 08/14/2012 04:57 AM, Minchan Kim wrote:
> >This patch introudes MIGRATE_DISCARD mode in migration.
> >It drop clean cache pages instead of migration so that
> >migration latency could be reduced. Of course, it could
> >evict code pages but latency of big contiguous memory
> >is more important than some background application's slow down
> >in mobile embedded enviroment.
> 
> Would it be an idea to only drop clean UNMAPPED
> page cache pages?

Firstly I thougt about that but I chose more agressive thing.
Namely, even drop mapped page cache.
Because it can reduce latency more(ex, memcpy + remapping cost
during migration) and it could not trivial if migration range is big.

> 
> >Signed-off-by: Minchan Kim <minchan@...nel.org>
> 
> >@@ -799,12 +802,39 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
> >  		goto skip_unmap;
> >  	}
> >
> >+	file = page_is_file_cache(page);
> >+	ttu_flags = TTU_IGNORE_MLOCK|TTU_IGNORE_ACCESS;
> >+
> >+	if (!(mode & MIGRATE_DISCARD) || !file || PageDirty(page))
> >+		ttu_flags |= TTU_MIGRATION;
> >+	else
> >+		discard_mode = true;
> >+
> >  	/* Establish migration ptes or remove ptes */
> >-	try_to_unmap(page, TTU_MIGRATION|TTU_IGNORE_MLOCK|TTU_IGNORE_ACCESS);
> >+	try_to_unmap(page, ttu_flags);
> 
> This bit looks wrong, because you end up ignoring
> mlock and then discarding the page.

Argh, Thanks!
I will fix it in next spin.

> 
> Only dropping clean page cache pages that are not
> mapped would avoid that problem, without introducing
> much complexity in the code.

Hmm, I don't think it makes code much complex.
How about this?

diff --git a/mm/rmap.c b/mm/rmap.c
index 0f3b7cd..0909d79 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1223,7 +1223,8 @@ out:
  * repeatedly from try_to_unmap_ksm, try_to_unmap_anon or try_to_unmap_file.
  */
 int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
-                    unsigned long address, enum ttu_flags flags)
+                    unsigned long address, enum ttu_flags flags,
+                    unsigned long *vm_flags)
 {
        struct mm_struct *mm = vma->vm_mm;
        pte_t *pte;
@@ -1235,6 +1236,7 @@ int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
        if (!pte)
                goto out;
 
+       vm_flags |= vma->vm_flags;
        /*
         * If the page is mlock()d, we cannot swap it out.
         * If it's recently referenced (perhaps page_referenced
@@ -1652,7 +1654,7 @@ out:
  * SWAP_FAIL   - the page is unswappable
  * SWAP_MLOCK  - page is mlocked.
  */
-int try_to_unmap(struct page *page, enum ttu_flags flags)
+int try_to_unmap(struct page *page, enum ttu_flags flags, unsigned long *vm_flags)
 {
        int ret;

<snip> 

+       file = page_is_file_cache(page);
+       ttu_flags = TTU_IGNORE_MLOCK|TTU_IGNORE_ACCESS;
+
+       if (!(mode & MIGRATE_DISCARD) || !file || PageDirty(page) ||
+               vm_flags & VM_LOCKED)
+               ttu_flags |= TTU_MIGRATION;
+       else
+               discard_mode = true;
+


> 
> That would turn the test above into:
> 
> 	if (!page_mapped(page))
> 		discard_mode = true;
> 
> >  skip_unmap:
> >-	if (!page_mapped(page))
> >-		rc = move_to_new_page(newpage, page, remap_swapcache, mode);
> >+	if (!page_mapped(page)) {
> >+		if (!discard_mode)
> >+			rc = move_to_new_page(newpage, page, remap_swapcache, mode);
> >+		else {
> >+			struct address_space *mapping;
> >+			mapping = page_mapping(page);
> >+
> >+			if (page_has_private(page)) {
> >+				if (!try_to_release_page(page, GFP_KERNEL)) {
> >+					rc = -EAGAIN;
> >+					goto uncharge;
> >+				}
> >+			}
> >+
> >+			if (remove_mapping(mapping, page))
> >+				rc = 0;
> >+			else
> >+				rc = -EAGAIN;
> >+			goto uncharge;
> >+		}
> >+	}
> 
> This big piece of code could probably be split out
> into its own function.
> 
> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@...ck.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>

-- 
Kind regards,
Minchan Kim
--
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/