All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michel Lespinasse <walken@google.com>
To: Rik van Riel <riel@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-mm@kvack.org, Ying Han <yinghan@google.com>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Nick Piggin <npiggin@kernel.dk>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 2/3] Retry page fault when blocking on disk transfer.
Date: Thu, 7 Oct 2010 21:39:56 -0700	[thread overview]
Message-ID: <20101008043956.GA25662@google.com> (raw)
In-Reply-To: <AANLkTimdACZ9Xm01DM2+E64+T5XfLffrkFBhf7CJ286p@mail.gmail.com>

On Tue, Oct 05, 2010 at 03:44:22PM -0700, Michel Lespinasse wrote:
> On Tue, Oct 5, 2010 at 10:38 AM, Rik van Riel <riel@redhat.com> wrote:
> > Looks like it should be relatively easy to do something
> > similar in do_swap_page also.
> 
> Good idea. We don't make use of swap too much, which is probably why
> we didn't have that in our kernel, but it seems like a good idea just
> for uniformity. I'll add this in a follow-on patch.

So here's the patch. Sorry for the delay - it did not take long to write,
but I couldn't test it before today.

Please have a look - I'd like to add this to the series I sent earlier.

----------------------------------- 8< ---------------------------------

Retry page fault when blocking on swap in

This change is the cousin of 'Retry page fault when blocking
on disk transfer'. The idea here is to reduce mmap_sem hold times
that are caused by disk transfers when swapping in pages. We drop
mmap_sem while waiting for the page lock, and return the VM_FAULT_RETRY
flag. do_page_fault will then re-acquire mmap_sem and retry the
page fault. It is expected that upon retry the page will now be cached,
and thus the retry will complete with a low mmap_sem hold time.

Signed-off-by: Michel Lespinasse <walken@google.com>

diff --git a/mm/memory.c b/mm/memory.c
index b068c68..0ec70b4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2613,6 +2613,21 @@ int vmtruncate_range(struct inode *inode, loff_t offset, loff_t end)
 	return 0;
 }
 
