Re: [RFC 04/12] drm/cgroup: Track clients per owning process

Stanislaw Gruszka
2023-01-17T16:03:11Z

Hi

On Thu, Jan 12, 2023 at 04:56:01PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> To enable propagation of settings from the cgroup drm controller to drm we
> need to start tracking which processes own which drm clients.
>
> Implement that by tracking the struct pid pointer of the owning process in
> a new XArray, pointing to a structure containing a list of associated
> struct drm_file pointers.
>
> Clients are added and removed under the filelist mutex and RCU list
> operations are used below it to allow for lockless lookup.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

<snip>

> +int drm_clients_open(struct drm_file *file_priv)
> +{
> + struct drm_device *dev = file_priv->minor->dev;
> + struct drm_pid_clients *clients;
> + bool new_client = false;
> + unsigned long pid;
> +
> + lockdep_assert_held(&dev->filelist_mutex);
> +
> + pid = (unsigned long)rcu_access_pointer(file_priv->pid);
> + clients = xa_load(&drm_pid_clients, pid);
> + if (!clients) {
> + clients = __alloc_clients();
> + if (!clients)
> + return -ENOMEM;
> + new_client = true;
> + }
> + atomic_inc(&clients->num);
> + list_add_tail_rcu(&file_priv->clink, &clients->file_list);
> + if (new_client) {
> + void *xret;
> +
> + xret = xa_store(&drm_pid_clients, pid, clients, GFP_KERNEL);
> + if (xa_err(xret)) {
> + list_del_init(&file_priv->clink);
> + kfree(clients);
> + return PTR_ERR(clients);

This looks incorrect, rather xa_err(xret) should be returned.

Regards
Stanislaw