From patchwork Mon Jul 3 10:31:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 9822467 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9688260246 for ; Mon, 3 Jul 2017 10:31:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 87A2F27F3E for ; Mon, 3 Jul 2017 10:31:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7A09F2811A; Mon, 3 Jul 2017 10:31:34 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id AF91227F3E for ; Mon, 3 Jul 2017 10:31:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 600CF6E0DB; Mon, 3 Jul 2017 10:31:32 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 24EF36E0DA for ; Mon, 3 Jul 2017 10:31:30 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 7572248-1500050 for multiple; Mon, 03 Jul 2017 11:31:20 +0100 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Mon, 03 Jul 2017 11:31:19 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Mon, 3 Jul 2017 11:31:07 +0100 Message-Id: <20170703103109.12891-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.13.2 X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Subject: [Intel-gfx] [PATCH 1/3] drm/i915: Refactor testing obj->mm.pages X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Since we occasionally stuff an error pointer into obj->mm.pages for a semi-permanent or even permanent failure, we have to be more careful and not just test against NULL when deciding if the object has a complete set of its concurrent pages. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h | 10 ++++++++-- drivers/gpu/drm/i915/i915_gem.c | 21 +++++++++++---------- drivers/gpu/drm/i915/i915_gem_clflush.c | 1 + drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +- drivers/gpu/drm/i915/i915_gem_shrinker.c | 10 +++++----- drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +- drivers/gpu/drm/i915/i915_gem_userptr.c | 2 +- 7 files changed, 28 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b08b56a2e680..31bd06700a0f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3341,10 +3341,16 @@ i915_gem_object_pin_pages(struct drm_i915_gem_object *obj) return __i915_gem_object_get_pages(obj); } +static inline bool +i915_gem_object_has_pages(struct drm_i915_gem_object *obj) +{ + return !IS_ERR_OR_NULL(READ_ONCE(obj->mm.pages)); +} + static inline void __i915_gem_object_pin_pages(struct drm_i915_gem_object *obj) { - GEM_BUG_ON(!obj->mm.pages); + GEM_BUG_ON(!i915_gem_object_has_pages(obj)); atomic_inc(&obj->mm.pages_pin_count); } @@ -3358,8 +3364,8 @@ i915_gem_object_has_pinned_pages(struct drm_i915_gem_object *obj) static inline void __i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj) { + GEM_BUG_ON(!i915_gem_object_has_pages(obj)); GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj)); - GEM_BUG_ON(!obj->mm.pages); atomic_dec(&obj->mm.pages_pin_count); } diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 5318e321ce50..7bc4e65b534f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -584,7 +584,7 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, return ret; __i915_gem_object_put_pages(obj, I915_MM_NORMAL); - if (obj->mm.pages) + if (i915_gem_object_has_pages(obj)) return -EBUSY; GEM_BUG_ON(obj->ops != &i915_gem_object_ops); @@ -2204,7 +2204,7 @@ void __i915_gem_object_invalidate(struct drm_i915_gem_object *obj) struct address_space *mapping; lockdep_assert_held(&obj->mm.lock); - GEM_BUG_ON(obj->mm.pages); + GEM_BUG_ON(i915_gem_object_has_pages(obj)); switch (obj->mm.madv) { case I915_MADV_DONTNEED: @@ -2267,7 +2267,7 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj, return; GEM_BUG_ON(obj->bind_count); - if (!READ_ONCE(obj->mm.pages)) + if (!i915_gem_object_has_pages(obj)) return; /* May be called by shrinker from within get_pages() (on another bo) */ @@ -2545,7 +2545,7 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj) if (err) return err; - if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) { + if (unlikely(!i915_gem_object_has_pages(obj))) { err = ____i915_gem_object_get_pages(obj); if (err) goto unlock; @@ -2623,7 +2623,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, pinned = true; if (!atomic_inc_not_zero(&obj->mm.pages_pin_count)) { - if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) { + if (unlikely(!i915_gem_object_has_pages(obj))) { ret = ____i915_gem_object_get_pages(obj); if (ret) goto err_unlock; @@ -2633,7 +2633,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj, atomic_inc(&obj->mm.pages_pin_count); pinned = false; } - GEM_BUG_ON(!obj->mm.pages); + GEM_BUG_ON(!i915_gem_object_has_pages(obj)); ptr = page_unpack_bits(obj->mm.mapping, &has_type); if (ptr && has_type != type) { @@ -2688,7 +2688,7 @@ i915_gem_object_pwrite_gtt(struct drm_i915_gem_object *obj, * allows it to avoid the cost of retrieving a page (either swapin * or clearing-before-use) before it is overwritten. */ - if (READ_ONCE(obj->mm.pages)) + if (i915_gem_object_has_pages(obj)) return -ENODEV; /* Before the pages are instantiated the object is treated as being @@ -4215,7 +4215,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, if (err) goto out; - if (obj->mm.pages && + if (i915_gem_object_has_pages(obj) && i915_gem_object_is_tiled(obj) && dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) { if (obj->mm.madv == I915_MADV_WILLNEED) { @@ -4234,7 +4234,8 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, obj->mm.madv = args->madv; /* if the object is no longer attached, discard its backing storage */ - if (obj->mm.madv == I915_MADV_DONTNEED && !obj->mm.pages) + if (obj->mm.madv == I915_MADV_DONTNEED && + !i915_gem_object_has_pages(obj)) i915_gem_object_truncate(obj); args->retained = obj->mm.madv != __I915_MADV_PURGED; @@ -4425,7 +4426,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, if (WARN_ON(i915_gem_object_has_pinned_pages(obj))) atomic_set(&obj->mm.pages_pin_count, 0); __i915_gem_object_put_pages(obj, I915_MM_NORMAL); - GEM_BUG_ON(obj->mm.pages); + GEM_BUG_ON(i915_gem_object_has_pages(obj)); if (obj->base.import_attach) drm_prime_gem_destroy(&obj->base, NULL); diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c index 152f16c11878..3c40a03b950b 100644 --- a/drivers/gpu/drm/i915/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c @@ -70,6 +70,7 @@ static const struct dma_fence_ops i915_clflush_ops = { static void __i915_do_clflush(struct drm_i915_gem_object *obj) { + GEM_BUG_ON(!i915_gem_object_has_pages(obj)); drm_clflush_sg(obj->mm.pages); intel_fb_obj_flush(obj, ORIGIN_CPU); } diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c index 7032c542a9b1..241d827b85fb 100644 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c @@ -229,7 +229,7 @@ int i915_gem_render_state_emit(struct drm_i915_gem_request *req) return 0; /* Recreate the page after shrinking */ - if (!so->vma->obj->mm.pages) + if (!i915_gem_object_has_pages(so->vma->obj)) so->batch_offset = -1; ret = i915_vma_pin(so->vma, 0, 0, PIN_GLOBAL | PIN_HIGH); diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index 1032f98add11..1668a62619fb 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -92,7 +92,7 @@ static bool swap_available(void) static bool can_release_pages(struct drm_i915_gem_object *obj) { - if (!obj->mm.pages) + if (!i915_gem_object_has_pages(obj)) return false; /* Consider only shrinkable ojects. */ @@ -124,7 +124,7 @@ static bool unsafe_drop_pages(struct drm_i915_gem_object *obj) { if (i915_gem_object_unbind(obj) == 0) __i915_gem_object_put_pages(obj, I915_MM_SHRINKER); - return !READ_ONCE(obj->mm.pages); + return !i915_gem_object_has_pages(obj); } /** @@ -238,7 +238,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, /* May arrive from get_pages on another bo */ mutex_lock_nested(&obj->mm.lock, I915_MM_SHRINKER); - if (!obj->mm.pages) { + if (!i915_gem_object_has_pages(obj)) { __i915_gem_object_invalidate(obj); list_del_init(&obj->global_link); count += obj->base.size >> PAGE_SHIFT; @@ -396,7 +396,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr) */ unbound = bound = unevictable = 0; list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) { - if (!obj->mm.pages) + if (!i915_gem_object_has_pages(obj)) continue; if (!can_release_pages(obj)) @@ -405,7 +405,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr) unbound += obj->base.size >> PAGE_SHIFT; } list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) { - if (!obj->mm.pages) + if (!i915_gem_object_has_pages(obj)) continue; if (!can_release_pages(obj)) diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index fb5231f98c0d..1294cf695df0 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -269,7 +269,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj, * due to the change in swizzling. */ mutex_lock(&obj->mm.lock); - if (obj->mm.pages && + if (i915_gem_object_has_pages(obj) && obj->mm.madv == I915_MADV_WILLNEED && i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) { if (tiling == I915_TILING_NONE) { diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c index ccd09e8419f5..f5d4f9875e32 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -82,7 +82,7 @@ static void cancel_userptr(struct work_struct *work) /* We are inside a kthread context and can't be interrupted */ if (i915_gem_object_unbind(obj) == 0) __i915_gem_object_put_pages(obj, I915_MM_NORMAL); - WARN_ONCE(obj->mm.pages, + WARN_ONCE(i915_gem_object_has_pages(obj), "Failed to release pages: bind_count=%d, pages_pin_count=%d, pin_display=%d\n", obj->bind_count, atomic_read(&obj->mm.pages_pin_count),