Re: [PATCH 2/4] drm: Add dispatcher and driver identification for DRM

["Dmitry V. Levin" <ldv@altlinux.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2876 bytes --]

On Wed, Jun 10, 2015 at 01:52:33PM +0200, Patrik Jakobsson wrote:
> On Wed, Jun 10, 2015 at 01:14:20AM +0300, Dmitry V. Levin wrote:
> > On Tue, Jun 09, 2015 at 01:26:42PM +0200, Patrik Jakobsson wrote:
[...]
> > > +#define DRM_MAX_NAME_LEN 128
> > > +
> > > +inline int drm_is_priv(const unsigned int num)
> > > +{
> > > +	return (_IOC_NR(num) >= DRM_COMMAND_BASE &&
> > > +		_IOC_NR(num) < DRM_COMMAND_END);
> > > +}
> > > +
> > > +static int drm_get_driver_name(struct tcb *tcp, char *name, size_t bufsize)
> > > +{
> > > +	char path[PATH_MAX];
> > > +	char link[PATH_MAX];
> > > +	int ret;
> > > +
> > > +	ret = getfdpath(tcp, tcp->u_arg[0], path, PATH_MAX - 1);
> > > +	if (!ret)
> > > +		return ret;
> > > +
> > > +	snprintf(link, PATH_MAX, "/sys/class/drm/%s/device/driver",
> > > +		 basename(path));
> > > +
> > > +	ret = readlink(link, path, PATH_MAX - 1);
> > > +	if (ret < 0)
> > > +		return ret;
> > > +
> > > +	path[ret] = '\0';
> > > +	strncpy(name, basename(path), bufsize);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int drm_is_driver(struct tcb *tcp, const char *name)
> > > +{
> > > +	char drv[DRM_MAX_NAME_LEN];
> > > +	int ret;
> > > +
> > > +	ret = drm_get_driver_name(tcp, drv, DRM_MAX_NAME_LEN);
> > > +	if (ret)
> > > +		return 0;
> > > +
> > > +	return strcmp(name, drv) == 0;
> > > +}
> > 
> > This interface will result to several getfdpath() calls per
> > ioctl_decode().  If the only purpose of drm_is_driver() is to help finding
> > the most appropriate function, let's create a table of pairs
> > {driver name, function} and pass this table to a function that will do a
> > single getfdpath() call, calculate the driver name, and choose the right
> > function from the table.
> 
> Yes I was thinking the same thing but it's a bit tricky. What I need is:
> fd -> path -> driver name. And fd -> path could change between ioctls. It is not
> a likely scenario but it's possible. I could get rid of the extra call in
> drm_decode_number() if I put it back in drm_ioctl as in my RFC. I could also
> optimize path -> driver name with a table but I don't know how expensive those
> calls actually are. Not sure what would be the best solution here.

drm_get_driver_name() is quite expensive as it does two readlink syscalls,
so it should be called at most once per ioctl_decode().

Another method to achieve this is to change drm_get_driver_name() to return
basename(path) instead of return code, so that drm_ioctl() would call it
once and pass the result to strcmp calls:

int
drm_ioctl(struct tcb *tcp, const unsigned int code, long arg)
{
	if (verbose(tcp) && drm_is_priv(code)) {
		const char *driver = drm_get_driver_name(tcp);
		if (!driver)
			return 0;
		if (!strcmp(driver, "i915"))
			return drm_i915_ioctl(tcp, code, arg);
	}
	return 0;
}


-- 
ldv

[-- Attachment #1.2: Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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