Re: [PATCH 2/2] drm: document DRM_IOCTL_GEM_CLOSE

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]

On Thu, 16 Feb 2023 20:48:05 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Thu, Feb 16, 2023 at 01:09:45PM +0000, Simon Ser wrote:
> > This is a bit tricky, because of the ref'counting considerations.
> > See also [1] for more discussion about this topic. Since this is
> > kernel docs, I've decided to elaborate a bit less on the user-space
> > details.
> > 
> > [1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110
> > 
> > Signed-off-by: Simon Ser <contact@emersion.fr>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: Pekka Paalanen <ppaalanen@gmail.com>
> > Cc: Daniel Stone <daniel@fooishbar.org>
> > ---
> >  include/uapi/drm/drm.h | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> > index 54b2313c8332..4829f1fa9570 100644
> > --- a/include/uapi/drm/drm.h
> > +++ b/include/uapi/drm/drm.h
> > @@ -972,6 +972,19 @@ extern "C" {
> >  #define DRM_IOCTL_GET_STATS             DRM_IOR( 0x06, struct drm_stats)
> >  #define DRM_IOCTL_SET_VERSION		DRM_IOWR(0x07, struct drm_set_version)
> >  #define DRM_IOCTL_MODESET_CTL           DRM_IOW(0x08, struct drm_modeset_ctl)
> > +/**
> > + * DRM_IOCTL_GEM_CLOSE - Close a GEM handle.
> > + *
> > + * GEM handles are not reference-counted by the kernel. User-space is
> > + * responsible for managing their lifetime. For example, if user-space imports
> > + * the same memory object twice on the same DRM file description, the same GEM
> > + * handle is returned by both imports, and user-space needs to ensure
> > + * &DRM_IOCTL_GEM_CLOSE is performed once only. The same situation can happen
> > + * when a memory object is allocated, then exported and imported again on the
> > + * same DRM file description. The &DRM_IOCTL_MODE_GETFB2 IOCTL is an exception
> > + * and always returns fresh new GEM handles even if an existing GEM handle
> > + * already refers to the same memory object before the IOCTL is performed.  
> 
> I'd duplicate the relevant parts into each ioctl doc too, just to increase
> the chances people notice these caveats. But that's stuf for these other
> patches. for this one:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> btw do the links from on ioctl text to the other work?
> -Daniel

This one

Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>

I agree with Daniel's comments to both patches, and nothing else came
to my mind.


Thanks,
pq


> 
> > + */
> >  #define DRM_IOCTL_GEM_CLOSE		DRM_IOW (0x09, struct drm_gem_close)
> >  #define DRM_IOCTL_GEM_FLINK		DRM_IOWR(0x0a, struct drm_gem_flink)
> >  #define DRM_IOCTL_GEM_OPEN		DRM_IOWR(0x0b, struct drm_gem_open)
> > -- 
> > 2.39.1
> > 
> >   
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]