From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 18/29] drm/i915: Use new bind/unbind in eviction code Date: Tue, 6 Aug 2013 23:29:49 +0200 Message-ID: <20130806212949.GO22035@phenom.ffwll.local> References: <1375315222-4785-1-git-send-email-ben@bwidawsk.net> <1375315222-4785-19-git-send-email-ben@bwidawsk.net> <20130806183950.GC22035@phenom.ffwll.local> <20130806212738.GA2462@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f180.google.com (mail-we0-f180.google.com [74.125.82.180]) by gabe.freedesktop.org (Postfix) with ESMTP id 83635E6303 for ; Tue, 6 Aug 2013 14:29:43 -0700 (PDT) Received: by mail-we0-f180.google.com with SMTP id p61so873207wes.11 for ; Tue, 06 Aug 2013 14:29:42 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130806212738.GA2462@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky Cc: Intel GFX List-Id: intel-gfx@lists.freedesktop.org On Tue, Aug 06, 2013 at 02:27:39PM -0700, Ben Widawsky wrote: > On Tue, Aug 06, 2013 at 08:39:50PM +0200, Daniel Vetter wrote: > > On Wed, Jul 31, 2013 at 05:00:11PM -0700, Ben Widawsky wrote: > > > @@ -183,9 +186,11 @@ i915_gem_evict_everything(struct drm_device *dev) > > > i915_gem_retire_requests(dev); > > > > > > /* Having flushed everything, unbind() should never raise an error */ > > > - list_for_each_entry_safe(obj, next, &vm->inactive_list, mm_list) > > > - if (obj->pin_count == 0) > > > - WARN_ON(i915_gem_object_ggtt_unbind(obj)); > > > + list_for_each_entry(vm, &dev_priv->vm_list, global_link) { > > > + list_for_each_entry_safe(obj, next, &vm->inactive_list, mm_list) > > > + if (obj->pin_count == 0) > > > + WARN_ON(i915_vma_unbind(i915_gem_obj_to_vma(obj, vm))); > > > + } > > > > The conversion of evict_everything looks a bit strange. Essentially we > > have tree callers: > > - ums+gem support code in leavevt to rid the gtt of all gem objects when > > the userspace X ums ddx stops controlling the hw. > > - When we seriously ran out of memory in, shrink_all. > > - In execbuf when we've fragmented the gtt address space so badly that we > > need to start over completely fresh. > > > > With this it imo would make sense to just loop over the global bound > > object lists. But for the execbuf caller adding a vm parameter (and only > > evicting from that special vm, skipping all others) would make sense. > > Other callers would pass NULL since they want everything to get evicted. > > Volunteered for that follow-up? > > > > We need evict_everything for #1. For #2, we call evict_something already > for the vm when we go through the out of space path. If that failed, > evicting everything for a specific VM is just the same operation. But > maybe I've glossed over something in the details. Please correct if I'm > wrong. Is there a case where evict something might fail with ENOSPC, and > evict everything in a VM would help? Yes, when we've terminally fragmented the gtt we kick out everything and start over. That's the 3rd usecase. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch