Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-18 Thread Guennadi Liakhovetski
On Mon, 18 May 2009, figo.zhang wrote:

 saa7134-video.c: poll method lose race condition
 
 
 Signed-off-by: Figo.zhang figo.zh...@kolorific.com

Hi,

I am not sure why you decide to cc me on your sa7a34 patches. Those are 
not the kind of devices I'm working with, so, don't think I'll be able to 
help with your patches.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-18 Thread figo.zhang
On Mon, 2009-05-18 at 08:28 +0200, Guennadi Liakhovetski wrote:
 Guennadi

hi,Guennadi,

I am sorry that it is my mistake.

Figo.zhang

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


Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-18 Thread Oldřich Jedlička
Hi Figo Zhang,

On Monday 18 of May 2009 at 04:13:13, figo.zhang wrote:
 saa7134-video.c: poll method lose race condition


 Signed-off-by: Figo.zhang figo.zh...@kolorific.com
 ---
 drivers/media/video/saa7134/saa7134-video.c |9 ++---
  1 files changed, 6 insertions(+), 3 deletions(-)

 diff --git a/drivers/media/video/saa7134/saa7134-video.c
 b/drivers/media/video/saa7134/saa7134-video.c index 493cad9..95733df 100644
 --- a/drivers/media/video/saa7134/saa7134-video.c
 +++ b/drivers/media/video/saa7134/saa7134-video.c
 @@ -1423,11 +1423,13 @@ video_poll(struct file *file, struct
 poll_table_struct *wait) {
   struct saa7134_fh *fh = file-private_data;
   struct videobuf_buffer *buf = NULL;
 + unsigned int rc = 0;

   if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type)
   return videobuf_poll_stream(file, fh-vbi, wait);

   if (res_check(fh,RESOURCE_VIDEO)) {
 + mutex_lock(fh-cap.vb_lock);
   if (!list_empty(fh-cap.stream))
   buf = list_entry(fh-cap.stream.next, struct 
 videobuf_buffer, stream);
   } else {
 @@ -1446,13 +1448,14 @@ video_poll(struct file *file, struct
 poll_table_struct *wait) }

   if (!buf)
 - return POLLERR;
 + rc = POLLERR;

Just one note (I don't know the future and meaning of this patch). The line 
above will change the meaning of the buf==NULL check. It will not return 
immediately, but call poll_wait with buf-done instead - NULL pointer access.

Cheers,
Oldrich.


   poll_wait(file, buf-done, wait);
   if (buf-state == VIDEOBUF_DONE ||
   buf-state == VIDEOBUF_ERROR)
 - return POLLIN|POLLRDNORM;
 - return 0;
 + rc = POLLIN|POLLRDNORM;
 + mutex_unlock(fh-cap.vb_lock);
 + return rc;

  err:
   mutex_unlock(fh-cap.vb_lock);


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


Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-17 Thread hermann pitton

Am Montag, den 18.05.2009, 10:13 +0800 schrieb figo.zhang:
 saa7134-video.c: poll method lose race condition
 
 
 Signed-off-by: Figo.zhang figo.zh...@kolorific.com
 --- 
 drivers/media/video/saa7134/saa7134-video.c |9 ++---
  1 files changed, 6 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/media/video/saa7134/saa7134-video.c 
 b/drivers/media/video/saa7134/saa7134-video.c
 index 493cad9..95733df 100644
 --- a/drivers/media/video/saa7134/saa7134-video.c
 +++ b/drivers/media/video/saa7134/saa7134-video.c
 @@ -1423,11 +1423,13 @@ video_poll(struct file *file, struct 
 poll_table_struct *wait)
  {
   struct saa7134_fh *fh = file-private_data;
   struct videobuf_buffer *buf = NULL;
 + unsigned int rc = 0;
  
   if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type)
   return videobuf_poll_stream(file, fh-vbi, wait);
  
   if (res_check(fh,RESOURCE_VIDEO)) {
 + mutex_lock(fh-cap.vb_lock);
   if (!list_empty(fh-cap.stream))
   buf = list_entry(fh-cap.stream.next, struct 
 videobuf_buffer, stream);
   } else {
 @@ -1446,13 +1448,14 @@ video_poll(struct file *file, struct 
 poll_table_struct *wait)
   }
  
   if (!buf)
 - return POLLERR;
 + rc = POLLERR;
  
   poll_wait(file, buf-done, wait);
   if (buf-state == VIDEOBUF_DONE ||
   buf-state == VIDEOBUF_ERROR)
 - return POLLIN|POLLRDNORM;
 - return 0;
 + rc = POLLIN|POLLRDNORM;
 + mutex_unlock(fh-cap.vb_lock);
 + return rc;
  
  err:
   mutex_unlock(fh-cap.vb_lock);
 
 

Can you please give some description on what your patch might do
something?

Or are you a robot?

Then, please give us your serial number, production year, and when we
can expect you are out of duty and replaced ;)

Cheers,
Hermann




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


Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-17 Thread hermann pitton

