From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Hellstrom Subject: Re: [PATCH] drm/ttm: do not check if list is empty in ttm_bo_force_list_clean, v2 Date: Fri, 30 Nov 2012 10:04:21 +0100 Message-ID: <50B87695.2090606@vmware.com> References: <50B868C1.8000703@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from smtp-outbound-1.vmware.com (smtp-outbound-1.vmware.com [208.91.2.12]) by gabe.freedesktop.org (Postfix) with ESMTP id BB8A9E5C74 for ; Fri, 30 Nov 2012 01:42:06 -0800 (PST) In-Reply-To: <50B868C1.8000703@canonical.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Maarten Lankhorst Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On 11/30/2012 09:05 AM, Maarten Lankhorst wrote: > Just use the return error from ttm_mem_evict_first instead. > > Changes since v1: > - Add warning if list is not empty, nothing else we can do here. Marten, when this function is called, all cross-device reservers have been shut out with the TTM lock or whatever similar mechanism is in place. It's a critical function that must succeed, unless there is a hardware failure to evict. As mentioned in the comments on the previous patch, ttm_bo_evict_first() may return 0 (OK) if it failed to reclaim the trylock in cleanup_refs_and_unlock(), with the assumption that another process will destroy the bo anyway (possibly at a later time which we know nothing about). The lru list needs to be empty when this function returns. This means we must either keep the while(list_empty) or perhaps better, retry instead of WARN if list_empty() is detected at the end of list. /Thomas > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/ttm/ttm_bo.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 8ab23ae..9028327 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -1323,25 +1323,25 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev, > struct ttm_bo_global *glob = bdev->glob; > int ret; > > - /* > - * Can't use standard list traversal since we're unlocking. > - */ > - > - spin_lock(&glob->lru_lock); > - while (!list_empty(&man->lru)) { > - spin_unlock(&glob->lru_lock); > + do { > ret = ttm_mem_evict_first(bdev, mem_type, false, false); > - if (ret) { > - if (allow_errors) { > - return ret; > - } else { > - pr_err("Cleanup eviction failed\n"); > - } > + if (ret && ret != -EBUSY && !allow_errors) { > + pr_err("Cleanup eviction failed with %i\n", ret); > + ret = 0; > } > + } while (!ret); > + > + if (likely(ret == -EBUSY)) { > + /* > + * lru list should be empty, verify this is the case. > + */ > spin_lock(&glob->lru_lock); > + WARN_ON(!list_empty(&man->lru)); > + if (list_empty(&man->lru) || !allow_errors) > + ret = 0; > + spin_unlock(&glob->lru_lock); > } > - spin_unlock(&glob->lru_lock); > - return 0; > + return ret; > } > > int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type)