From: Steven Price <steven.price@arm.com> To: Boris Brezillon <boris.brezillon@collabora.com>, Rob Herring <robh+dt@kernel.org>, Tomeu Vizoso <tomeu@tomeuvizoso.net>, Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> Cc: stable@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH 6/8] drm/panfrost: Make sure imported/exported BOs are never purged Date: Fri, 29 Nov 2019 14:45:41 +0000 [thread overview] Message-ID: <4094e136-a6e3-9a0d-9b56-8c196217e4d4@arm.com> (raw) In-Reply-To: <20191129135908.2439529-7-boris.brezillon@collabora.com> On 29/11/2019 13:59, Boris Brezillon wrote: > We don't want imported/exported BOs to be purges, as those are shared s/purges/purged/ > with other processes that might still use them. We should also refuse > to export a BO if it's been marked purgeable or has already been purged. > > Fixes: 013b65101315 ("drm/panfrost: Add madvise and shrinker support") > Cc: <stable@vger.kernel.org> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 19 ++++++++++++++++- > drivers/gpu/drm/panfrost/panfrost_gem.c | 27 +++++++++++++++++++++++++ > 2 files changed, 45 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 1c67ac434e10..751df975534f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -343,6 +343,7 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > struct drm_panfrost_madvise *args = data; > struct panfrost_device *pfdev = dev->dev_private; > struct drm_gem_object *gem_obj; > + int ret; > > gem_obj = drm_gem_object_lookup(file_priv, args->handle); > if (!gem_obj) { > @@ -350,6 +351,19 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > return -ENOENT; > } > > + /* > + * We don't want to mark exported/imported BOs as purgeable: we're not > + * the only owner in that case. > + */ > + mutex_lock(&dev->object_name_lock); > + if (gem_obj->dma_buf) > + ret = -EINVAL; > + else > + ret = 0; > + > + if (ret) > + goto out_unlock_object_name; > + This might be better by setting ret = 0 at the beginning of the function. This can then be re-written as the more normal: if (gem_obj->dma_buf) { ret = -EINVAL; goto out_unlock_object_name; ] > mutex_lock(&pfdev->shrinker_lock); > args->retained = drm_gem_shmem_madvise(gem_obj, args->madv); > > @@ -364,8 +378,11 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > } > mutex_unlock(&pfdev->shrinker_lock); > > +out_unlock_object_name: > + mutex_unlock(&dev->object_name_lock); > + > drm_gem_object_put_unlocked(gem_obj); > - return 0; > + return ret; > } > > int panfrost_unstable_ioctl_check(void) > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 92a95210a899..31d6417dd21c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -99,6 +99,32 @@ void panfrost_gem_close(struct drm_gem_object *obj, struct drm_file *file_priv) > spin_unlock(&priv->mm_lock); > } > > +static struct dma_buf * > +panfrost_gem_export(struct drm_gem_object *obj, int flags) > +{ > + struct panfrost_gem_object *bo = to_panfrost_bo(obj); > + int ret; > + > + /* > + * We must make sure the BO has not been marked purgeable/purged before > + * adding the mapping. > + * Note that we don't need to protect this test with a lock because > + * &drm_gem_object_funcs.export() is called with > + * &drm_device.object_lock held, and panfrost_ioctl_madvise() takes > + * this lock before calling drm_gem_shmem_madvise() (the function that > + * modifies bo->base.madv). > + */ > + if (bo->base.madv == PANFROST_MADV_WILLNEED) > + ret = -EINVAL; > + else > + ret = 0; > + > + if (ret) > + return ERR_PTR(ret); No need for ret here: if (bo->base.madv == PANFROST_MADV_WILLNEED) return ERR_PTR(-EINVAL); Steve > + > + return drm_gem_prime_export(obj, flags); > +} > + > static int panfrost_gem_pin(struct drm_gem_object *obj) > { > if (to_panfrost_bo(obj)->is_heap) > @@ -112,6 +138,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs = { > .open = panfrost_gem_open, > .close = panfrost_gem_close, > .print_info = drm_gem_shmem_print_info, > + .export = panfrost_gem_export, > .pin = panfrost_gem_pin, > .unpin = drm_gem_shmem_unpin, > .get_sg_table = drm_gem_shmem_get_sg_table, >
WARNING: multiple messages have this Message-ID (diff)
From: Steven Price <steven.price@arm.com> To: Boris Brezillon <boris.brezillon@collabora.com>, Rob Herring <robh+dt@kernel.org>, Tomeu Vizoso <tomeu@tomeuvizoso.net>, Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> Cc: dri-devel@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [PATCH 6/8] drm/panfrost: Make sure imported/exported BOs are never purged Date: Fri, 29 Nov 2019 14:45:41 +0000 [thread overview] Message-ID: <4094e136-a6e3-9a0d-9b56-8c196217e4d4@arm.com> (raw) Message-ID: <20191129144541.TmA2Hdl8cZ2FB_4MUXIb_4zFM3a-2JUZLK-9Bi1BvZM@z> (raw) In-Reply-To: <20191129135908.2439529-7-boris.brezillon@collabora.com> On 29/11/2019 13:59, Boris Brezillon wrote: > We don't want imported/exported BOs to be purges, as those are shared s/purges/purged/ > with other processes that might still use them. We should also refuse > to export a BO if it's been marked purgeable or has already been purged. > > Fixes: 013b65101315 ("drm/panfrost: Add madvise and shrinker support") > Cc: <stable@vger.kernel.org> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 19 ++++++++++++++++- > drivers/gpu/drm/panfrost/panfrost_gem.c | 27 +++++++++++++++++++++++++ > 2 files changed, 45 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 1c67ac434e10..751df975534f 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -343,6 +343,7 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > struct drm_panfrost_madvise *args = data; > struct panfrost_device *pfdev = dev->dev_private; > struct drm_gem_object *gem_obj; > + int ret; > > gem_obj = drm_gem_object_lookup(file_priv, args->handle); > if (!gem_obj) { > @@ -350,6 +351,19 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > return -ENOENT; > } > > + /* > + * We don't want to mark exported/imported BOs as purgeable: we're not > + * the only owner in that case. > + */ > + mutex_lock(&dev->object_name_lock); > + if (gem_obj->dma_buf) > + ret = -EINVAL; > + else > + ret = 0; > + > + if (ret) > + goto out_unlock_object_name; > + This might be better by setting ret = 0 at the beginning of the function. This can then be re-written as the more normal: if (gem_obj->dma_buf) { ret = -EINVAL; goto out_unlock_object_name; ] > mutex_lock(&pfdev->shrinker_lock); > args->retained = drm_gem_shmem_madvise(gem_obj, args->madv); > > @@ -364,8 +378,11 @@ static int panfrost_ioctl_madvise(struct drm_device *dev, void *data, > } > mutex_unlock(&pfdev->shrinker_lock); > > +out_unlock_object_name: > + mutex_unlock(&dev->object_name_lock); > + > drm_gem_object_put_unlocked(gem_obj); > - return 0; > + return ret; > } > > int panfrost_unstable_ioctl_check(void) > diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c > index 92a95210a899..31d6417dd21c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_gem.c > +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c > @@ -99,6 +99,32 @@ void panfrost_gem_close(struct drm_gem_object *obj, struct drm_file *file_priv) > spin_unlock(&priv->mm_lock); > } > > +static struct dma_buf * > +panfrost_gem_export(struct drm_gem_object *obj, int flags) > +{ > + struct panfrost_gem_object *bo = to_panfrost_bo(obj); > + int ret; > + > + /* > + * We must make sure the BO has not been marked purgeable/purged before > + * adding the mapping. > + * Note that we don't need to protect this test with a lock because > + * &drm_gem_object_funcs.export() is called with > + * &drm_device.object_lock held, and panfrost_ioctl_madvise() takes > + * this lock before calling drm_gem_shmem_madvise() (the function that > + * modifies bo->base.madv). > + */ > + if (bo->base.madv == PANFROST_MADV_WILLNEED) > + ret = -EINVAL; > + else > + ret = 0; > + > + if (ret) > + return ERR_PTR(ret); No need for ret here: if (bo->base.madv == PANFROST_MADV_WILLNEED) return ERR_PTR(-EINVAL); Steve > + > + return drm_gem_prime_export(obj, flags); > +} > + > static int panfrost_gem_pin(struct drm_gem_object *obj) > { > if (to_panfrost_bo(obj)->is_heap) > @@ -112,6 +138,7 @@ static const struct drm_gem_object_funcs panfrost_gem_funcs = { > .open = panfrost_gem_open, > .close = panfrost_gem_close, > .print_info = drm_gem_shmem_print_info, > + .export = panfrost_gem_export, > .pin = panfrost_gem_pin, > .unpin = drm_gem_shmem_unpin, > .get_sg_table = drm_gem_shmem_get_sg_table, > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2019-11-29 14:45 UTC|newest] Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-29 13:59 [PATCH 0/8] panfrost: Fixes for 5.4 Boris Brezillon 2019-11-29 13:59 ` [PATCH 1/8] drm/panfrost: Make panfrost_job_run() return an ERR_PTR() instead of NULL Boris Brezillon 2019-11-29 13:59 ` Boris Brezillon 2019-11-29 14:19 ` Steven Price 2019-11-29 14:19 ` Steven Price 2019-11-29 14:31 ` Boris Brezillon 2019-11-29 14:31 ` Boris Brezillon 2019-11-29 14:38 ` Steven Price 2019-11-29 14:38 ` Steven Price 2019-11-29 19:32 ` Boris Brezillon 2019-11-29 19:32 ` Boris Brezillon 2019-11-29 13:59 ` [PATCH 2/8] drm/panfrost: Fix a race in panfrost_ioctl_madvise() Boris Brezillon 2019-11-29 13:59 ` Boris Brezillon 2019-11-29 14:24 ` Steven Price 2019-11-29 14:24 ` Steven Price 2019-11-29 14:33 ` Boris Brezillon 2019-11-29 14:33 ` Boris Brezillon 2019-11-29 14:40 ` Steven Price 2019-11-29 14:40 ` Steven Price 2019-11-29 20:07 ` Daniel Vetter 2019-11-29 20:07 ` Daniel Vetter 2019-11-29 21:45 ` Boris Brezillon 2019-11-29 21:45 ` Boris Brezillon 2019-12-05 23:08 ` Rob Herring 2019-12-05 23:08 ` Rob Herring 2019-12-06 7:53 ` Boris Brezillon 2019-12-06 7:53 ` Boris Brezillon 2019-12-06 8:08 ` Boris Brezillon 2019-12-06 8:08 ` Boris Brezillon 2019-11-29 13:59 ` [PATCH 3/8] drm/panfrost: Fix a BO leak in panfrost_ioctl_mmap_bo() Boris Brezillon 2019-11-29 13:59 ` Boris Brezillon 2019-11-29 14:26 ` Steven Price 2019-11-29 14:26 ` Steven Price 2019-11-29 13:59 ` [PATCH 4/8] drm/panfrost: Fix a race in panfrost_gem_free_object() Boris Brezillon 2019-11-29 13:59 ` Boris Brezillon 2019-11-29 14:28 ` Steven Price 2019-11-29 14:28 ` Steven Price 2019-11-29 13:59 ` [PATCH 5/8] drm/panfrost: Open/close the perfcnt BO Boris Brezillon 2019-11-29 13:59 ` Boris Brezillon 2019-11-29 14:34 ` Steven Price 2019-11-29 14:34 ` Steven Price 2019-11-29 13:59 ` [PATCH 6/8] drm/panfrost: Make sure imported/exported BOs are never purged Boris Brezillon 2019-11-29 13:59 ` Boris Brezillon 2019-11-29 14:14 ` Boris Brezillon 2019-11-29 14:14 ` Boris Brezillon 2019-11-29 14:45 ` Steven Price [this message] 2019-11-29 14:45 ` Steven Price 2019-11-29 14:52 ` Boris Brezillon 2019-11-29 14:52 ` Boris Brezillon 2019-11-29 20:12 ` Daniel Vetter 2019-11-29 20:12 ` Daniel Vetter 2019-11-29 21:09 ` Boris Brezillon 2019-11-29 21:09 ` Boris Brezillon 2019-12-02 8:52 ` Daniel Vetter 2019-12-02 8:52 ` Daniel Vetter 2019-12-02 9:50 ` Boris Brezillon 2019-12-02 9:50 ` Boris Brezillon 2019-11-29 13:59 ` [PATCH 7/8] drm/panfrost: Add the panfrost_gem_mapping concept Boris Brezillon 2019-11-29 13:59 ` Boris Brezillon 2019-11-29 15:37 ` Steven Price 2019-11-29 15:37 ` Steven Price 2019-11-29 20:14 ` Daniel Vetter 2019-11-29 20:14 ` Daniel Vetter 2019-11-29 21:36 ` Boris Brezillon 2019-11-29 21:36 ` Boris Brezillon 2019-12-02 8:55 ` Daniel Vetter 2019-12-02 8:55 ` Daniel Vetter 2019-12-02 9:13 ` Boris Brezillon 2019-12-02 9:13 ` Boris Brezillon 2019-12-02 9:44 ` Daniel Vetter 2019-12-02 9:44 ` Daniel Vetter 2019-12-04 11:41 ` Steven Price 2019-12-04 11:41 ` Steven Price 2019-11-29 13:59 ` [PATCH 8/8] drm/panfrost: Make sure the shrinker does not reclaim referenced BOs Boris Brezillon 2019-11-29 13:59 ` Boris Brezillon 2019-11-29 15:48 ` Steven Price 2019-11-29 15:48 ` Steven Price 2019-11-29 16:07 ` Boris Brezillon 2019-11-29 16:07 ` Boris Brezillon 2019-11-29 16:12 ` Steven Price 2019-11-29 16:12 ` Steven Price 2019-12-02 12:50 ` Robin Murphy 2019-12-02 12:50 ` Robin Murphy 2019-12-02 13:32 ` Boris Brezillon 2019-12-02 13:32 ` Boris Brezillon 2019-11-29 14:23 ` [PATCH 0/8] panfrost: Fixes for 5.4 Alyssa Rosenzweig 2019-12-06 17:16 ` Rob Herring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4094e136-a6e3-9a0d-9b56-8c196217e4d4@arm.com \ --to=steven.price@arm.com \ --cc=alyssa.rosenzweig@collabora.com \ --cc=boris.brezillon@collabora.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=robh+dt@kernel.org \ --cc=stable@vger.kernel.org \ --cc=tomeu@tomeuvizoso.net \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.