Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA

["Hans J. Koch" <hjk@linutronix.de>]

On Tue, Dec 15, 2009 at 05:34:19AM -0800, Earl Chew wrote:
> Hans,

Hi Earl,

> 
> Thanks for the considered reply.
> 
> 
> Hans J. Koch wrote:
> > The general thing is this: The UIO core supports only static mappings.
> > The possible number of mappings is usually set at compile time or module
> > load time and is currently limited to MAX_UIO_MAPS (== 5). This number
> > is usually sufficient for devices like PCI cards, which have a limited
> > number of mappings, too. All drivers currently in the kernel only need
> > one or two.
> 
> 
> I'd like to proceed by changing struct uio_mem [MAX_UIO_MAPS] to a
> linked list.
> 
> The driver code in uio_find_mem_index(), uio_dev_add_attributes(), etc,
> iterate through the (small) array anyway, and the list space and
> performance overhead is not significant for the cases mentioned.
> 
> Such a change would make it easier to track dynamically allocated
> regions as well as pre-allocated mapping regions in the same data
> structure.

Sorry, I think I wasn't clear enough: The current interface for static
mappings shouldn't be changed. Dynamically added mappings need a new
interface.

> 
> It also plays more nicely into the next part ...
> 
> > The current implementation of the UIO core is simply not made for
> > dynamic allocation of an unlimited amount of new mappings at runtime. As
> > we have seen in this patch, it needs raping of a documented kernel
> > interface to userspace. This is not acceptable.
> > 
> > So the easiest correct solution is to create a new device (e.g.
> > /dev/uioN-dma, as Peter suggested). It should only be created for a UIO
> > driver if it has a certain flag set, something like UIO_NEEDS_DYN_DMA_ALLOC.
> 
> An approach which would play better with our existing codebase would
> be to introduce a two-step ioctl-mmap.
> 
> a. Use an ioctl() to allocate the DMA buffer. The ioctl returns two
>    things:

No. We don't want any new ioctls in the kernel.

> 
> 	1.  A mapping (page) number
> 	2.  A physical (bus) address
> 
> 
> b. Use the existing mmap() interface to gain access to the
>    DMA buffer allocated in (a). Clients would invoke mmap()
>    and use the mapping (page) number returned in (a) to
>    obtain userspace access to the DMA buffer.
> 
> 
> I think that the second step (b) would play nicely with the existing
> mmap() interface exposed by the UIO driver.

The existing interface is for static mappings only.

> 
> 
> Using an ioctl() provides a cleaner way to return the physical
> (bus) address of the DMA buffer.

ioctl() is out of fashion today. We have sysfs. Note that ioctls are neither
typesafe nor much faster than sysfs.

> 
> 
> Existing client code that is not interested in DMA buffers do
> not incur a penalty because it will not invoke the new ioctl().

What about userspace tools that rely on the fact that the number of mappings
for a UIO device cannot change? This is a documented property of UIO.

Dynamically allocated mappings really call for a new device as Peter suggested.
In fact, that would make life much easier for you. Since your the one who
implements that stuff, your free to define a new interface. Surely that new
interface will be discussed and rejected two or three times, but in the end
we'll have a nice interface that allows UIO to use DMA, even with dyynamically
allocated buffers.

Use that freedom and create a new device with a new interface. There's no
point in trying to change existing and well documented interfaces to userspace.

Thanks,
Hans