+static inline int lock_page_or_retry(struct page *page, struct mm_struct *mm,
+				     unsigned int flags)
+{
+	if (trylock_page(page))
+		return 1;
+	if (!(flags & FAULT_FLAG_ALLOW_RETRY)) {
+		__lock_page(page);
+		return 1;
+	}
+
+	up_read(&mm->mmap_sem);
+	wait_on_page_locked(page);
+	return 0;
+}
+
 /*
  * We enter with non-exclusive mmap_sem (to exclude vma changes,
  * but allow concurrent faults), and pte mapped but not yet locked.
@@ -2626,6 +2641,7 @@ static int do_swap_page(struct mm_struct *mm, struct vm_area_struct *vma,
 	struct page *page, *swapcache = NULL;
 	swp_entry_t entry;
 	pte_t pte;
+	int locked;
 	struct mem_cgroup *ptr = NULL;
 	int exclusive = 0;
 	int ret = 0;
@@ -2676,8 +2692,12 @@ static int do_swap_page(struct mm_struct *mm, struct vm_area_struct *vma,
 		goto out_release;
 	}
 
-	lock_page(page);
+	locked = lock_page_or_retry(page, mm, flags);
 	delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
+	if (!locked) {
+		ret |= VM_FAULT_RETRY;
+		goto out_release;
+	}
 
 	/*
 	 * Make sure try_to_free_swap or reuse_swap_page or swapoff did not


-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

WARNING: multiple messages have this Message-ID (diff)
From: Michel Lespinasse <walken@google.com>
To: Rik van Riel <riel@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-mm@kvack.org, Ying Han <yinghan@google.com>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Nick Piggin <npiggin@kernel.dk>,
	Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 2/3] Retry page fault when blocking on disk transfer.
Date: Thu, 7 Oct 2010 21:39:56 -0700	[thread overview]
Message-ID: <20101008043956.GA25662@google.com> (raw)
In-Reply-To: <AANLkTimdACZ9Xm01DM2+E64+T5XfLffrkFBhf7CJ286p@mail.gmail.com>

On Tue, Oct 05, 2010 at 03:44:22PM -0700, Michel Lespinasse wrote:
> On Tue, Oct 5, 2010 at 10:38 AM, Rik van Riel <riel@redhat.com> wrote:
> > Looks like it should be relatively easy to do something
> > similar in do_swap_page also.
> 
> Good idea. We don't make use of swap too much, which is probably why
> we didn't have that in our kernel, but it seems like a good idea just
> for uniformity. I'll add this in a follow-on patch.

So here's the patch. Sorry for the delay - it did not take long to write,
but I couldn't test it before today.

Please have a look - I'd like to add this to the series I sent earlier.

----------------------------------- 8< ---------------------------------

Retry page fault when blocking on swap in

This change is the cousin of 'Retry page fault when blocking
on disk transfer'. The idea here is to reduce mmap_sem hold times
that are caused by disk transfers when swapping in pages. We drop
mmap_sem while waiting for the page lock, and return the VM_FAULT_RETRY
flag. do_page_fault will then re-acquire mmap_sem and retry the
page fault. It is expected that upon retry the page will now be cached,
and thus the retry will complete with a low mmap_sem hold time.

Signed-off-by: Michel Lespinasse <walken@google.com>

diff --git a/mm/memory.c b/mm/memory.c
index b068c68..0ec70b4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2613,6 +2613,21 @@ int vmtruncate_range(struct inode *inode, loff_t offset, loff_t end)
 	return 0;
 }
 
+static inline int lock_page_or_retry(struct page *page, struct mm_struct *mm,
+				     unsigned int flags)
+{
+	if (trylock_page(page))
+		return 1;
+	if (!(flags & FAULT_FLAG_ALLOW_RETRY)) {
+		__lock_page(page);
+		return 1;
+	}
+
+	up_read(&mm->mmap_sem);
+	wait_on_page_locked(page);
+	return 0;
+}
+
 /*
  * We enter with non-exclusive mmap_sem (to exclude vma changes,
  * but allow concurrent faults), and pte mapped but not yet locked.
@@ -2626,6 +2641,7 @@ static int do_swap_page(struct mm_struct *mm, struct vm_area_struct *vma,
 	struct page *page, *swapcache = NULL;
 	swp_entry_t entry;
 	pte_t pte;
+	int locked;
 	struct mem_cgroup *ptr = NULL;
 	int exclusive = 0;
 	int ret = 0;
@@ -2676,8 +2692,12 @@ static int do_swap_page(struct mm_struct *mm, struct vm_area_struct *vma,
 		goto out_release;
 	}
 
-	lock_page(page);
+	locked = lock_page_or_retry(page, mm, flags);
 	delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
+	if (!locked) {
+		ret |= VM_FAULT_RETRY;
+		goto out_release;
+	}
 
 	/*
 	 * Make sure try_to_free_swap or reuse_swap_page or swapoff did not


-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2010-10-08  4:40 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-05  7:53 [PATCH 0/3] V2: Reduce mmap_sem hold times during file backed page faults Michel Lespinasse
2010-10-05  7:53 ` Michel Lespinasse
2010-10-05  7:53 ` [PATCH 1/3] filemap_fault: unique path for locking page Michel Lespinasse
2010-10-05  7:53   ` Michel Lespinasse
2010-10-05 17:07   ` Rik van Riel
2010-10-05 17:07     ` Rik van Riel
2010-10-05  7:53 ` [PATCH 2/3] Retry page fault when blocking on disk transfer Michel Lespinasse
2010-10-05  7:53   ` Michel Lespinasse
2010-10-05 17:33   ` Linus Torvalds
2010-10-05 17:33     ` Linus Torvalds
2010-10-05 17:38   ` Rik van Riel
2010-10-05 17:38     ` Rik van Riel
2010-10-05 22:44     ` Michel Lespinasse
2010-10-05 22:44       ` Michel Lespinasse
2010-10-08  4:39       ` Michel Lespinasse [this message]
2010-10-08  4:39         ` Michel Lespinasse
2010-10-08 13:24         ` Rik van Riel
2010-10-08 13:24           ` Rik van Riel
2010-10-08 20:06           ` Michel Lespinasse
2010-10-08 20:06             ` Michel Lespinasse
2010-10-09  1:22             ` Michel Lespinasse
2010-10-09  1:22               ` Michel Lespinasse
2010-10-11 22:25               ` Andrew Morton
2010-10-11 22:25                 ` Andrew Morton
2010-10-11 22:43                 ` Michel Lespinasse
2010-10-11 22:43                   ` Michel Lespinasse
2010-10-13 23:17               ` Andrew Morton
2010-10-13 23:17                 ` Andrew Morton
2010-11-02 20:05               ` Michel Lespinasse
2010-11-02 20:11                 ` Rik van Riel
2010-10-06  4:02   ` H. Peter Anvin
2010-10-06  4:02     ` H. Peter Anvin
2010-10-05  7:53 ` [PATCH 3/3] access_error API cleanup Michel Lespinasse
2010-10-05  7:53   ` Michel Lespinasse
2010-10-05 19:44   ` Rik van Riel
2010-10-05 19:44     ` Rik van Riel
2010-10-06  4:02   ` H. Peter Anvin
2010-10-06  4:02     ` H. Peter Anvin
2010-10-06  4:14     ` Michel Lespinasse
2010-10-06  4:14       ` Michel Lespinasse
2010-10-06  4:18       ` Andrew Morton
2010-10-06  4:18         ` Andrew Morton
2010-10-06  4:20         ` H. Peter Anvin
2010-10-06  4:20           ` H. Peter Anvin
2010-10-05 14:55 ` [PATCH 0/3] V2: Reduce mmap_sem hold times during file backed page faults Rik van Riel
2010-10-05 14:55   ` Rik van Riel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101008043956.GA25662@google.com \
    --to=walken@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=npiggin@kernel.dk \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=yinghan@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.