Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-13 Thread Mauro Carvalho Chehab

Em Qua, 2007-12-12 às 23:19 +0100, Jean Delvare escreveu:
> Hi Mauro,
> 
> On Wed, 12 Dec 2007 12:21:56 -0200, Mauro Carvalho Chehab wrote:
> > What happened is that changeset 19bc5133dae9562e8824ef101464061f9854c1d8
> > fixed some bad locks. 
> > 
> > After this changeset, videobuf_read_stream() holds q->lock and calls
> > videobuf_read_start(). To avoid waiting forever for the lock to be
> > released, he removed the mutex from videobuf_read_start with this line
> > [1]:
> > 
> > -   err = videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
> > +   err = __videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
> > 
> > So, after the patch, videobuf_read_start() can't be safely called. So,
> > just adding EXPORT_SYMBOL_GPL() breaks videobuf-dvb at runtime.
> > 
> > The proper solution is provided by this changeset:
> > http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc
> 
> There's a "static" missing in that patch: __videobuf_read_start is only
> used internally.

Thanks. It seems that I were not on my best day when I wrote this
patch

Fixed.

-- 
Cheers,
Mauro

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-13 Thread Mauro Carvalho Chehab

Em Qua, 2007-12-12 às 23:19 +0100, Jean Delvare escreveu:
 Hi Mauro,
 
 On Wed, 12 Dec 2007 12:21:56 -0200, Mauro Carvalho Chehab wrote:
  What happened is that changeset 19bc5133dae9562e8824ef101464061f9854c1d8
  fixed some bad locks. 
  
  After this changeset, videobuf_read_stream() holds q-lock and calls
  videobuf_read_start(). To avoid waiting forever for the lock to be
  released, he removed the mutex from videobuf_read_start with this line
  [1]:
  
  -   err = videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
  +   err = __videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
  
  So, after the patch, videobuf_read_start() can't be safely called. So,
  just adding EXPORT_SYMBOL_GPL() breaks videobuf-dvb at runtime.
  
  The proper solution is provided by this changeset:
  http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc
 
 There's a static missing in that patch: __videobuf_read_start is only
 used internally.

Thanks. It seems that I were not on my best day when I wrote this
patch

Fixed.

-- 
Cheers,
Mauro

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Jean Delvare
Hi Mauro,

On Wed, 12 Dec 2007 12:21:56 -0200, Mauro Carvalho Chehab wrote:
> What happened is that changeset 19bc5133dae9562e8824ef101464061f9854c1d8
> fixed some bad locks. 
> 
> After this changeset, videobuf_read_stream() holds q->lock and calls
> videobuf_read_start(). To avoid waiting forever for the lock to be
> released, he removed the mutex from videobuf_read_start with this line
> [1]:
> 
> -   err = videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
> +   err = __videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
> 
> So, after the patch, videobuf_read_start() can't be safely called. So,
> just adding EXPORT_SYMBOL_GPL() breaks videobuf-dvb at runtime.
> 
> The proper solution is provided by this changeset:
> http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc

There's a "static" missing in that patch: __videobuf_read_start is only
used internally.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Shane
On Dec 12, 2007 11:37 AM, Shane <[EMAIL PROTECTED]> wrote:
> On Dec 12, 2007 9:21 AM, Mauro Carvalho Chehab <[EMAIL PROTECTED]> wrote:
> ...
> > The proper solution is provided by this changeset:
> > http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc
>
> I applied this and it seems fine with a bttv card.

Ugh, after further testing with a bttv card it seems this is not fine.

vbi doesn't work anymore and my application gets stuck in a Zombie,
unkillable, have to reboot state :(

mythtv3683 1  -3  2.4  0.0  0 0 ?Z

Reverting Mauro's patch above does fix the problem.

Shane
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Shane
On Dec 12, 2007 9:21 AM, Mauro Carvalho Chehab <[EMAIL PROTECTED]> wrote:
...
> The proper solution is provided by this changeset:
> http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc

I applied this and it seems fine with a bttv card.

Mauro and Adrian,

Thanks for your prompt attention to this and for promptly pushing the
fix to Linus.

Regards,

Shane
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Mauro Carvalho Chehab
Em Qua, 2007-12-12 às 13:03 +0100, Adrian Bunk escreveu:
> On Wed, Dec 12, 2007 at 09:03:14AM -0200, Mauro Carvalho Chehab wrote:
> >...
> > I'm afraid that this wouldn't avoid this bug, however. 
> > 
> > The removal of the EXPORT_SYMBOL_GPL seemed to be the intention of
> > Brandon, since he renamed the function, removing the locks. I think he
> > didn't noticed that videobuf_dvb were using videobuf_read_start. The
> > patch I've just send fixes it properly.
> 
> At least in the commit in Linus' tree, videobuf_read_start() stayed 
> nearly unchanged.

Hmm... I should have reviewed the patch instead of trusting on my
memory. Yes, you're right. Brandon didn't rename this function.

What happened is that changeset 19bc5133dae9562e8824ef101464061f9854c1d8
fixed some bad locks. 

After this changeset, videobuf_read_stream() holds q->lock and calls
videobuf_read_start(). To avoid waiting forever for the lock to be
released, he removed the mutex from videobuf_read_start with this line
[1]:

-   err = videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
+   err = __videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);

So, after the patch, videobuf_read_start() can't be safely called. So,
just adding EXPORT_SYMBOL_GPL() breaks videobuf-dvb at runtime.

The proper solution is provided by this changeset:
http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc

This renames the old videobuf_read_start() to __videobuf_read_start()
and adds a newer one to be called externally, that holds the lock,
before calling __videobuf_mmap_setup().

[1] After his changeset, all functions with __foo() don't touch on
q->lock.

-- 
Cheers,
Mauro

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Adrian Bunk
On Wed, Dec 12, 2007 at 09:03:14AM -0200, Mauro Carvalho Chehab wrote:
>...
> I'm afraid that this wouldn't avoid this bug, however. 
> 
> The removal of the EXPORT_SYMBOL_GPL seemed to be the intention of
> Brandon, since he renamed the function, removing the locks. I think he
> didn't noticed that videobuf_dvb were using videobuf_read_start. The
> patch I've just send fixes it properly.

At least in the commit in Linus' tree, videobuf_read_start() stayed 
nearly unchanged.

> Btw, Shane patch reveals a small trouble with EXPORT_SYMBOL_GPL: adding
> the tag for a non-existing function didn't rise any error.

See above.

> IMO, it
> should generate some compilation error, if you try to export a symbol
> that doesn't exist at the file that is being compiled.

It does give you a compile error when there's not at least a prototype 
for the stuff to be exported, and a link error if there's no
variable/function.

> Cheers,
> Mauro

cu
Adrian

-- 

   "Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
   "Only a promise," Lao Er said.
   Pearl S. Buck - Dragon Seed

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Mauro Carvalho Chehab
> Any objections against a later path that changes the exports to the 
> general "immediately after the function" convention?

No objections. Please generate against "devel" branch on my -git, since
I did a patch fixing most CodingStyle issues reported by checkpatch.pl.

Several files under /media still uses the old convention of having such
things at the end of the file.

> It would have avoided at least two such bugs in this file alone since 
> 2.6.23...

I'm afraid that this wouldn't avoid this bug, however. 

The removal of the EXPORT_SYMBOL_GPL seemed to be the intention of
Brandon, since he renamed the function, removing the locks. I think he
didn't noticed that videobuf_dvb were using videobuf_read_start. The
patch I've just send fixes it properly.

Btw, Shane patch reveals a small trouble with EXPORT_SYMBOL_GPL: adding
the tag for a non-existing function didn't rise any error. IMO, it
should generate some compilation error, if you try to export a symbol
that doesn't exist at the file that is being compiled.
 
Cheers,
Mauro

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Mauro Carvalho Chehab

Em Qua, 2007-12-12 às 00:20 -0500, Shane escreveu:
> In 2.6.24-rc5+, I hit this problem with videobuf_read_start
> not being exported. Patch attached, only compile tested.
> 
>   CHK include/linux/version.h
>   CHK include/linux/utsrelease.h
>   CALLscripts/checksyscalls.sh
>   CHK include/linux/compile.h
>   CC [M]  drivers/media/video/videobuf-core.o
>   Building modules, stage 2.
> Kernel: arch/x86/boot/bzImage is ready  (#1)
>   MODPOST 202 modules
> ERROR: "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
Thanks for reporting.

The patch is incomplete. I've just sent Linus the complete fix for this.

The issue here is that videobuf_read_start were replaced by
__videobuf_read_start, but mutex is missing at the last.

The patch I've just sent adds a mutex-safe version of
videobuf_read_start, adding the proper EXPORT_SYMBOL_GPL().
 
Cheers,
Mauro

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Soeren Sonnenburg
On Wed, 2007-12-12 at 00:20 -0500, Shane wrote:
> In 2.6.24-rc5+, I hit this problem with videobuf_read_start
> not being exported. Patch attached, only compile tested.
> 
>   CHK include/linux/version.h
>   CHK include/linux/utsrelease.h
>   CALLscripts/checksyscalls.sh
>   CHK include/linux/compile.h
>   CC [M]  drivers/media/video/videobuf-core.o
>   Building modules, stage 2.
> Kernel: arch/x86/boot/bzImage is ready  (#1)
>   MODPOST 202 modules
> ERROR: "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2

FWIW, I've seen the same thing and Shane's patch fixes things for me.

Soeren
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Adrian Bunk
Patch looks good, it seems a merge conflict of
commit 19bc5133dae9562e8824ef101464061f9854c1d8
was resolved the wrong way.

@Mauro:
Any objections against a later path that changes the exports to the 
general "immediately after the function" convention?
It would have avoided at least two such bugs in this file alone since 
2.6.23...

cu
Adrian


On Wed, Dec 12, 2007 at 12:20:31AM -0500, Shane wrote:
> In 2.6.24-rc5+, I hit this problem with videobuf_read_start
> not being exported. Patch attached, only compile tested.
> 
>   CHK include/linux/version.h
>   CHK include/linux/utsrelease.h
>   CALLscripts/checksyscalls.sh
>   CHK include/linux/compile.h
>   CC [M]  drivers/media/video/videobuf-core.o
>   Building modules, stage 2.
> Kernel: arch/x86/boot/bzImage is ready  (#1)
>   MODPOST 202 modules
> ERROR: "videobuf_read_start" [drivers/media/video/videobuf-dvb.ko] undefined!
> make[1]: *** [__modpost] Error 1
> make: *** [modules] Error 2
> 
> Shane

> diff --git a/drivers/media/video/videobuf-core.c 
> b/drivers/media/video/videobuf-core.c
> index de2f56b..44fa76b 100644
> --- a/drivers/media/video/videobuf-core.c
> +++ b/drivers/media/video/videobuf-core.c
> @@ -1058,6 +1058,7 @@ EXPORT_SYMBOL_GPL(videobuf_dqbuf);
>  EXPORT_SYMBOL_GPL(videobuf_streamon);
>  EXPORT_SYMBOL_GPL(videobuf_streamoff);
>  
> +EXPORT_SYMBOL_GPL(videobuf_read_start);
>  EXPORT_SYMBOL_GPL(videobuf_read_stop);
>  EXPORT_SYMBOL_GPL(videobuf_stop);
>  EXPORT_SYMBOL_GPL(videobuf_read_stream);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Adrian Bunk
Patch looks good, it seems a merge conflict of
commit 19bc5133dae9562e8824ef101464061f9854c1d8
was resolved the wrong way.

@Mauro:
Any objections against a later path that changes the exports to the 
general immediately after the function convention?
It would have avoided at least two such bugs in this file alone since 
2.6.23...

cu
Adrian


On Wed, Dec 12, 2007 at 12:20:31AM -0500, Shane wrote:
 In 2.6.24-rc5+, I hit this problem with videobuf_read_start
 not being exported. Patch attached, only compile tested.
 
   CHK include/linux/version.h
   CHK include/linux/utsrelease.h
   CALLscripts/checksyscalls.sh
   CHK include/linux/compile.h
   CC [M]  drivers/media/video/videobuf-core.o
   Building modules, stage 2.
 Kernel: arch/x86/boot/bzImage is ready  (#1)
   MODPOST 202 modules
 ERROR: videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!
 make[1]: *** [__modpost] Error 1
 make: *** [modules] Error 2
 
 Shane

 diff --git a/drivers/media/video/videobuf-core.c 
 b/drivers/media/video/videobuf-core.c
 index de2f56b..44fa76b 100644
 --- a/drivers/media/video/videobuf-core.c
 +++ b/drivers/media/video/videobuf-core.c
 @@ -1058,6 +1058,7 @@ EXPORT_SYMBOL_GPL(videobuf_dqbuf);
  EXPORT_SYMBOL_GPL(videobuf_streamon);
  EXPORT_SYMBOL_GPL(videobuf_streamoff);
  
 +EXPORT_SYMBOL_GPL(videobuf_read_start);
  EXPORT_SYMBOL_GPL(videobuf_read_stop);
  EXPORT_SYMBOL_GPL(videobuf_stop);
  EXPORT_SYMBOL_GPL(videobuf_read_stream);


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Soeren Sonnenburg
On Wed, 2007-12-12 at 00:20 -0500, Shane wrote:
 In 2.6.24-rc5+, I hit this problem with videobuf_read_start
 not being exported. Patch attached, only compile tested.
 
   CHK include/linux/version.h
   CHK include/linux/utsrelease.h
   CALLscripts/checksyscalls.sh
   CHK include/linux/compile.h
   CC [M]  drivers/media/video/videobuf-core.o
   Building modules, stage 2.
 Kernel: arch/x86/boot/bzImage is ready  (#1)
   MODPOST 202 modules
 ERROR: videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!
 make[1]: *** [__modpost] Error 1
 make: *** [modules] Error 2

FWIW, I've seen the same thing and Shane's patch fixes things for me.

Soeren
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Mauro Carvalho Chehab

Em Qua, 2007-12-12 às 00:20 -0500, Shane escreveu:
 In 2.6.24-rc5+, I hit this problem with videobuf_read_start
 not being exported. Patch attached, only compile tested.
 
   CHK include/linux/version.h
   CHK include/linux/utsrelease.h
   CALLscripts/checksyscalls.sh
   CHK include/linux/compile.h
   CC [M]  drivers/media/video/videobuf-core.o
   Building modules, stage 2.
 Kernel: arch/x86/boot/bzImage is ready  (#1)
   MODPOST 202 modules
 ERROR: videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!
 make[1]: *** [__modpost] Error 1
 make: *** [modules] Error 2
Thanks for reporting.

The patch is incomplete. I've just sent Linus the complete fix for this.

The issue here is that videobuf_read_start were replaced by
__videobuf_read_start, but mutex is missing at the last.

The patch I've just sent adds a mutex-safe version of
videobuf_read_start, adding the proper EXPORT_SYMBOL_GPL().
 
Cheers,
Mauro

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Mauro Carvalho Chehab
 Any objections against a later path that changes the exports to the 
 general immediately after the function convention?

No objections. Please generate against devel branch on my -git, since
I did a patch fixing most CodingStyle issues reported by checkpatch.pl.

Several files under /media still uses the old convention of having such
things at the end of the file.

 It would have avoided at least two such bugs in this file alone since 
 2.6.23...

I'm afraid that this wouldn't avoid this bug, however. 

The removal of the EXPORT_SYMBOL_GPL seemed to be the intention of
Brandon, since he renamed the function, removing the locks. I think he
didn't noticed that videobuf_dvb were using videobuf_read_start. The
patch I've just send fixes it properly.

Btw, Shane patch reveals a small trouble with EXPORT_SYMBOL_GPL: adding
the tag for a non-existing function didn't rise any error. IMO, it
should generate some compilation error, if you try to export a symbol
that doesn't exist at the file that is being compiled.
 
Cheers,
Mauro

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Adrian Bunk
On Wed, Dec 12, 2007 at 09:03:14AM -0200, Mauro Carvalho Chehab wrote:
...
 I'm afraid that this wouldn't avoid this bug, however. 
 
 The removal of the EXPORT_SYMBOL_GPL seemed to be the intention of
 Brandon, since he renamed the function, removing the locks. I think he
 didn't noticed that videobuf_dvb were using videobuf_read_start. The
 patch I've just send fixes it properly.

At least in the commit in Linus' tree, videobuf_read_start() stayed 
nearly unchanged.

 Btw, Shane patch reveals a small trouble with EXPORT_SYMBOL_GPL: adding
 the tag for a non-existing function didn't rise any error.

See above.

 IMO, it
 should generate some compilation error, if you try to export a symbol
 that doesn't exist at the file that is being compiled.

It does give you a compile error when there's not at least a prototype 
for the stuff to be exported, and a link error if there's no
variable/function.

 Cheers,
 Mauro

cu
Adrian

-- 

   Is there not promise of rain? Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
   Only a promise, Lao Er said.
   Pearl S. Buck - Dragon Seed

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Mauro Carvalho Chehab
Em Qua, 2007-12-12 às 13:03 +0100, Adrian Bunk escreveu:
 On Wed, Dec 12, 2007 at 09:03:14AM -0200, Mauro Carvalho Chehab wrote:
 ...
  I'm afraid that this wouldn't avoid this bug, however. 
  
  The removal of the EXPORT_SYMBOL_GPL seemed to be the intention of
  Brandon, since he renamed the function, removing the locks. I think he
  didn't noticed that videobuf_dvb were using videobuf_read_start. The
  patch I've just send fixes it properly.
 
 At least in the commit in Linus' tree, videobuf_read_start() stayed 
 nearly unchanged.

Hmm... I should have reviewed the patch instead of trusting on my
memory. Yes, you're right. Brandon didn't rename this function.

What happened is that changeset 19bc5133dae9562e8824ef101464061f9854c1d8
fixed some bad locks. 

After this changeset, videobuf_read_stream() holds q-lock and calls
videobuf_read_start(). To avoid waiting forever for the lock to be
released, he removed the mutex from videobuf_read_start with this line
[1]:

-   err = videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
+   err = __videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);

So, after the patch, videobuf_read_start() can't be safely called. So,
just adding EXPORT_SYMBOL_GPL() breaks videobuf-dvb at runtime.

The proper solution is provided by this changeset:
http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc

This renames the old videobuf_read_start() to __videobuf_read_start()
and adds a newer one to be called externally, that holds the lock,
before calling __videobuf_mmap_setup().

[1] After his changeset, all functions with __foo() don't touch on
q-lock.

-- 
Cheers,
Mauro

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Shane
On Dec 12, 2007 9:21 AM, Mauro Carvalho Chehab [EMAIL PROTECTED] wrote:
...
 The proper solution is provided by this changeset:
 http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc

I applied this and it seems fine with a bttv card.

Mauro and Adrian,

Thanks for your prompt attention to this and for promptly pushing the
fix to Linus.

Regards,

Shane
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Shane
On Dec 12, 2007 11:37 AM, Shane [EMAIL PROTECTED] wrote:
 On Dec 12, 2007 9:21 AM, Mauro Carvalho Chehab [EMAIL PROTECTED] wrote:
 ...
  The proper solution is provided by this changeset:
  http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc

 I applied this and it seems fine with a bttv card.

Ugh, after further testing with a bttv card it seems this is not fine.

vbi doesn't work anymore and my application gets stuck in a Zombie,
unkillable, have to reboot state :(

mythtv3683 1  -3  2.4  0.0  0 0 ?Zl  13:40:35
00:00:06 [mythbackend] defunct

Reverting Mauro's patch above does fix the problem.

Shane
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: 2.6.24-rc5 videobuf_read_start [drivers/media/video/videobuf-dvb.ko] undefined!

2007-12-12 Thread Jean Delvare
Hi Mauro,

On Wed, 12 Dec 2007 12:21:56 -0200, Mauro Carvalho Chehab wrote:
 What happened is that changeset 19bc5133dae9562e8824ef101464061f9854c1d8
 fixed some bad locks. 
 
 After this changeset, videobuf_read_stream() holds q-lock and calls
 videobuf_read_start(). To avoid waiting forever for the lock to be
 released, he removed the mutex from videobuf_read_start with this line
 [1]:
 
 -   err = videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
 +   err = __videobuf_mmap_setup(q, count, size, V4L2_MEMORY_USERPTR);
 
 So, after the patch, videobuf_read_start() can't be safely called. So,
 just adding EXPORT_SYMBOL_GPL() breaks videobuf-dvb at runtime.
 
 The proper solution is provided by this changeset:
 http://git.kernel.org/?p=linux/kernel/git/mchehab/v4l-dvb.git;a=commitdiff;h=19fb1457990b6b7e15586ec7331541a184233acc

There's a static missing in that patch: __videobuf_read_start is only
used internally.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/