Re: [PATCH] drm/vc4: Add the DRM_IOCTL_VC4_GEM_MADVISE ioctl

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1.1: Type: text/plain, Size: 4001 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Wed, Sep 27, 2017 at 11:49:21AM -0700, Eric Anholt wrote:
>> Boris Brezillon <boris.brezillon@free-electrons.com> writes:
>> 
>> > This ioctl will allow us to purge inactive userspace buffers when the
>> > system is running out of contiguous memory.
>> >
>> > For now, the purge logic is rather dumb in that it does not try to
>> > release only the amount of BO needed to meet the last CMA alloc request
>> > but instead purges all objects placed in the purgeable pool as soon as
>> > we experience a CMA allocation failure.
>> >
>> > Note that the in-kernel BO cache is always purged before the purgeable
>> > cache because those objects are known to be unused while objects marked
>> > as purgeable by a userspace application/library might have to be
>> > restored when they are marked back as unpurgeable, which can be
>> > expensive.
>> >
>> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>> > ---
>> > Hello,
>> >
>> > Updates to libdrm, mesa and igt making use of this kernel feature can
>> > be found on my github repos [1][2][3].
>> >
>> > There's currently no debugfs hook to manually force a purge, but this
>> > is being discussed and will probably be added in v2.
>> >
>> > Regards,
>> 
>> > diff --git a/include/uapi/drm/vc4_drm.h b/include/uapi/drm/vc4_drm.h
>> > index afae87004963..c01b93d453db 100644
>> > --- a/include/uapi/drm/vc4_drm.h
>> > +++ b/include/uapi/drm/vc4_drm.h
>> > @@ -41,6 +41,7 @@ extern "C" {
>> >  #define DRM_VC4_SET_TILING                        0x08
>> >  #define DRM_VC4_GET_TILING                        0x09
>> >  #define DRM_VC4_LABEL_BO                          0x0a
>> > +#define DRM_VC4_GEM_MADVISE                       0x0b
>> >  
>> >  #define DRM_IOCTL_VC4_SUBMIT_CL           DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_SUBMIT_CL, struct drm_vc4_submit_cl)
>> >  #define DRM_IOCTL_VC4_WAIT_SEQNO          DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_WAIT_SEQNO, struct drm_vc4_wait_seqno)
>> > @@ -53,6 +54,7 @@ extern "C" {
>> >  #define DRM_IOCTL_VC4_SET_TILING          DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_SET_TILING, struct drm_vc4_set_tiling)
>> >  #define DRM_IOCTL_VC4_GET_TILING          DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_GET_TILING, struct drm_vc4_get_tiling)
>> >  #define DRM_IOCTL_VC4_LABEL_BO            DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_LABEL_BO, struct drm_vc4_label_bo)
>> > +#define DRM_IOCTL_VC4_GEM_MADVISE         DRM_IOWR(DRM_COMMAND_BASE + DRM_VC4_GEM_MADVISE, struct drm_vc4_gem_madvise)
>> >  
>> >  struct drm_vc4_submit_rcl_surface {
>> >  	__u32 hindex; /* Handle index, or ~0 if not present. */
>> > @@ -333,6 +335,16 @@ struct drm_vc4_label_bo {
>> >  	__u64 name;
>> >  };
>> >  
>> > +#define VC4_MADV_WILLNEED			0
>> > +#define VC4_MADV_DONTNEED			1
>> > +#define __VC4_MADV_PURGED			2
>> > +
>> > +struct drm_vc4_gem_madvise {
>> > +	__u32 handle;
>> > +	__u32 madv;
>> > +	__u32 retained;
>> > +};
>> 
>> danvet had a note in
>> http://blog.ffwll.ch/2013/11/botching-up-ioctls.html:
>> 
>>     Pad the entire struct to a multiple of 64bits - the structure size
>>     will otherwise differ on 32bit versus 64bit. Which hurts when
>>     passing arrays of structures to the kernel. Or with the ioctl
>>     structure size checking that e.g. the drm core does.
>> 
>> I'm surprised that i915's ioctl didn't do this or have compat code to
>> handle it.
>
> This advise is defensive just in case you ever make an array of any of
> your uabi structs, and there's a 64 bit value in there somewhere. It only
> matters for that case really. But since gpus have a few of those ioctls
> (especially command submission) I figured better safe than sorry.

It talked about there being some core ioctl size checking -- does that
not exist any more?  I've had other people comment on size alignment of
my (non-array) ioctl structs based on your post, so some clarification
would be nice.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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