Re: [RFC 01/12] drm/i915: Expose list of clients in sysfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/03/2020 11:41, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2020-03-09 18:31:18)
+static int
+__i915_drm_client_register(struct i915_drm_client *client,
+                          struct task_struct *task)
+{
+       struct i915_drm_clients *clients = client->clients;
+       struct device_attribute *attr;
+       int ret = -ENOMEM;
+       char idstr[32];
+
+       client->pid = get_task_pid(task, PIDTYPE_PID);
+
+       client->name = kstrdup(task->comm, GFP_KERNEL);
+       if (!client->name)
+               goto err_name;
+
+       if (!clients->root)
+               return 0; /* intel_fbdev_init registers a client before sysfs */
+
+       snprintf(idstr, sizeof(idstr), "%u", client->id);
+       client->root = kobject_create_and_add(idstr, clients->root);
+       if (!client->root)
+               goto err_client;
+
+       attr = &client->attr.name;
+       sysfs_attr_init(&attr->attr);
+       attr->attr.name = "name";
+       attr->attr.mode = 0444;
+       attr->show = show_client_name;
+
+       ret = sysfs_create_file(client->root, (struct attribute *)attr);
+       if (ret)
+               goto err_attr;
+
+       attr = &client->attr.pid;
+       sysfs_attr_init(&attr->attr);
+       attr->attr.name = "pid";
+       attr->attr.mode = 0444;
+       attr->show = show_client_pid;
+
+       ret = sysfs_create_file(client->root, (struct attribute *)attr);
+       if (ret)
+               goto err_attr;

How do we think we will extend this (e.g. for client/1/(trace,debug))?

i915_drm_client_add_attr() ?

Or should we put all the attr here and make them known a priori?

I think I prefer i915_drm_client_add_attr, but that will also require a
notification chain? And that smells like overengineering.

At any rate we have 2 other definite users around the corner for the
client sysfs, so we should look at what API suits us.

It sounds acceptable to me to just call their setup from here. __i915_drm_client_register sounds clear enough place.

We potentially just split the function into "client core" and "add-on users" for better readability:

__i915_drm_client_register
{
	...register_client();

	...register_client_busy(client, ...);

	...register_client_xxx(client, ...);
}

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux