All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>,
	Roman Jarosz <kedgedev@gmail.com>,
	A Rojas <nqn1976list@gmail.com>,
	"A. Boulan" <arnaud.boulan@libertysurf.fr>,
	michael@reinelt.co.at, jcnengel@googlemail.com,
	rientjes@google.com, earny@net4u.de,
	linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	Chris Wilson <chris@chris-wilson.co.uk>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Hugh Dickins <hugh.dickins@tiscali.co.uk>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	Eric Anholt <eric@anholt.net>,
	stable@kernel.org
Subject: [PATCH] drm/i915: Selectively enable self-reclaim
Date: Wed, 27 Jan 2010 15:25:32 +0000	[thread overview]
Message-ID: <1264605932-8540-1-git-send-email-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <alpine.LFD.2.00.1001270327580.24253@localhost.localdomain>

Having missed the ENOMEM return via i915_gem_fault(), there are probably
other paths that I also missed. By not enabling NORETRY by default these
paths can run the shrinker and take memory from the system (but not from
our own inactive lists because our shrinker can not run whilst we hold
the struct mutex) and this may allow the system to survive a little longer
whilst our drivers consume all available memory.

References:
  OOM killer unexpectedly called with kernel 2.6.32
  http://bugzilla.kernel.org/show_bug.cgi?id=14933

v2: Pass gfp into page mapping.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Eric Anholt <eric@anholt.net>
Cc: stable@kernel.org
---
 drivers/gpu/drm/drm_gem.c           |   13 ----
 drivers/gpu/drm/i915/i915_debugfs.c |    2 +-
 drivers/gpu/drm/i915/i915_drv.h     |    2 +-
 drivers/gpu/drm/i915/i915_gem.c     |  112 +++++++++++++++++++++++------------
 4 files changed, 77 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index e9dbb48..8bf3770 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -142,19 +142,6 @@ drm_gem_object_alloc(struct drm_device *dev, size_t size)
 	if (IS_ERR(obj->filp))
 		goto free;
 
