Message ID | 1242612794.3442.19.camel@myhost (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
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.zhang@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.zhang@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.zhang@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. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.zhang@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 18 May 2009, figo.zhang wrote: > saa7134-video.c: poll method lose race condition > > > Signed-off-by: Figo.zhang <figo.zhang@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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.zhang@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 majordomo@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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
saa7134-video.c: poll method lose race condition Signed-off-by: Figo.zhang <figo.zhang@kolorific.com> --- drivers/media/video/saa7134/saa7134-video.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html