From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B4E87C432C0 for ; Fri, 29 Nov 2019 15:48:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9247A2075C for ; Fri, 29 Nov 2019 15:48:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726808AbfK2PsT (ORCPT ); Fri, 29 Nov 2019 10:48:19 -0500 Received: from foss.arm.com ([217.140.110.172]:49610 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726608AbfK2PsS (ORCPT ); Fri, 29 Nov 2019 10:48:18 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 12C5B30E; Fri, 29 Nov 2019 07:48:18 -0800 (PST) Received: from [10.1.194.43] (e112269-lin.cambridge.arm.com [10.1.194.43]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 308953F68E; Fri, 29 Nov 2019 07:48:17 -0800 (PST) Subject: Re: [PATCH 8/8] drm/panfrost: Make sure the shrinker does not reclaim referenced BOs To: Boris Brezillon , Rob Herring , Tomeu Vizoso , Alyssa Rosenzweig Cc: stable@vger.kernel.org, dri-devel@lists.freedesktop.org References: <20191129135908.2439529-1-boris.brezillon@collabora.com> <20191129135908.2439529-9-boris.brezillon@collabora.com> From: Steven Price Message-ID: <64a6a09a-e83a-05be-8576-79a74f971286@arm.com> Date: Fri, 29 Nov 2019 15:48:15 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: <20191129135908.2439529-9-boris.brezillon@collabora.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org On 29/11/2019 13:59, Boris Brezillon wrote: > Userspace might tag a BO purgeable while it's still referenced by GPU > jobs. We need to make sure the shrinker does not purge such BOs until > all jobs referencing it are finished. > > Fixes: 013b65101315 ("drm/panfrost: Add madvise and shrinker support") > Cc: > Signed-off-by: Boris Brezillon I'm happy with this, but I would also argue that it was previously user space's job not to mark a BO purgeable while it's in use by the GPU. This is in some ways an ABI change so we should be sure this is something that we want to support "forever" more. But if Mesa has 'always' been assuming this behaviour we might as well match. Reviewed-by: Steven Price Steve > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 1 + > drivers/gpu/drm/panfrost/panfrost_gem.h | 6 ++++++ > drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 2 ++ > drivers/gpu/drm/panfrost/panfrost_job.c | 7 ++++++- > 4 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index b406b5243b40..297c0e7304d2 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -166,6 +166,7 @@ panfrost_lookup_bos(struct drm_device *dev, > break; > } > > + atomic_inc(&bo->gpu_usecount); > job->mappings[i] = mapping; > } > > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h > index ca1bc9019600..b3517ff9630c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.h > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h > @@ -30,6 +30,12 @@ struct panfrost_gem_object { > struct mutex lock; > } mappings; > > + /* > + * Count the number of jobs referencing this BO so we don't let the > + * shrinker reclaim this object prematurely. > + */ > + atomic_t gpu_usecount; > + > bool noexec :1; > bool is_heap :1; > }; > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > index b36df326c860..288e46c40673 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c > @@ -41,6 +41,8 @@ static bool panfrost_gem_purge(struct drm_gem_object *obj) > struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj); > struct panfrost_gem_object *bo = to_panfrost_bo(obj); > > + if (atomic_read(&bo->gpu_usecount)) > + return false; > > if (!mutex_trylock(&shmem->pages_lock)) > return false; > diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c > index c85d45be3b5e..2b12aa87ff32 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_job.c > +++ b/drivers/gpu/drm/panfrost/panfrost_job.c > @@ -270,8 +270,13 @@ static void panfrost_job_cleanup(struct kref *ref) > dma_fence_put(job->render_done_fence); > > if (job->mappings) { > - for (i = 0; i < job->bo_count; i++) > + for (i = 0; i < job->bo_count; i++) { > + if (!job->mappings[i]) > + break; > + > + atomic_dec(&job->mappings[i]->obj->gpu_usecount); > panfrost_gem_mapping_put(job->mappings[i]); > + } > kvfree(job->mappings); > } > >