On Thu, Jun 11, 2015 at 04:11:49PM +0200, Patrik Jakobsson wrote: > On Thu, Jun 11, 2015 at 02:26:59AM +0300, Dmitry V. Levin wrote: > > 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; > > } > > I misunderstood you. As you say, we shouldn't do a drm_get_driver_name() more > than once (no matter how many drm drivers we are looking for) so your suggestion > above would fix that. I was thinking about how to get rid of the extra call in > drm_decode_number() (if we could somehow squash them together). But that would > make things rather ugly. If ok with you we could just have the same approach in > drm_decode_number() as above and live with the fact that we get two calls to > drm_get_driver_name() per DRM device specific ioctl. One for drm_decode_number() > and one for drm_ioctl(). This way we would end up with three drm_get_driver_name() calls per ioctl: twice on entering syscall and once on exiting. Maybe we could cache result of the first of these three calls somewhere? -- ldv
Attachment:
pgp2cki9TKMBI.pgp
Description: PGP signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx