linux-kernel - Re: [PATCH] drm/i915: Selectively enable self-reclaim

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 27 Jan 2010 13:20:38 +0200
From:	Pekka Enberg <penberg@...helsinki.fi>
To:	Chris Wilson <chris@...is-wilson.co.uk>
CC:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Roman Jarosz <kedgedev@...il.com>,
	A Rojas <nqn1976list@...il.com>,
	"A. Boulan" <arnaud.boulan@...ertysurf.fr>, michael@...nelt.co.at,
	jcnengel@...glemail.com, rientjes@...gle.com, earny@...4u.de,
	linux-kernel@...r.kernel.org, intel-gfx@...ts.freedesktop.org,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Eric Anholt <eric@...olt.net>, stable@...nel.org
Subject: Re: [PATCH] drm/i915: Selectively enable self-reclaim

Chris Wilson kirjoitti:
> 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
> 
> Signed-off-by: Chris Wilson <chris@...is-wilson.co.uk>
> Cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> Cc: Hugh Dickins <hugh.dickins@...cali.co.uk>
> Cc: Jesse Barnes <jbarnes@...tuousgeek.org>
> Cc: Eric Anholt <eric@...olt.net>
> Cc: stable@...nel.org

Roman, can you give this patch a spin?

> ---
>  drivers/gpu/drm/drm_gem.c       |   13 -------------
>  drivers/gpu/drm/i915/i915_gem.c |   22 ++++++++++------------
>  2 files changed, 10 insertions(+), 25 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_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 28b8f03..1ef5b54 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -336,25 +336,25 @@ i915_gem_object_set_page_gfp_mask (struct drm_gem_object *obj, gfp_t gfp)
>  static int
>  i915_gem_object_get_pages_or_evict(struct drm_gem_object *obj)
>  {
> +	gfp_t gfp;
>  	int ret;
>  
> +	gfp = i915_gem_object_get_page_gfp_mask(obj);
> +	i915_gem_object_set_page_gfp_mask(obj, gfp | __GFP_NORETRY | __GFP_NOWARN);
>  	ret = i915_gem_object_get_pages(obj);
> +	i915_gem_object_set_page_gfp_mask (obj, gfp);
>  
>  	/* 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);
>  	}
>  
>  	return ret;
> @@ -2580,6 +2580,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
>  	struct drm_i915_gem_object *obj_priv = obj->driver_private;
>  	struct drm_mm_node *free_space;
>  	bool retry_alloc = false;
> +	gfp_t gfp;
>  	int ret;
>  
>  	if (obj_priv->madv != I915_MADV_WILLNEED) {
> @@ -2623,15 +2624,12 @@ 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);
> -	}
> +	gfp = i915_gem_object_get_page_gfp_mask(obj);
> +	if (! retry_alloc)
> +		i915_gem_object_set_page_gfp_mask (obj, gfp | __GFP_NORETRY | __GFP_NOWARN);
>  	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);
> -	}
> +	i915_gem_object_set_page_gfp_mask (obj, gfp);
> +
>  	if (ret) {
>  		drm_mm_put_block(obj_priv->gtt_space);
>  		obj_priv->gtt_space = NULL;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/