On 26 January 2018 at 17:27, Matt Roper <matthew.d.roper@xxxxxxxxx> wrote: > 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@xxxxxxxxx> 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@xxxxxxxxx> >> > --- >> > 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. > Good call - have an agreement about the interface and usage first. Then iron out all the fiddly bits. >> 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). > Please rename it now. Otherwise we'll get a build break for new kernel and old userspace. And yes, validating (returning -EINVAL IIRC) the flags is a must. Thanks Emil _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
