[linux-dvb] [PATCH] dvb/cinergyT2: Fix cinergyt2_poll() to allow
for non-blocking IO on the frontend device
Dyks, Axel (XL)
xl at xlsigned.net
Sun May 14 14:30:58 CEST 2006
Johannes Stezenbach wrote:
> On Sat, May 13, 2006, Dyks, Axel (XL) wrote:
>>"cinergyt2_poll()" shouldn't return (POLLIN | POLLRDNORM | POLLPRI) when
>>there are no pending events. User space programs that do non-bocking IO
>>using "select()" and/or "poll()" would otherwise produce high system load.
>>See http://forum.videolan.org/viewtopic.php?t=20529 for details.
>>Signed-off-by: Axel Dyks <xl at xlsigned.net>
>>--- linux-2.6.16.orig/drivers/media/dvb/cinergyT2/cinergyT2.c 2006-03-20 06:53:29.000000000 +0100
>>+++ linux-2.6.16/drivers/media/dvb/cinergyT2/cinergyT2.c 2006-05-13 01:15:24.000000000 +0200
>>@@ -540,15 +540,18 @@
>> struct dvb_device *dvbdev = file->private_data;
>> struct cinergyt2 *cinergyt2 = dvbdev->priv;
>>+ unsigned int mask = 0;
>> if (cinergyt2->disconnect_pending || down_interruptible(&cinergyt2->sem))
>> return -ERESTARTSYS;
>> poll_wait(file, &cinergyt2->poll_wq, wait);
>>+ if (cinergyt2->pending_fe_events != 0) mask |= (POLLIN | POLLRDNORM | POLLPRI);
> Please *never* do this. Better:
> if (cinergyt2->pending_fe_events != 0)
> mask |= (POLLIN | POLLRDNORM | POLLPRI);
No problem. Feel free to change the indention.
Or shall I resend a modified version of the patch?
>>- return (POLLIN | POLLRDNORM | POLLPRI);
>>+ return mask;
> BTW, the sem stuff has been replaced by mutex. However, I wonder
> why that sem/mutex is used at all. What data does it protect?
> *cinergyt2 won't be released before the fd is closed (if it
> would, cinergyt2->sem would also be invalid)...
Yes, I've noted that someone has auto-converted the cinergyT2 module to use
mutexes instead of semaphores (signed-off by Andrew Morton, I think).
If needed, I can base by patch on the "mutexed" version. Shall I?
BTW: Is this the correct place for contributing patches that meant
to be integrated into the linux kernel sources?
Regarding the semaphore:
I think it "protects" the "wait queue" or rather the "pending-fe-events"
value which may be asynchronly modified ...
More information about the linux-dvb