Am Montag, den 18.05.2009, 11:53 +0800 schrieb figo.zhang:
 On Mon, 2009-05-18 at 05:07 +0200, hermann pitton wrote:
  Am Montag, den 18.05.2009, 10:13 +0800 schrieb figo.zhang:
   saa7134-video.c: poll method lose race condition
   
   
   Signed-off-by: Figo.zhang figo.zh...@kolorific.com
   --- 
   drivers/media/video/saa7134/saa7134-video.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
   
   diff --git a/drivers/media/video/saa7134/saa7134-video.c 
   b/drivers/media/video/saa7134/saa7134-video.c
   index 493cad9..95733df 100644
   --- a/drivers/media/video/saa7134/saa7134-video.c
   +++ b/drivers/media/video/saa7134/saa7134-video.c
   @@ -1423,11 +1423,13 @@ video_poll(struct file *file, struct 
   poll_table_struct *wait)
{
 struct saa7134_fh *fh = file-private_data;
 struct videobuf_buffer *buf = NULL;
   + unsigned int rc = 0;

 if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type)
 return videobuf_poll_stream(file, fh-vbi, wait);

 if (res_check(fh,RESOURCE_VIDEO)) {
   + mutex_lock(fh-cap.vb_lock);
 if (!list_empty(fh-cap.stream))
 buf = list_entry(fh-cap.stream.next, struct 
   videobuf_buffer, stream);
 } else {
   @@ -1446,13 +1448,14 @@ video_poll(struct file *file, struct 
   poll_table_struct *wait)
 }

 if (!buf)
   - return POLLERR;
   + rc = POLLERR;

 poll_wait(file, buf-done, wait);
 if (buf-state == VIDEOBUF_DONE ||
 buf-state == VIDEOBUF_ERROR)
   - return POLLIN|POLLRDNORM;
   - return 0;
   + rc = POLLIN|POLLRDNORM;
   + mutex_unlock(fh-cap.vb_lock);
   + return rc;

err:
 mutex_unlock(fh-cap.vb_lock);
   
   
  
  Can you please give some description on what your patch might do
  something?
  
  Or are you a robot?
  
  Then, please give us your serial number, production year, and when we
  can expect you are out of duty and replaced ;)
  
  Cheers,
  Hermann
  
  
  
 
 hi, I just using the saa7134 chip to do a video capture card. The
 saa7134 driver in linux kernel, i found that the poll method have lose 
 race condition for RESOURCE_VIDEO. It have better to add a mutex lock.
 

OK then, but better stay away from the core files.

On what kernel this is?

Give us some copy/paste dmesg output for your card with i2c_scan=1.
Don't lose tuner stuff on this.

Thanks,
Hermann




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


Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-17 Thread figo.zhang
On Mon, 2009-05-18 at 05:49 +0200, hermann pitton wrote:
 Am Montag, den 18.05.2009, 11:53 +0800 schrieb figo.zhang:
  On Mon, 2009-05-18 at 05:07 +0200, hermann pitton wrote:
   Am Montag, den 18.05.2009, 10:13 +0800 schrieb figo.zhang:
saa7134-video.c: poll method lose race condition


Signed-off-by: Figo.zhang figo.zh...@kolorific.com
--- 
drivers/media/video/saa7134/saa7134-video.c |9 ++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/video/saa7134/saa7134-video.c 
b/drivers/media/video/saa7134/saa7134-video.c
index 493cad9..95733df 100644
--- a/drivers/media/video/saa7134/saa7134-video.c
+++ b/drivers/media/video/saa7134/saa7134-video.c
@@ -1423,11 +1423,13 @@ video_poll(struct file *file, struct 
poll_table_struct *wait)
 {
struct saa7134_fh *fh = file-private_data;
struct videobuf_buffer *buf = NULL;
+   unsigned int rc = 0;
 
if (V4L2_BUF_TYPE_VBI_CAPTURE == fh-type)
return videobuf_poll_stream(file, fh-vbi, wait);
 
if (res_check(fh,RESOURCE_VIDEO)) {
+   mutex_lock(fh-cap.vb_lock);
if (!list_empty(fh-cap.stream))
buf = list_entry(fh-cap.stream.next, struct 
videobuf_buffer, stream);
} else {
@@ -1446,13 +1448,14 @@ video_poll(struct file *file, struct 
poll_table_struct *wait)
}
 
if (!buf)
-   return POLLERR;
+   rc = POLLERR;
 
poll_wait(file, buf-done, wait);
if (buf-state == VIDEOBUF_DONE ||
buf-state == VIDEOBUF_ERROR)
-   return POLLIN|POLLRDNORM;
-   return 0;
+   rc = POLLIN|POLLRDNORM;
+   mutex_unlock(fh-cap.vb_lock);
+   return rc;
 
 err:
mutex_unlock(fh-cap.vb_lock);


   
   Can you please give some description on what your patch might do
   something?
   
   Or are you a robot?
   
   Then, please give us your serial number, production year, and when we
   can expect you are out of duty and replaced ;)
   
   Cheers,
   Hermann
   
   
   
  
  hi, I just using the saa7134 chip to do a video capture card. The
  saa7134 driver in linux kernel, i found that the poll method have lose 
  race condition for RESOURCE_VIDEO. It have better to add a mutex lock.
  
 
 OK then, but better stay away from the core files.
 
 On what kernel this is?
 
 Give us some copy/paste dmesg output for your card with i2c_scan=1.
 Don't lose tuner stuff on this.
 
 Thanks,
 Hermann
 
 
 
hi, hermann pitton ,i am using the git-kernel, the card have some
hardware problems now.

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


Re: [PATCH]saa7134-video.c: poll method lose race condition

2009-05-17 Thread hermann pitton
[big snip]
  
  
  
 hi, hermann pitton ,i am using the git-kernel, the card have some
 hardware problems now.

If it is a new card, 2.6.27 is for sure the last stable.

We might get it all down, but 2.6.30 looked promising again.

Seems there is some hungry black hole in between ;)

Cheers,
Hermann


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