linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Andres Lagar-Cavilla <andreslc@google.com>
Cc: Gleb Natapov <gleb@redhat.com>, Rik van Riel <riel@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Mel Gorman <mgorman@suse.de>,
	Andy Lutomirski <luto@amacapital.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Sasha Levin <sasha.levin@oracle.com>,
	Jianyu Zhan <nasa4836@gmail.com>,
	Paul Cassella <cassella@cray.com>,
	Hugh Dickins <hughd@google.com>,
	Peter Feiner <pfeiner@google.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH] kvm: Faults which trigger IO release the mmap_sem
Date: Wed, 17 Sep 2014 09:43:46 +0200	[thread overview]
Message-ID: <54193BB2.8010500@redhat.com> (raw)
In-Reply-To: <CAJu=L58z-=_KkZXpEiPjDUup8GpH7079HH39csmvgUxGkvXy0A@mail.gmail.com>

Il 16/09/2014 20:42, Andres Lagar-Cavilla ha scritto:
> On Tue, Sep 16, 2014 at 11:29 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>     I think a first patch should introduce kvm_get_user_page_retry ("Retry a
>>>     fault after a gup with FOLL_NOWAIT.") and the second would add
>>>     FOLL_TRIED ("This properly relinquishes mmap semaphore if the
>>>     filemap/swap has to wait on page lock (and retries the gup to completion
>>>     after that").
>>>
>>> That's not what FOLL_TRIED does. The relinquishing of mmap semaphore is
>>> done by this patch minus the FOLL_TRIED bits. FOLL_TRIED will let the
>>> fault handler (e.g. filemap) know that we've been there and waited on
>>> the IO already, so in the common case we won't need to redo the IO.
>>
>> Yes, that's not what FOLL_TRIED does.  But it's the difference between
>> get_user_pages and kvm_get_user_page_retry, right?
> 
> Unfortunately get_user_pages does not expose the param (int
> *nonblocking) that __gup will use to set FAULT_FLAG_ALLOW_RETRY. So
> that's one difference. The second difference is that kvm_gup_retry
> will call two times if necessary (the second without _RETRY but with
> _TRIED).

Yeah, that's how it is in your patch.  I can see that.

What I'm saying is that your patch is two changes in one:

1) do not use gup_fast in hva_to_pfn_slow, instead use gup as in
async_pf_execute.  This change can already introduce a function called
kvm_get_user_page_retry, and can already use it in async_pf_execute and
hva_to_pfn_slow

2) introduce the two-phase RETRY + TRIED mechanism in
kvm_get_user_page_retry, so that the mmap semaphore is relinquished
properly if the filemap or swap has to wait on the page lock.

I would prefer to split it in two patches.  Is it clearer now?

Paolo

--
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:[~2014-09-17  7:43 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-15 20:11 [PATCH] kvm: Faults which trigger IO release the mmap_sem Andres Lagar-Cavilla
2014-09-16 13:51 ` Paolo Bonzini
2014-09-16 16:52   ` Andres Lagar-Cavilla
2014-09-16 16:55     ` Andres Lagar-Cavilla
2014-09-16 18:29     ` Paolo Bonzini
2014-09-16 18:42       ` Andres Lagar-Cavilla
2014-09-17  7:43         ` Paolo Bonzini [this message]
2014-09-17 16:58           ` Andres Lagar-Cavilla
2014-09-17 20:01             ` Paolo Bonzini
2014-09-16 20:51   ` Radim Krčmář
2014-09-16 21:01     ` Andres Lagar-Cavilla
2014-09-16 22:34       ` Radim Krčmář
2014-09-17  4:15         ` Andres Lagar-Cavilla
2014-09-17 11:35       ` Radim Krčmář
2014-09-17 10:26 ` Gleb Natapov
2014-09-17 11:27   ` Radim Krčmář
2014-09-17 11:42     ` Gleb Natapov
2014-09-17 17:00       ` Andres Lagar-Cavilla
2014-09-17 17:08         ` Gleb Natapov
2014-09-17 17:13           ` Andres Lagar-Cavilla
2014-09-17 17:21             ` Gleb Natapov
2014-09-17 17:41               ` Andres Lagar-Cavilla
2014-09-17 17:51 ` [PATCH v2] " Andres Lagar-Cavilla
2014-09-18  0:29   ` Wanpeng Li
2014-09-18  6:13     ` Gleb Natapov
2014-09-19  0:32       ` Wanpeng Li
2014-09-19  3:58         ` Andres Lagar-Cavilla
2014-09-19  6:08           ` Paolo Bonzini
2014-09-22 20:49             ` Andres Lagar-Cavilla
2014-09-22 21:32               ` Paolo Bonzini
2014-09-22 21:53                 ` Andrew Morton
2014-09-18  6:15   ` Gleb Natapov
2014-09-25 21:16   ` Andrea Arcangeli
2014-09-25 21:50     ` Andres Lagar-Cavilla

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=54193BB2.8010500@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreslc@google.com \
    --cc=cassella@cray.com \
    --cc=gleb@redhat.com \
    --cc=hughd@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mgorman@suse.de \
    --cc=nasa4836@gmail.com \
    --cc=peterz@infradead.org \
    --cc=pfeiner@google.com \
    --cc=riel@redhat.com \
    --cc=sasha.levin@oracle.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).