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 <p.osciak@xxxxxxxxxxx>
> >
> > 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 <p.osciak@xxxxxxxxxxx>
> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > CC: Pawel Osciak <pawel@xxxxxxxxxx>
> > ---
> > 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 <p.osciak@xxxxxxxxxxx>
> > + *
> > + * 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
