Re: [PATCH 1/2] drm/shmem: Add madvise state and purge helpers

From: Rob Clark <robdclark@gmail.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Rob Clark <robdclark@chromium.org>,
	Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Steven Price <steven.price@arm.com>,
	David Airlie <airlied@linux.ie>,
	Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>,
	Sean Paul <sean@poorly.run>
Subject: Re: [PATCH 1/2] drm/shmem: Add madvise state and purge helpers
Date: Mon, 5 Aug 2019 10:47:14 -0700	[thread overview]
Message-ID: <CAF6AEGtKS4V_CHiYwdUB+USw772n7GVb_sHOoVQ2dDHRxyaU+Q@mail.gmail.com> (raw)
In-Reply-To: <20190805163526.GV7444@phenom.ffwll.local>

On Mon, Aug 5, 2019 at 9:35 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Aug 05, 2019 at 08:33:57AM -0600, Rob Herring wrote:
> > Add support to the shmem GEM helpers for tracking madvise state and
> > purging pages. This is based on the msm implementation.
> >
> > The BO provides a list_head, but the list management is handled outside
> > of the shmem helpers as there are different locking requirements.
> >
> > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Eric Anholt <eric@anholt.net>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/gpu/drm/drm_gem_shmem_helper.c | 57 ++++++++++++++++++++++++++
> >  include/drm/drm_gem_shmem_helper.h     | 15 +++++++
> >  2 files changed, 72 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 2f64667ac805..4b442576de1c 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -75,6 +75,7 @@ struct drm_gem_shmem_object *drm_gem_shmem_create(struct drm_device *dev, size_t
> >       shmem = to_drm_gem_shmem_obj(obj);
> >       mutex_init(&shmem->pages_lock);
> >       mutex_init(&shmem->vmap_lock);
>
> Maybe a bit late, but for reasons (interop with ttm, which will be more
> important once we have dynamic dma-buf) it would be real nice to use the
> reservation_obj lock for all this stuff. msm, being struct_mutex based,
> isn't a great example here. The downside is that it will be a lot harder
> to get  msm to use these then, but much better to not spread struct_mutex
> inspired locking too far.
>

but somewhere you need to protect access to list of bo's that are
available to shrink.. iirc that is the only thing we should need
struct_mutex for now in shrinker path in drm/msm.  The per-bo state is
protected by a per-bo lock.

BR,
-R
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel