From: Andrew Chew <achew@nvidia.com>

There are two struct list_head's in struct videobuf_buffer.
Prior to this fix, all we did for initialization of struct videobuf_buffer
was to zero out its memory.  This does not properly initialize this struct's
two list_head members.

This patch immediately calls INIT_LIST_HEAD on both lists after the kzalloc,
so that the two lists are initialized properly.

Signed-off-by: Andrew Chew <achew@nvidia.com>
---
I thought I'd submit a patch for this anyway.  Without this, the existing
camera host drivers will spew an ugly warning on every videobuf allocation,
which gets annoying really fast.

 drivers/media/video/videobuf-dma-contig.c |    2 ++
 drivers/media/video/videobuf-dma-sg.c     |    2 ++
 drivers/media/video/videobuf-vmalloc.c    |    2 ++
 3 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/videobuf-dma-contig.c b/drivers/media/video/videobuf-dma-contig.c
index c969111..f7e0f86 100644
--- a/drivers/media/video/videobuf-dma-contig.c
+++ b/drivers/media/video/videobuf-dma-contig.c
@@ -193,6 +193,8 @@ static struct videobuf_buffer *__videobuf_alloc_vb(size_t size)
 	if (vb) {
 		mem = vb->priv = ((char *)vb) + size;
 		mem->magic = MAGIC_DC_MEM;
+		INIT_LIST_HEAD(&vb->stream);
+		INIT_LIST_HEAD(&vb->queue);
 	}
 
 	return vb;
diff --git a/drivers/media/video/videobuf-dma-sg.c b/drivers/media/video/videobuf-dma-sg.c
index 20f227e..5af3217 100644
--- a/drivers/media/video/videobuf-dma-sg.c
+++ b/drivers/media/video/videobuf-dma-sg.c
@@ -430,6 +430,8 @@ static struct videobuf_buffer *__videobuf_alloc_vb(size_t size)
 
 	mem = vb->priv = ((char *)vb) + size;
 	mem->magic = MAGIC_SG_MEM;
+	INIT_LIST_HEAD(&vb->stream);
+	INIT_LIST_HEAD(&vb->queue);
 
 	videobuf_dma_init(&mem->dma);
 
diff --git a/drivers/media/video/videobuf-vmalloc.c b/drivers/media/video/videobuf-vmalloc.c
index df14258..8babedd 100644
--- a/drivers/media/video/videobuf-vmalloc.c
+++ ...
[ message continues ]

i think it no need to be init, it just a list-entry.
--

[ message continues ]

yes, i think those WARN_ONs are no need.
--

[ message continues ]

These list entries need to be inited. It is bad form to have uninitialized
list entries. It is not as if this is a big deal to initialize them properly.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco
--

[ message continues ]

in kernel source code, list entry are not often to be inited.

for example, see mm/vmscan.c register_shrinker(), no one init the 
shrinker->list.

another example: see mm/swapfile.c  add_swap_extent(), no one init the
new_se->list.





--

[ message continues ]

I have to think some more about this. I'll get back to this today. BTW, I do
agree that the WARN_ON's are bogus and should be removed.

And BTW, this isn't going to work either (mx1_camera.c):

static int mx1_camera_setup_dma(struct mx1_camera_dev *pcdev)
{
        struct videobuf_buffer *vbuf = &pcdev->active->vb;
        struct device *dev = pcdev->icd->dev.parent;
        int ret;

        if (unlikely(!pcdev->active)) {
                dev_err(dev, "DMA End IRQ with no active buffer\n");
                return -EFAULT;
        }

The vbuf assignment should be moved after the 'if'.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco
--

[ message continues ]

You are right. I got confused due to some problems I had in the past in
another driver, but it turned out to be a list header that caused the
problems, not a list entry. So removing the bogus WARN_ONs is sufficient.

Regards,

       Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

--

[ message continues ]

Hi Hans,


I disagree with that. List heads must be initialized, but there's no point in 
initializing list entries.

-- 
Regards,

Laurent Pinchart
--

[ message continues ]

Rather than doing this for all videobuf variants I would suggest that you
do this in videobuf-core.c, videobuf_alloc_vb().

Regards,


-- 
Hans Verkuil - video4linux developer - sponsored by Cisco
--

[ message continues ]