On Wed, 2005-06-29 at 21:29 +1000, Paul Mackerras wrote: > Egbert Eich writes: > > > My patches support a wider range of chipsets (Matrox, R128 and Radeon) > > and provide a framework which makes it easy to add ioctl32 support > > to more chipsets. > > Unfortunately your macros have the effect of increasing the effort > required by a kernel developer to read and understand the code. In > the kernel we care much more about code being easy to read than about > it being easy to write. A kernel developer reading my code sees the > function names that they know (access_ok, copy_to_user, etc.) and can > see at a glance what the code is doing. > > > Furthermore they have support for both the old style ioctl32 support > > and the one that uses compat_alloc_user_space() so they should work > > on a wider range of kernels. > > The biggest problem, though, is that you are still using > register_ioctl32_conversion, even for your "new" style support. That > function is about to be removed. We need to supply a compat_ioctl > function in the file_operations vector, which is what the code which > is now upstream does. > > I would also like to discuss the treatment of handles a bit more. You > have taken the approach of always hashing handles. I had concluded > some time ago that that approach had problems because of the tendency > of the DRI userspace to use physical addresses (of the framebuffer and > card registers) as the offset in mmap calls (if I remember correctly, > it was a while ago that I last looked at this stuff). I'm interested > to know how your patch solves that problem.
I've taken a look, and started writing down my understanding of handles and offsets here: http://dri.freedesktop.org/wiki/DrmMapHandling As far as I could tell, math was not being done on handles. This makes sense, since the DRM mmap handler is comparing whether the passed in offset is exactly equal to (not just in the range of) a map's offset/length. But I'm really concerned about the mixing of handles and offsets that I'm seeing, which I really can't reconcile with the code actually working. But if we could confirm that all DRI drivers were using handles to map from, then I think we've got a lot of freedom in deciding what type a handle is. If someone who's looked at this as well could read that page and correct anything I've misunderstood, that would be great. -- Eric Anholt [EMAIL PROTECTED] http://people.freebsd.org/~anholt/ [EMAIL PROTECTED] ------------------------------------------------------- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click -- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
