[PATCH 4/4] drm/amdkfd: Check against device cgroup

[Kasiviswanathan, Harish]

Hi Roman,

Thanks for your feedback. I will rework and send new patch soon.

Best Regards,
Harish
 




From: Roman Gushchin <guro at fb.com>
Sent: Tuesday, May 14, 2019 1:37 PM
To: Kasiviswanathan, Harish
Cc: cgroups at vger.kernel.org; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 4/4] drm/amdkfd: Check against device cgroup
  

[CAUTION: External Email]

On Tue, May 14, 2019 at 01:58:40AM +0000, Roman Gushchin wrote:
> On Wed, May 01, 2019 at 02:59:29PM +0000, Kasiviswanathan, Harish wrote:
> > Participate in device cgroup. All kfd devices are exposed via /dev/kfd.
> > So use /dev/dri/renderN node.
> >
> > Before exposing the device to a task check if it has permission to
> > access it. If the task (based on its cgroup) can access /dev/dri/renderN
> > then expose the device via kfd node.
> >
> > If the task cannot access /dev/dri/renderN then process device data
> > (pdd) is not created. This will ensure that task cannot use the device.
> >
> > In sysfs topology, all device nodes are visible irrespective of the task
> > cgroup. The sysfs node directories are created at driver load time and
> > cannot be changed dynamically. However, access to information inside
> > nodes is controlled based on the task's cgroup permissions.
> >
> > Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan at amd.com>
> > Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>
>
> Hello, Harish!
>
> Cgroup/device controller part looks good to me.
> Please, feel free to use my acks for patches 3 and 4:
> Acked-by: Roman Gushchin <guro at fb.com>

Hello!

After the second look at the patchset I came to an understanding that
exporting cgroup_v1-only __devcgroup_check_permission() isn't the best idea.

Instead it would be better to export devcgroup_check_permission(), which
provides an universal interface for both cgroup v1 and v2 device controllers.
It  require some refactorings, but should be not hard.

Does it makes sense to you? Can you, please, rework this part?

Thanks!