All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
To: Dominik Dingel <dingel@linux.vnet.ibm.com>
Cc: linux-s390@vger.kernel.org, linux-mm@kvack.org,
	Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Eric B Munson <emunson@akamai.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Mel Gorman <mgorman@suse.de>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"Jason J. Herne" <jjherne@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] s390/mm: allow gmap code to retry on faulting in guest memory
Date: Thu, 19 Nov 2015 09:18:08 +0100	[thread overview]
Message-ID: <20151119091808.5d84c8ba@mschwide> (raw)
In-Reply-To: <1447890598-56860-3-git-send-email-dingel@linux.vnet.ibm.com>

On Thu, 19 Nov 2015 00:49:58 +0100
Dominik Dingel <dingel@linux.vnet.ibm.com> wrote:

> The userfaultfd does need FAULT_FLAG_ALLOW_RETRY to not return
> VM_FAULT_SIGBUS.  So we improve the gmap code to handle one
> VM_FAULT_RETRY.
> 
> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
> ---
>  arch/s390/mm/pgtable.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
> index 54ef3bc..8a0025d 100644
> --- a/arch/s390/mm/pgtable.c
> +++ b/arch/s390/mm/pgtable.c
> @@ -577,15 +577,22 @@ int gmap_fault(struct gmap *gmap, unsigned long gaddr,
>  	       unsigned int fault_flags)
>  {
>  	unsigned long vmaddr;
> -	int rc;
> +	int rc, fault;
> 
> +	fault_flags |= FAULT_FLAG_ALLOW_RETRY;
> +retry:
>  	down_read(&gmap->mm->mmap_sem);
>  	vmaddr = __gmap_translate(gmap, gaddr);
>  	if (IS_ERR_VALUE(vmaddr)) {
>  		rc = vmaddr;
>  		goto out_up;
>  	}
> -	if (fixup_user_fault(current, gmap->mm, vmaddr, fault_flags)) {
> +	fault = fixup_user_fault(current, gmap->mm, vmaddr, fault_flags);
> +	if (fault & VM_FAULT_RETRY) {
> +		fault_flags &= ~FAULT_FLAG_ALLOW_RETRY;
> +		fault_flags |= FAULT_FLAG_TRIED;
> +		goto retry;
> +	} else if (fault) {
>  		rc = -EFAULT;
>  		goto out_up;
>  	}

Me thinks that you want to add the retry code into fixup_user_fault itself.
You basically have the same code around the three calls to fixup_user_fault.
Yes, it will be a common code patch but I guess that it will be acceptable
given userfaultfd as a reason.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
To: Dominik Dingel <dingel@linux.vnet.ibm.com>
Cc: linux-s390@vger.kernel.org, linux-mm@kvack.org,
	Andrew Morton <akpm@linux-foundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Rientjes <rientjes@google.com>,
	Eric B Munson <emunson@akamai.com>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	Mel Gorman <mgorman@suse.de>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"Jason J. Herne" <jjherne@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] s390/mm: allow gmap code to retry on faulting in guest memory
Date: Thu, 19 Nov 2015 09:18:08 +0100	[thread overview]
Message-ID: <20151119091808.5d84c8ba@mschwide> (raw)
In-Reply-To: <1447890598-56860-3-git-send-email-dingel@linux.vnet.ibm.com>

On Thu, 19 Nov 2015 00:49:58 +0100
Dominik Dingel <dingel@linux.vnet.ibm.com> wrote:

> The userfaultfd does need FAULT_FLAG_ALLOW_RETRY to not return
> VM_FAULT_SIGBUS.  So we improve the gmap code to handle one
> VM_FAULT_RETRY.
> 
> Signed-off-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
> ---
>  arch/s390/mm/pgtable.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
> index 54ef3bc..8a0025d 100644
> --- a/arch/s390/mm/pgtable.c
> +++ b/arch/s390/mm/pgtable.c
> @@ -577,15 +577,22 @@ int gmap_fault(struct gmap *gmap, unsigned long gaddr,
>  	       unsigned int fault_flags)
>  {
>  	unsigned long vmaddr;
> -	int rc;
> +	int rc, fault;
> 
> +	fault_flags |= FAULT_FLAG_ALLOW_RETRY;
> +retry:
>  	down_read(&gmap->mm->mmap_sem);
>  	vmaddr = __gmap_translate(gmap, gaddr);
>  	if (IS_ERR_VALUE(vmaddr)) {
>  		rc = vmaddr;
>  		goto out_up;
>  	}
> -	if (fixup_user_fault(current, gmap->mm, vmaddr, fault_flags)) {
> +	fault = fixup_user_fault(current, gmap->mm, vmaddr, fault_flags);
> +	if (fault & VM_FAULT_RETRY) {
> +		fault_flags &= ~FAULT_FLAG_ALLOW_RETRY;
> +		fault_flags |= FAULT_FLAG_TRIED;
> +		goto retry;
> +	} else if (fault) {
>  		rc = -EFAULT;
>  		goto out_up;
>  	}

Me thinks that you want to add the retry code into fixup_user_fault itself.
You basically have the same code around the three calls to fixup_user_fault.
Yes, it will be a common code patch but I guess that it will be acceptable
given userfaultfd as a reason.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


  reply	other threads:[~2015-11-19  8:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-18 23:49 [PATCH 0/2] Allow gmap fault to retry Dominik Dingel
2015-11-18 23:49 ` Dominik Dingel
2015-11-18 23:49 ` [PATCH 1/2] mm: fixup_userfault returns VM_FAULT_RETRY if asked Dominik Dingel
2015-11-18 23:49   ` Dominik Dingel
2015-11-19  0:14   ` Kirill A. Shutemov
2015-11-19  0:14     ` Kirill A. Shutemov
2015-11-18 23:49 ` [PATCH 2/2] s390/mm: allow gmap code to retry on faulting in guest memory Dominik Dingel
2015-11-18 23:49   ` Dominik Dingel
2015-11-19  8:18   ` Martin Schwidefsky [this message]
2015-11-19  8:18     ` Martin Schwidefsky
2015-11-19  8:25     ` Christian Borntraeger
2015-11-19  8:25       ` Christian Borntraeger
2015-11-19 10:50       ` Dominik Dingel
2015-11-19 10:50         ` Dominik Dingel

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=20151119091808.5d84c8ba@mschwide \
    --to=schwidefsky@de.ibm.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=borntraeger@de.ibm.com \
    --cc=dingel@linux.vnet.ibm.com \
    --cc=emunson@akamai.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jjherne@linux.vnet.ibm.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=pbonzini@redhat.com \
    --cc=rientjes@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.