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

[Tvrtko Ursulin]

On 17/01/2023 16:03, Stanislaw Gruszka wrote:
> Hi
>
> On Thu, Jan 12, 2023 at 04:56:01PM +0000, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
>
> <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.

Thanks, looks like a copy & paste error. Noted to correct before next 
public post.

Regards,

Tvrtko