mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Souptick Joarder <jrdr.linux@gmail.com>
To: l.stach@pengutronix.de, linux+etnaviv@armlinux.org.uk,
	christian.gmeiner@gmail.com, airlied@linux.ie,
	willy@infradead.org, daniel.vetter@ffwll.ch
Cc: etnaviv@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH] gpu: drm: etnaviv: Change return type to vm_fault_t
Date: Mon, 21 May 2018 22:42:41 +0530	[thread overview]
Message-ID: <20180521171241.GA18441@jordon-HP-15-Notebook-PC> (raw)

Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.

Ref- commit 1c8f422059ae ("mm: change return type to vm_fault_t")

Previously vm_insert_page() returns err which driver
mapped into VM_FAULT_* type. The new function 
vmf_insert_page() will replace this inefficiency by
returning VM_FAULT_* type.

A non-fatal signal being delivered to a task which is
in the middle of a page fault may well end up in an
infinite loop, attempting to handle the page fault and
failing forever.

On seeing NOPAGE, the fault handler believes the PTE
is in the page table, so does nothing before it returns
to arch code. It will end up returning to userspace if
the signal is non-fatal, at which point it'll go right
back into the page fault handler, and mutex_lock_interruptible()
will immediately fail.  So we've converted a sleeping lock
into the most expensive spinlock.

I don't think the graphics drivers really want to be
interrupted by any signal.  I think they want to be
interruptible by fatal signals and should use the
mutex_lock_killable / fatal_signal_pending family of
functions. So mutex_lock_interruptible() is replaced
by mutex_lock_killable()

vmf_error() is the newly introduce inline function
in 4.17-rc6.

Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.h |  3 ++-
 drivers/gpu/drm/etnaviv/etnaviv_gem.c | 31 ++++++-------------------------
 2 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index a54f0b7..f6777f0 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -28,6 +28,7 @@
 #include <linux/list.h>
 #include <linux/types.h>
 #include <linux/sizes.h>
+#include <linux/mm_types.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
@@ -62,7 +63,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		struct drm_file *file);
 
 int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma);
-int etnaviv_gem_fault(struct vm_fault *vmf);
+vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf);
 int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset);
 struct sg_table *etnaviv_gem_prime_get_sg_table(struct drm_gem_object *obj);
 void *etnaviv_gem_prime_vmap(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index fcc969f..d9b13f0 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -180,7 +180,7 @@ int etnaviv_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	return obj->ops->mmap(obj, vma);
 }
 
-int etnaviv_gem_fault(struct vm_fault *vmf)
+vm_fault_t etnaviv_gem_fault(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 	struct drm_gem_object *obj = vma->vm_private_data;
@@ -191,20 +191,19 @@ int etnaviv_gem_fault(struct vm_fault *vmf)
 
 	/*
 	 * Make sure we don't parallel update on a fault, nor move or remove
-	 * something from beneath our feet.  Note that vm_insert_page() is
+	 * something from beneath our feet.  Note that vmf_insert_page() is
 	 * specifically coded to take care of this, so we don't have to.
 	 */
-	ret = mutex_lock_interruptible(&etnaviv_obj->lock);
+	ret = mutex_lock_killable(&etnaviv_obj->lock);
 	if (ret)
-		goto out;
-
+		return VM_FAULT_NOPAGE;
 	/* make sure we have pages attached now */
 	pages = etnaviv_gem_get_pages(etnaviv_obj);
 	mutex_unlock(&etnaviv_obj->lock);
 
 	if (IS_ERR(pages)) {
 		ret = PTR_ERR(pages);
-		goto out;
+		return vmf_error(ret);
 	}
 
 	/* We don't use vmf->pgoff since that has the fake offset: */
@@ -215,25 +214,7 @@ int etnaviv_gem_fault(struct vm_fault *vmf)
 	VERB("Inserting %p pfn %lx, pa %lx", (void *)vmf->address,
 	     page_to_pfn(page), page_to_pfn(page) << PAGE_SHIFT);
 
-	ret = vm_insert_page(vma, vmf->address, page);
-
-out:
-	switch (ret) {
-	case -EAGAIN:
-	case 0:
-	case -ERESTARTSYS:
-	case -EINTR:
-	case -EBUSY:
-		/*
-		 * EBUSY is ok: this just means that another thread
-		 * already did the job.
-		 */
-		return VM_FAULT_NOPAGE;
-	case -ENOMEM:
-		return VM_FAULT_OOM;
-	default:
-		return VM_FAULT_SIGBUS;
-	}
+	return vmf_insert_page(vma, vmf->address, page);
 }
 
 int etnaviv_gem_mmap_offset(struct drm_gem_object *obj, u64 *offset)
-- 
1.9.1

             reply	other threads:[~2018-05-21 17:10 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-21 17:12 Souptick Joarder [this message]
2018-05-24  3:35 ` Souptick Joarder
2018-05-28 12:50 ` Lucas Stach
2018-05-28 15:01   ` Souptick Joarder

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=20180521171241.GA18441@jordon-HP-15-Notebook-PC \
    --to=jrdr.linux@gmail.com \
    --cc=airlied@linux.ie \
    --cc=christian.gmeiner@gmail.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=etnaviv@lists.freedesktop.org \
    --cc=l.stach@pengutronix.de \
    --cc=linux+etnaviv@armlinux.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=willy@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®