All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gleb Natapov <gleb@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: avi@redhat.com, mtosatti@redhat.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH 1/2] Allow GUP to fail instead of waiting on a page.
Date: Wed, 2 Feb 2011 15:31:57 +0200	[thread overview]
Message-ID: <20110202133157.GI14984@redhat.com> (raw)
In-Reply-To: <20110201164240.9a5c06e9.akpm@linux-foundation.org>

On Tue, Feb 01, 2011 at 04:42:40PM -0800, Andrew Morton wrote:
> On Tue,  1 Feb 2011 13:21:46 +0200
> Gleb Natapov <gleb@redhat.com> wrote:
> 
> > GUP user may want to try to acquire a reference to a page if it is already
> > in memory, but not if IO, to bring it in, is needed. For example KVM may
> > tell vcpu to schedule another guest process if current one is trying to
> > access swapped out page. Meanwhile, the page will be swapped in and the
> > guest process, that depends on it, will be able to run again.
> > 
> > This patch adds FAULT_FLAG_RETRY_NOWAIT (suggested by Linus) and
> > FOLL_NOWAIT follow_page flags. FAULT_FLAG_RETRY_NOWAIT, when used in
> > conjunction with VM_FAULT_ALLOW_RETRY, indicates to handle_mm_fault that
> > it shouldn't drop mmap_sem and wait on a page, but return VM_FAULT_RETRY
> > instead.
> >
> > ...
> >
> > +#define FOLL_NOWAIT	0x20	/* return if disk transfer is needed */
> 
> The comment is a little misleading.  Or incomplete.
> 
> For both swap-backed and file-backed pages, the code will initiate the
> disk transfer and will then return without waiting for it to complete. 
> This (important!) information isn't really presented in either the
> changelog or the code itself.
> 
> This?
> 
Yes, this is better. Thanks you. I see that the patch below is in your queue
already. Should I re-spin my patch with improved comment anyway?

> --- a/include/linux/mm.h~mm-allow-gup-to-fail-instead-of-waiting-on-a-page-fix
> +++ a/include/linux/mm.h
> @@ -1537,7 +1537,8 @@ struct page *follow_page(struct vm_area_
>  #define FOLL_GET	0x04	/* do get_page on page */
>  #define FOLL_DUMP	0x08	/* give error on hole if it would be zero */
>  #define FOLL_FORCE	0x10	/* get_user_pages read/write w/o permission */
> -#define FOLL_NOWAIT	0x20	/* return if disk transfer is needed */
> +#define FOLL_NOWAIT	0x20	/* if a disk transfer is needed, start the IO
> +				 * and return without waiting upon it */
>  #define FOLL_MLOCK	0x40	/* mark page as mlocked */
>  #define FOLL_SPLIT	0x80	/* don't return transhuge pages, split them */
>  
> _

--
			Gleb.

WARNING: multiple messages have this Message-ID (diff)
From: Gleb Natapov <gleb@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: avi@redhat.com, mtosatti@redhat.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Rik van Riel <riel@redhat.com>, Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH 1/2] Allow GUP to fail instead of waiting on a page.
Date: Wed, 2 Feb 2011 15:31:57 +0200	[thread overview]
Message-ID: <20110202133157.GI14984@redhat.com> (raw)
In-Reply-To: <20110201164240.9a5c06e9.akpm@linux-foundation.org>

On Tue, Feb 01, 2011 at 04:42:40PM -0800, Andrew Morton wrote:
> On Tue,  1 Feb 2011 13:21:46 +0200
> Gleb Natapov <gleb@redhat.com> wrote:
> 
> > GUP user may want to try to acquire a reference to a page if it is already
> > in memory, but not if IO, to bring it in, is needed. For example KVM may
> > tell vcpu to schedule another guest process if current one is trying to
> > access swapped out page. Meanwhile, the page will be swapped in and the
> > guest process, that depends on it, will be able to run again.
> > 
> > This patch adds FAULT_FLAG_RETRY_NOWAIT (suggested by Linus) and
> > FOLL_NOWAIT follow_page flags. FAULT_FLAG_RETRY_NOWAIT, when used in
> > conjunction with VM_FAULT_ALLOW_RETRY, indicates to handle_mm_fault that
> > it shouldn't drop mmap_sem and wait on a page, but return VM_FAULT_RETRY
> > instead.
> >
> > ...
> >
> > +#define FOLL_NOWAIT	0x20	/* return if disk transfer is needed */
> 
> The comment is a little misleading.  Or incomplete.
> 
> For both swap-backed and file-backed pages, the code will initiate the
> disk transfer and will then return without waiting for it to complete. 
> This (important!) information isn't really presented in either the
> changelog or the code itself.
> 
> This?
> 
Yes, this is better. Thanks you. I see that the patch below is in your queue
already. Should I re-spin my patch with improved comment anyway?

> --- a/include/linux/mm.h~mm-allow-gup-to-fail-instead-of-waiting-on-a-page-fix
> +++ a/include/linux/mm.h
> @@ -1537,7 +1537,8 @@ struct page *follow_page(struct vm_area_
>  #define FOLL_GET	0x04	/* do get_page on page */
>  #define FOLL_DUMP	0x08	/* give error on hole if it would be zero */
>  #define FOLL_FORCE	0x10	/* get_user_pages read/write w/o permission */
> -#define FOLL_NOWAIT	0x20	/* return if disk transfer is needed */
> +#define FOLL_NOWAIT	0x20	/* if a disk transfer is needed, start the IO
> +				 * and return without waiting upon it */
>  #define FOLL_MLOCK	0x40	/* mark page as mlocked */
>  #define FOLL_SPLIT	0x80	/* don't return transhuge pages, split them */
>  
> _

--
			Gleb.

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-02-02 13:32 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-01 11:21 [PATCH 0/2] KVM: enable asynchronous page faults Gleb Natapov
2011-02-01 11:21 ` Gleb Natapov
2011-02-01 11:21 ` [PATCH 1/2] Allow GUP to fail instead of waiting on a page Gleb Natapov
2011-02-01 11:21   ` Gleb Natapov
2011-02-01 16:48   ` Rik van Riel
2011-02-01 16:48     ` Rik van Riel
2011-02-02  0:42   ` Andrew Morton
2011-02-02  0:42     ` Andrew Morton
2011-02-02 13:31     ` Gleb Natapov [this message]
2011-02-02 13:31       ` Gleb Natapov
2011-02-02 19:37       ` Andrew Morton
2011-02-02 19:37         ` Andrew Morton
2011-02-01 11:21 ` [PATCH 2/2] KVM: Enable async page fault processing Gleb Natapov
2011-02-01 11:21   ` Gleb Natapov
2011-02-22  8:38   ` Lai Jiangshan
2011-02-22  8:38     ` Lai Jiangshan
2011-03-24 12:22   ` Gleb Natapov
2011-03-24 12:22     ` Gleb Natapov
2011-03-24 13:23     ` Avi Kivity
2011-03-24 13:23       ` Avi Kivity

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=20110202133157.GI14984@redhat.com \
    --to=gleb@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=avi@redhat.com \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mtosatti@redhat.com \
    --cc=riel@redhat.com \
    --cc=torvalds@linux-foundation.org \
    /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.