Re: [RFC] dma-buf: Implement test module

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]

On Thu, Dec 12, 2013 at 06:04:13PM -0800, Greg Kroah-Hartman wrote:
> On Thu, Dec 12, 2013 at 03:36:29PM +0100, Thierry Reding wrote:
> > This is a simple test module that can be used to allocate, export and
> > delete DMA-BUF objects. It can be used to test DMA-BUF sharing in
> > systems that lack a real second driver.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/base/Kconfig        |   4 +
> >  drivers/base/Makefile       |   1 +
> >  drivers/base/dma-buf-test.c | 308 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 313 insertions(+)
> >  create mode 100644 drivers/base/dma-buf-test.c
> > 
> > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> > index e373671652b0..bed2abb9491b 100644
> > --- a/drivers/base/Kconfig
> > +++ b/drivers/base/Kconfig
> > @@ -200,6 +200,10 @@ config DMA_SHARED_BUFFER
> >  	  APIs extension; the file's descriptor can then be passed on to other
> >  	  driver.
> >  
> > +config DMA_BUF_TEST
> > +	tristate "DMA-BUF test module"
> > +	depends on DMA_SHARED_BUFFER
> 
> We need some good documentation here.

I agree. The description should go into more details about what exactly
this is meant to address.

> > > +static struct miscdevice dmabuf_device = {
> > +	.minor = 128,
> 
> Why did you pick this minor?  Why not just make it dynamic?

It seemed like minors are statically allocated for miscdevice and 128
seemed to be as good as any. The tentative plan was to go through the
official way of having one allocated as explained in the comment in
include/linux/miscdevice.h.

Reading that comment again, there's MISC_DYNAMIC_MINOR, which I guess
would be appropriate here. Chances are that if you want to test DMA-BUF
you'll have a reasonably modern userspace that will create the device
dynamically.

Alternatively I guess I could instead turn this into a more full-fledged
cdev and do the whole alloc_chrdev_region() dance. Although that will
only allocate the major dynamically.

I'm not aware of any function that just allocates a single major/minor
pair completely dynamically. Is there one that you could point me to?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]