-	/* Basically we want to disable the OOM killer and handle ENOMEM
-	 * ourselves by sacrificing pages from cached buffers.
-	 * XXX shmem_file_[gs]et_gfp_mask()
-	 */
-	mapping_set_gfp_mask(obj->filp->f_path.dentry->d_inode->i_mapping,
-			     GFP_HIGHUSER |
-			     __GFP_COLD |
-			     __GFP_FS |
-			     __GFP_RECLAIMABLE |
-			     __GFP_NORETRY |
-			     __GFP_NOWARN |
-			     __GFP_NOMEMALLOC);
-
 	kref_init(&obj->refcount);
 	kref_init(&obj->handlecount);
 	obj->size = size;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9c9998c..a894ade 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -290,7 +290,7 @@ static int i915_batchbuffer_info(struct seq_file *m, void *data)
 	list_for_each_entry(obj_priv, &dev_priv->mm.active_list, list) {
 		obj = obj_priv->obj;
 		if (obj->read_domains & I915_GEM_DOMAIN_COMMAND) {
-		    ret = i915_gem_object_get_pages(obj);
+		    ret = i915_gem_object_get_pages(obj, 0);
 		    if (ret) {
 			    DRM_ERROR("Failed to get pages: %d\n", ret);
 			    spin_unlock(&dev_priv->mm.active_list_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2c16694..aaf934d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -872,7 +872,7 @@ int i915_gem_attach_phys_object(struct drm_device *dev,
 void i915_gem_detach_phys_object(struct drm_device *dev,
 				 struct drm_gem_object *obj);
 void i915_gem_free_all_phys_object(struct drm_device *dev);
-int i915_gem_object_get_pages(struct drm_gem_object *obj);
+int i915_gem_object_get_pages(struct drm_gem_object *obj, gfp_t gfpmask);
 void i915_gem_object_put_pages(struct drm_gem_object *obj);
 void i915_gem_release(struct drm_device * dev, struct drm_file *file_priv);
 void i915_gem_object_flush_write_domain(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0c67924..f74a099 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -277,7 +277,7 @@ i915_gem_shmem_pread_fast(struct drm_device *dev, struct drm_gem_object *obj,
 
 	mutex_lock(&dev->struct_mutex);
 
-	ret = i915_gem_object_get_pages(obj);
+	ret = i915_gem_object_get_pages(obj, 0);
 	if (ret != 0)
 		goto fail_unlock;
 
@@ -321,40 +321,24 @@ fail_unlock:
 	return ret;
 }
 
-static inline gfp_t
-i915_gem_object_get_page_gfp_mask (struct drm_gem_object *obj)
-{
-	return mapping_gfp_mask(obj->filp->f_path.dentry->d_inode->i_mapping);
-}
-
-static inline void
-i915_gem_object_set_page_gfp_mask (struct drm_gem_object *obj, gfp_t gfp)
-{
-	mapping_set_gfp_mask(obj->filp->f_path.dentry->d_inode->i_mapping, gfp);
-}
-
 static int
 i915_gem_object_get_pages_or_evict(struct drm_gem_object *obj)
 {
 	int ret;
 
-	ret = i915_gem_object_get_pages(obj);
+	ret = i915_gem_object_get_pages(obj, __GFP_NORETRY | __GFP_NOWARN);
 
 	/* If we've insufficient memory to map in the pages, attempt
 	 * to make some space by throwing out some old buffers.
 	 */
 	if (ret == -ENOMEM) {
 		struct drm_device *dev = obj->dev;
-		gfp_t gfp;
 
 		ret = i915_gem_evict_something(dev, obj->size);
 		if (ret)
 			return ret;
 
-		gfp = i915_gem_object_get_page_gfp_mask(obj);
-		i915_gem_object_set_page_gfp_mask(obj, gfp & ~__GFP_NORETRY);
-		ret = i915_gem_object_get_pages(obj);
-		i915_gem_object_set_page_gfp_mask (obj, gfp);
+		ret = i915_gem_object_get_pages(obj, 0);
 	}
 
 	return ret;
@@ -790,7 +774,7 @@ i915_gem_shmem_pwrite_fast(struct drm_device *dev, struct drm_gem_object *obj,
 
 	mutex_lock(&dev->struct_mutex);
 
-	ret = i915_gem_object_get_pages(obj);
+	ret = i915_gem_object_get_pages(obj, 0);
 	if (ret != 0)
 		goto fail_unlock;
 
@@ -2229,8 +2213,70 @@ i915_gem_evict_something(struct drm_device *dev, int min_size)
 	}
 }
 
+/* like read_mapping_page(), but takes a gfp mask for the allocation */
+static struct page *
+i915_gem_read_cache_page(struct address_space *mapping,
+			 pgoff_t index,
+			 gfp_t gfpmask)
+{
+	struct page *page;
+	int err;
+
+retry:
+	page = find_get_page(mapping, index);
+	if (!page) {
+		page = __page_cache_alloc(mapping_gfp_mask(mapping) | __GFP_COLD | gfpmask);
+		if (!page)
+			return ERR_PTR(-ENOMEM);
+
+		err = add_to_page_cache_lru(page, mapping, index, GFP_KERNEL);
+		if (unlikely(err)) {
+			page_cache_release(page);
+			if (err == -EEXIST)
+				goto retry;
+
+			/* Presumably ENOMEM for radix tree node */
+			return ERR_PTR(err);
+		}
+	} else {
+		if (PageUptodate(page)) {
+			mark_page_accessed(page);
+			return page;
+		}
+
+		lock_page(page);
+		if (!page->mapping) {
+			unlock_page(page);
+			page_cache_release(page);
+			goto retry;
+		}
+
+		if (PageUptodate(page)) {
+			mark_page_accessed(page);
+			unlock_page(page);
+			return page;
+		}
+	}
+
+	err = mapping->a_ops->readpage(NULL, page);
+	if (err < 0) {
+		unlock_page(page);
+		page_cache_release(page);
+		return ERR_PTR(err);
+	}
+	mark_page_accessed(page);
+	wait_on_page_locked(page);
+	if (!PageUptodate(page)) {
+		page_cache_release(page);
+		return ERR_PTR(-EIO);
+	}
+
+	return page;
+}
+
 int
-i915_gem_object_get_pages(struct drm_gem_object *obj)
+i915_gem_object_get_pages(struct drm_gem_object *obj,
+			  gfp_t gfpmask)
 {
 	struct drm_i915_gem_object *obj_priv = obj->driver_private;
 	int page_count, i;
@@ -2256,7 +2302,7 @@ i915_gem_object_get_pages(struct drm_gem_object *obj)
 	inode = obj->filp->f_path.dentry->d_inode;
 	mapping = inode->i_mapping;
 	for (i = 0; i < page_count; i++) {
-		page = read_mapping_page(mapping, i, NULL);
+		page = i915_gem_read_cache_page(mapping, i, gfpmask);
 		if (IS_ERR(page)) {
 			ret = PTR_ERR(page);
 			i915_gem_object_put_pages(obj);
@@ -2579,7 +2625,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj_priv = obj->driver_private;
 	struct drm_mm_node *free_space;
-	bool retry_alloc = false;
+	gfp_t gfpmask =  __GFP_NORETRY | __GFP_NOWARN;
 	int ret;
 
 	if (obj_priv->madv != I915_MADV_WILLNEED) {
@@ -2623,15 +2669,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
 	DRM_INFO("Binding object of size %zd at 0x%08x\n",
 		 obj->size, obj_priv->gtt_offset);
 #endif
-	if (retry_alloc) {
-		i915_gem_object_set_page_gfp_mask (obj,
-						   i915_gem_object_get_page_gfp_mask (obj) & ~__GFP_NORETRY);
-	}
-	ret = i915_gem_object_get_pages(obj);
-	if (retry_alloc) {
-		i915_gem_object_set_page_gfp_mask (obj,
-						   i915_gem_object_get_page_gfp_mask (obj) | __GFP_NORETRY);
-	}
+	ret = i915_gem_object_get_pages(obj, gfpmask);
 	if (ret) {
 		drm_mm_put_block(obj_priv->gtt_space);
 		obj_priv->gtt_space = NULL;
@@ -2641,9 +2679,9 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
 			ret = i915_gem_evict_something(dev, obj->size);
 			if (ret) {
 				/* now try to shrink everyone else */
-				if (! retry_alloc) {
-				    retry_alloc = true;
-				    goto search_free;
+				if (gfpmask) {
+					gfpmask = 0;
+					goto search_free;
 				}
 
 				return ret;
@@ -4946,7 +4984,7 @@ void i915_gem_detach_phys_object(struct drm_device *dev,
 	if (!obj_priv->phys_obj)
 		return;
 
-	ret = i915_gem_object_get_pages(obj);
+	ret = i915_gem_object_get_pages(obj, 0);
 	if (ret)
 		goto out;
 
@@ -5004,7 +5042,7 @@ i915_gem_attach_phys_object(struct drm_device *dev,
 	obj_priv->phys_obj = dev_priv->mm.phys_objs[id - 1];
 	obj_priv->phys_obj->cur_obj = obj;
 
-	ret = i915_gem_object_get_pages(obj);
+	ret = i915_gem_object_get_pages(obj, 0);
 	if (ret) {
 		DRM_ERROR("failed to get page list\n");
 		goto out;
-- 
1.6.6


  parent reply	other threads:[~2010-01-27 15:25 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-14 13:15 OOM-Killer kills too much with 2.6.32.2 Roman Jarosz
2010-01-23  0:40 ` David Rientjes
2010-01-25 22:12   ` Roman Jarosz
2010-01-25  1:48 ` KOSAKI Motohiro
2010-01-25 20:47   ` Roman Jarosz
2010-01-26  5:19     ` KOSAKI Motohiro
2010-01-26  7:51       ` A Rojas
2010-01-26  9:06       ` Roman Jarosz
2010-01-26 11:07         ` KOSAKI Motohiro
2010-01-26 12:33           ` Chris Wilson
2010-01-26 13:03             ` KOSAKI Motohiro
2010-01-26 13:18               ` Chris Wilson
2010-01-26 13:59                 ` Michael Reinelt
2010-01-26 14:07                   ` Michael Reinelt
2010-01-27  0:50                 ` KOSAKI Motohiro
2010-01-27  9:56                   ` Pekka Enberg
2010-01-27 10:55                     ` Linus Torvalds
2010-01-27 11:12                       ` Pekka Enberg
2010-01-27 11:14                       ` [PATCH] drm/i915: Selectively enable self-reclaim Chris Wilson
2010-01-27 11:20                         ` Pekka Enberg
2010-01-27 11:30                           ` Michael Reinelt
2010-01-28  3:15                           ` Michael Reinelt
2010-01-28 18:21                             ` Roman Jarosz
2010-01-27 11:50                         ` KOSAKI Motohiro
2010-01-27 12:16                         ` Linus Torvalds
2010-01-27 12:28                           ` Linus Torvalds
2010-01-27 15:25                           ` Chris Wilson [this message]
2010-01-27 16:09                             ` Linus Torvalds
2010-01-27 17:14                               ` Chris Wilson
2010-01-27 17:19                                 ` Linus Torvalds
2010-01-27 21:03                                   ` Roman Jarosz
2010-06-30  6:54                                   ` [Intel-gfx] " Dave Airlie
2010-06-30  7:05                                     ` Chris Wilson
2010-06-30  7:05                                       ` Chris Wilson
2010-06-30 23:07                                       ` [Intel-gfx] " Linus Torvalds
2010-07-01  1:24                                         ` Linus Torvalds
2010-07-01  1:55                                           ` KOSAKI Motohiro
2010-07-01 10:15                                           ` Dave Airlie
2010-07-01 11:19                                           ` Chris Wilson
2010-07-01 11:19                                             ` Chris Wilson
2010-07-01 22:34                                           ` M. Vefa Bicakci
2010-07-01 23:59                                             ` Linus Torvalds
2010-07-01 23:59                                               ` Linus Torvalds
2010-07-02  0:06                                               ` [Intel-gfx] " Dave Airlie
2010-07-02  0:49                                                 ` Dave Airlie
2010-07-02  1:28                                                   ` Linus Torvalds
2010-07-02  1:28                                                     ` Linus Torvalds
2010-07-17 18:58                                                     ` [Intel-gfx] " M. Vefa Bicakci
2010-07-17 19:15                                                       ` Linus Torvalds
2010-07-18 14:27                                                         ` M. Vefa Bicakci
2010-07-18 14:27                                                           ` M. Vefa Bicakci
2010-07-18 16:59                                                           ` Linus Torvalds
2010-01-28  6:37                                 ` Willy Tarreau
2010-01-26 13:41           ` OOM-Killer kills too much with 2.6.32.2 Roman Jarosz
2010-01-27  0:14             ` KOSAKI Motohiro
2010-01-27  9:53               ` Roman Jarosz
2010-01-26 13:57       ` Pekka Enberg

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=1264605932-8540-1-git-send-email-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=arnaud.boulan@libertysurf.fr \
    --cc=earny@net4u.de \
    --cc=eric@anholt.net \
    --cc=hugh.dickins@tiscali.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jbarnes@virtuousgeek.org \
    --cc=jcnengel@googlemail.com \
    --cc=kedgedev@gmail.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael@reinelt.co.at \
    --cc=nqn1976list@gmail.com \
    --cc=penberg@cs.helsinki.fi \
    --cc=rientjes@google.com \
    --cc=stable@kernel.org \
    --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.