From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH] drm/vc4: Add the DRM_IOCTL_VC4_GEM_MADVISE ioctl Date: Thu, 28 Sep 2017 10:24:33 -0700 Message-ID: <87y3oyg2i6.fsf@anholt.net> References: <20170927141054.32728-1-boris.brezillon@free-electrons.com> <8760c4dlji.fsf@anholt.net> <20170928071746.6jgpddeplpgspbm6@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1450777961==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id 51C486E9B3 for ; Thu, 28 Sep 2017 17:24:36 +0000 (UTC) In-Reply-To: <20170928071746.6jgpddeplpgspbm6@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Boris Brezillon , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1450777961== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Daniel Vetter writes: > On Wed, Sep 27, 2017 at 11:49:21AM -0700, Eric Anholt wrote: >> Boris Brezillon writes: >>=20 >> > 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 >> > --- >> > 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, >>=20 >> > 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 >> >=20=20 >> > #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) >> >=20=20 >> > 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; >> > }; >> >=20=20 >> > +#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; >> > +}; >>=20 >> danvet had a note in >> http://blog.ffwll.ch/2013/11/botching-up-ioctls.html: >>=20 >> 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. >>=20 >> 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. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAlnNMFEACgkQtdYpNtH8 nuiQPw//RlQKhV1hAsCJVxMu3GyyhBRkuUVFvpHU2HN560YA7ta46cQ1yeeOMmY9 y28glsw3gEOcBon6JZTXU5OsBPIw8ViJW4e1vR4riRtATK2YUqNKuhL3sLrqVRZP 5Jex2l+wvLef/rTKwBoEZGSEAuyzcb1qKEdgKfKRVs33Sa0g/WyL7jSc7QWe+p7s 2ID9Vpw/IS1DywyNPslrHkgwYT4ZzyWPWChUQGldnQLmIdKiYs41DnoO+RwfTkVc Am5pF5T45H3f8npWu744BB8Qeoiz6v6fGrws0b9mliDJMdS5dzKSgCYlqdMRtzC3 MwMqJ5pHd/c6q8id4Hq6MCz4hYC91Lk0EZxKosgRCBYedGC9nAFHhI/xOt58CzNc 6MZNoiWwEf4oEtGW9xe46JOyz/8W69/S7wq2gxdYUM3lOcmsc8HG2Ar98hqhXyOc N+ExUAWEjrDzJUkcvqFNtPlK02U+mthJGOVNWWyrwrD4UOQ5BbseJoZuLUhGbP9g f1MtR20cvWoJYhzVbfa+trWOpglVQwxUkOzA/2dZo/Ue6K/xZfhG1Fq/99YY/Spy tVeZ/eIAWyrJRZRkjOFexmdblfUXnpbuev8lZy5zvqxycxFO3KXJdup89LZWjkhF 3lPAZ5QGv1PEAa3K1dP3ztdJO5kVaFX2T3VCpjSq0prWEjYJix8= =We32 -----END PGP SIGNATURE----- --=-=-=-- --===============1450777961== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1450777961==--