Hello,

On Thursday, November 25, 2010 11:07 AM Hans Verkuil wrote:

> On Friday, November 19, 2010 16:55:40 Marek Szyprowski wrote:
> > From: Pawel Osciak <[email protected]>
> >
> > Add an implementation of contiguous virtual memory allocator and handling
> > routines for videobuf2, implemented on top of vmalloc()/vfree() calls.
> >
> > Signed-off-by: Pawel Osciak <[email protected]>
> > Signed-off-by: Kyungmin Park <[email protected]>
> > Signed-off-by: Marek Szyprowski <[email protected]>
> > CC: Pawel Osciak <[email protected]>
> > ---
> >  drivers/media/video/Kconfig             |    5 +
> >  drivers/media/video/Makefile            |    1 +
> >  drivers/media/video/videobuf2-vmalloc.c |  177 
> > +++++++++++++++++++++++++++++++
> >  include/media/videobuf2-vmalloc.h       |   21 ++++
> >  4 files changed, 204 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/media/video/videobuf2-vmalloc.c
> >  create mode 100644 include/media/videobuf2-vmalloc.h
> >
> > diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> > index 83ce858..9351423 100644
> > --- a/drivers/media/video/Kconfig
> > +++ b/drivers/media/video/Kconfig
> > @@ -55,6 +55,11 @@ config VIDEOBUF2_CORE
> >  config VIDEOBUF2_MEMOPS
> >     tristate
> >
> > +config VIDEOBUF2_VMALLOC
> > +   select VIDEOBUF2_CORE
> > +   select VIDEOBUF2_MEMOPS
> > +   tristate
> > +
> >  #
> >  # Multimedia Video device configuration
> >  #
> > diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> > index a97a2a0..538bee9 100644
> > --- a/drivers/media/video/Makefile
> > +++ b/drivers/media/video/Makefile
> > @@ -116,6 +116,7 @@ obj-$(CONFIG_VIDEO_BTCX)  += btcx-risc.o
> >
> >  obj-$(CONFIG_VIDEOBUF2_CORE)               += videobuf2-core.o
> >  obj-$(CONFIG_VIDEOBUF2_MEMOPS)             += videobuf2-memops.o
> > +obj-$(CONFIG_VIDEOBUF2_VMALLOC)            += videobuf2-vmalloc.o
> >
> >  obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
> >
> > diff --git a/drivers/media/video/videobuf2-vmalloc.c 
> > b/drivers/media/video/videobuf2-vmalloc.c
> > new file mode 100644
> > index 0000000..3310900
> > --- /dev/null
> > +++ b/drivers/media/video/videobuf2-vmalloc.c
> > @@ -0,0 +1,177 @@
> > +/*
> > + * videobuf2-vmalloc.c - vmalloc memory allocator for videobuf2
> > + *
> > + * Copyright (C) 2010 Samsung Electronics
> > + *
> > + * Author: Pawel Osciak <[email protected]>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/mm.h>
> > +#include <linux/slab.h>
> > +#include <linux/vmalloc.h>
> > +
> > +#include <media/videobuf2-core.h>
> > +#include <media/videobuf2-memops.h>
> > +
> > +struct vb2_vmalloc_conf {
> > +   struct vb2_alloc_ctx    alloc_ctx;
> > +};
> > +
> > +struct vb2_vmalloc_buf {
> > +   void                    *vaddr;
> > +   unsigned long           size;
> > +   unsigned int            refcount;
> > +};
> > +
> > +static void *vb2_vmalloc_alloc(const struct vb2_alloc_ctx *alloc_ctx,
> > +                           unsigned long size)
> > +{
> > +   struct vb2_vmalloc_buf *buf;
> > +
> > +   buf = kzalloc(sizeof *buf, GFP_KERNEL);
> > +   if (!buf)
> > +           return NULL;
> > +
> > +   buf->size = size;
> > +   buf->vaddr = vmalloc_user(buf->size);
> > +   if (!buf->vaddr) {
> > +           printk(KERN_ERR "vmalloc of size %ld failed\n", buf->size);
> > +           kfree(buf);
> > +           return NULL;
> > +   }
> > +
> > +   buf->refcount++;
> > +   printk(KERN_DEBUG "Allocated vmalloc buffer of size %ld at vaddr=%p\n",
> > +                   buf->size, buf->vaddr);
> > +
> > +   return buf;
> > +}
> > +
> > +static void vb2_vmalloc_put(void *buf_priv)
> > +{
> > +   struct vb2_vmalloc_buf *buf = buf_priv;
> > +
> > +   buf->refcount--;
> > +
> > +   if (0 == buf->refcount) {
> 
> I think I would feel happier if refcount was of type atomic_t and you would 
> use
> the atomic decrease-and-test operation here. I know that this is almost 
> certainly
> called with some lock, but still...

Yes, it is called with mm_sem taken in all cases. Maybe a comment in the source 
will
be enough to indicate that this variable does not need to be atomic_t type?
I can change it to atomic_t if this is really required anyway.

<snip>

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to