[Intel-gfx] [PATCH 3/3] drm/prime: Use anon_drm_getfile() for an internal drm struct file

[Chris Wilson]

Quoting Daniel Vetter (2019-11-06 13:06:26)
> On Wed, Nov 6, 2019 at 11:45 AM Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Quoting Daniel Vetter (2019-11-06 10:21:57)
> > > On Wed, Nov 06, 2019 at 10:07:16AM +0000, Chris Wilson wrote:
> > > > Currently the drm_prime mmap fallback uses a mock struct file to provide
> > > > the file pointer into the backend mmap routine. Now that we can create
> > > > fully fledged anonymous struct file around the drm device, put it to
> > > > use.
> > > >
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > ---
> > > >  drivers/gpu/drm/drm_prime.c | 26 ++++++++------------------
> > > >  1 file changed, 8 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > > index 0814211b0f3f..5faa63713ec8 100644
> > > > --- a/drivers/gpu/drm/drm_prime.c
> > > > +++ b/drivers/gpu/drm/drm_prime.c
> > > > @@ -709,8 +709,7 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vunmap);
> > > >   */
> > > >  int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > > >  {
> > > > -     struct drm_file *priv;
> > > > -     struct file *fil;
> > > > +     struct file *file;
> > > >       int ret;
> > > >
> > > >       if (obj->funcs && obj->funcs->mmap) {
> > >
> > > obj->funcs->mmap is the new way of doing this (and hopefully finally
> > > something clean), I'd really like to retire the below hack outright.
> > >
> > > Plus I'm not sure why you need an anon inode here? If a driver needs this
> > > for unmap_mapping_range or similar I think it'd be better to try and make
> > > something work cleanly for obj->funcs->mmap.
> >
> > It's faking one currently. If the fake is not good enough, you are
> > playing whack-a-mole until you finally do create a fully fledged file.
> >
> > If you are going to the trouble of having to create a struct file to
> > provide to the fallback routines, might as well avoid stinky code :)
> 
> We're currently not faking the inode at all, we're just using the one
> that comes with the dma-buf. So distinct from the drm_device file, and
> hence unmap_mapping_range won't work (or at least doing that on the
> drm_device inode wont shoot down the ptes for redirected dma-buf
> mmaps). obj->funcs->mmap has the same issue.
> 
> But since all current users of this don't expect unmap_mapping_range
> to work correctly, it's not an real issue. If that changes then imo we
> should fix up the obj->funcs->mmap path to have the correct inode, not
> the deprecated path you're updating here. But since there's no patch 4
> in this series to start using this for i915 or someone else, I'm not
> seeing the point.

There's a bug in anon_drm_inode() in that it requires an extra:

+       /* Everyone shares a single global address space */
+       file->f_mapping = dev->anon_inode->i_mapping;

I'm up to 5 patches now, but only i915/selftests & this here fallback as
direct users.
-Chris