[Intel-gfx] [PATCH libdrm] tests: Add drm_set_cgrp_param

[Matt Roper]

On Fri, Jan 26, 2018 at 05:08:48PM +0000, Emil Velikov wrote:
> On 22 January 2018 at 15:44, Matt Roper <matthew.d.roper at intel.com> wrote:
> > drm_set_cgrp_param is a simple tool to set DRM parameters associated with a
> > cgroup.  It is intended to be called at system initialization time (e.g., from
> > a sysv-init script or systemd service) to configure graphics policy and
> > resource management according to the wishes of the system integrator.
> >
> > Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> > ---
> >  configure.ac                                  |  1 +
> >  tests/Makefile.am                             |  2 +-
> >  tests/drm_set_cgrp_param/Makefile.am          | 18 ++++++
> >  tests/drm_set_cgrp_param/drm_set_cgrp_param.c | 80 +++++++++++++++++++++++++++
> >  4 files changed, 100 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/drm_set_cgrp_param/Makefile.am
> >  create mode 100644 tests/drm_set_cgrp_param/drm_set_cgrp_param.c
> >
> Hi Matt,
> 
> Adding a small test/demo in libdrm sounds good. Although I think we
> need some IGT tests, if you haven't prepped them already.
> After all we need to ensure the kernel correctly validates and errors
> when we feed it the wrong info through the IOCTL.

Yeah, agreed.  Writing real IGT's was in the TODO list of the
cover-letter for my kernel patch series.  This kernel work is still a
pretty early RFC just to gather feedback on the general approach of
integrating cgroups with DRM; I figured I'd wait on writing real IGT's
until we're more confident that this is the right approach in general.

I'm working on a v2 right now that makes some pretty significant changes
to the series, but I'm not sure yet whether the uapi will change in my
next iteration or not.

> There's a small suggestions about the IOCTL design.
> s/reserved/flags/ to make future extension are possible - as mentioned in [2]

Yeah, that's why I added the reserved field; we don't have any actual
flags yet, but as soon as we do we'd rename the field to flags and
document it accordingly.  I can rename the field immediately if you
think that's easier.  I think the most important thing that's missing at
the moment is that the kernel patches forgot to check that the reserved
field is actually empty (i.e., we should reject calls with any garbage
in there now so that we don't break ABI in the future when we start
really using those bits for something).

Thanks for the feedback!

Matt

> 
> Thanks
> Emil
> 
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ioctl/botching-up-ioctls.txt?h=v4.15-rc9#n64

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795