[PATCH] drm/i915: VM eviction only targets address space not physical pages

[PATCH] drm/i915: VM eviction only targets address space not physical pages

[Chris Wilson]

During eviction, we are only considering how to free up space within the
current address space and not concerned with freeing up physical memory.
As such we need only skip nodes that pinned in the current VM and not
globally.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_evict.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 525b242ac512..a745966b7e16 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -36,8 +36,7 @@
 static bool
 mark_free(struct i915_vma *vma, struct list_head *unwind)
 {
-	/* Freeing up memory requires no VMAs are pinned */
-	if (i915_gem_obj_is_pinned(vma->obj))
+	if (vma->pin_count)
 		return false;
 
 	if (WARN_ON(!list_empty(&vma->exec_list)))
-- 
1.8.5.3

Re: [PATCH] drm/i915: VM eviction only targets address space not physical pages

[Ben Widawsky]

On Tue, Jan 28, 2014 at 06:08:38PM +0000, Chris Wilson wrote:
> During eviction, we are only considering how to free up space within the
> current address space and not concerned with freeing up physical memory.
> As such we need only skip nodes that pinned in the current VM and not
> globally.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>

Can you rename i915_gem_evict_something() or add a comment at the top
that the function is not used by the shrinker, and is for evicting a VM?

The comment we currently have in the function doesn't suggest this at
all.

Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  drivers/gpu/drm/i915/i915_gem_evict.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index 525b242ac512..a745966b7e16 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -36,8 +36,7 @@
>  static bool
>  mark_free(struct i915_vma *vma, struct list_head *unwind)
>  {
> -	/* Freeing up memory requires no VMAs are pinned */
> -	if (i915_gem_obj_is_pinned(vma->obj))
> +	if (vma->pin_count)
>  		return false;
>  
>  	if (WARN_ON(!list_empty(&vma->exec_list)))
> -- 
> 1.8.5.3
> 

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH] drm/i915: VM eviction only targets address space not physical pages

[Daniel Vetter]

On Wed, Jan 29, 2014 at 12:06:36PM -0800, Ben Widawsky wrote:
> On Tue, Jan 28, 2014 at 06:08:38PM +0000, Chris Wilson wrote:
> > During eviction, we are only considering how to free up space within the
> > current address space and not concerned with freeing up physical memory.
> > As such we need only skip nodes that pinned in the current VM and not
> > globally.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> 
> Can you rename i915_gem_evict_something() or add a comment at the top
> that the function is not used by the shrinker, and is for evicting a VM?
> 
> The comment we currently have in the function doesn't suggest this at
> all.

I'll do a general kerneldoc cleanup for i915_gem_evict.c as a separate
patch.

> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Queued for -next, thanks for the patch.
-Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_evict.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> > index 525b242ac512..a745966b7e16 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> > @@ -36,8 +36,7 @@
> >  static bool
> >  mark_free(struct i915_vma *vma, struct list_head *unwind)
> >  {
> > -	/* Freeing up memory requires no VMAs are pinned */
> > -	if (i915_gem_obj_is_pinned(vma->obj))
> > +	if (vma->pin_count)
> >  		return false;
> >  
> >  	if (WARN_ON(!list_empty(&vma->exec_list)))
> > -- 
> > 1.8.5.3
> > 
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: VM eviction only targets address space not physical pages

[Chris Wilson]

On Wed, Jan 29, 2014 at 12:06:36PM -0800, Ben Widawsky wrote:
> On Tue, Jan 28, 2014 at 06:08:38PM +0000, Chris Wilson wrote:
> > During eviction, we are only considering how to free up space within the
> > current address space and not concerned with freeing up physical memory.
> > As such we need only skip nodes that pinned in the current VM and not
> > globally.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> 
> Can you rename i915_gem_evict_something() or add a comment at the top
> that the function is not used by the shrinker, and is for evicting a VM?

purge is shrinker freeing up objects for physical pages
evict is eviction freeing up objects for address space

That shrink_all calls evict_everything is just a matter of convenience.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre