Hi,
I have done a minimaly invasive patch for the stable 2.6.34 kernel and stress-tested
it for many hours, definitely seems to improve the behaviour.
I have left out your beautification suggestion for now, want to do more playing with
other aspects of the driver. There still seem to be issues when the device is unplugged
while in use and such.
--- linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c.rz 2010-06-03 21:58:11.000000000 +0200
+++ linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c 2010-06-04 23:00:35.000000000 +0200
@@ -1100,31 +1100,26 @@
*
* @return pointer to descriptor on success, NULL on error.
*/
-struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
+
+struct smscore_buffer_t *get_entry(void)
{
struct smscore_buffer_t *cb = NULL;
unsigned long flags;
- DEFINE_WAIT(wait);
-
spin_lock_irqsave(&coredev->bufferslock, flags);
-
- /* This function must return a valid buffer, since the buffer list is
- * finite, we check that there is an available buffer, if not, we wait
- * until such buffer become available.
- */
-
- prepare_to_wait(&coredev->buffer_mng_waitq, &wait, TASK_INTERRUPTIBLE);
-
- if (list_empty(&coredev->buffers))
- schedule();
-
- finish_wait(&coredev->buffer_mng_waitq, &wait);
-
+ if (!list_empty(&coredev->buffers)) {
cb = (struct smscore_buffer_t *) coredev->buffers.next;
list_del(&cb->entry);
-
+ }
spin_unlock_irqrestore(&coredev->bufferslock, flags);
+ return cb;
+}
+
+struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
+{
+ struct smscore_buffer_t *cb = NULL;
+
+ wait_event(coredev->buffer_mng_waitq, (cb = get_entry()));
return cb;
}
Richard
On 06/06/2010 02:43 PM, Richard Zidlicky wrote:
> Hi,
>
> I have done a minimaly invasive patch for the stable 2.6.34 kernel and stress-tested
> it for many hours, definitely seems to improve the behaviour.
>
> I have left out your beautification suggestion for now, want to do more playing with
> other aspects of the driver. There still seem to be issues when the device is unplugged
> while in use and such.
>
> --- linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c.rz 2010-06-03 21:58:11.000000000 +0200
> +++ linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c 2010-06-04 23:00:35.000000000 +0200
> @@ -1100,31 +1100,26 @@
> *
> * @return pointer to descriptor on success, NULL on error.
> */
> -struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
> +
> +struct smscore_buffer_t *get_entry(void)
> {
> struct smscore_buffer_t *cb = NULL;
> unsigned long flags;
>
> - DEFINE_WAIT(wait);
> -
> spin_lock_irqsave(&coredev->bufferslock, flags);
Sorry, maybe I'm just blind, but where is 'coredev' defined in this
scope? You probably forgot to pass it to get_entry?
How could this be compiled? Is there coredev defined globally?
> -
> - /* This function must return a valid buffer, since the buffer list is
> - * finite, we check that there is an available buffer, if not, we wait
> - * until such buffer become available.
> - */
> -
> - prepare_to_wait(&coredev->buffer_mng_waitq, &wait, TASK_INTERRUPTIBLE);
> -
> - if (list_empty(&coredev->buffers))
> - schedule();
> -
> - finish_wait(&coredev->buffer_mng_waitq, &wait);
> -
> + if (!list_empty(&coredev->buffers)) {
> cb = (struct smscore_buffer_t *) coredev->buffers.next;
> list_del(&cb->entry);
> -
> + }
> spin_unlock_irqrestore(&coredev->bufferslock, flags);
> + return cb;
> +}
> +
> +struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
> +{
> + struct smscore_buffer_t *cb = NULL;
> +
> + wait_event(coredev->buffer_mng_waitq, (cb = get_entry()));
>
> return cb;
> }
--
js
On Sun, Jun 06, 2010 at 07:51:56PM +0200, Jiri Slaby wrote:
> On 06/06/2010 02:43 PM, Richard Zidlicky wrote:
> > Hi,
> >
> > I have done a minimaly invasive patch for the stable 2.6.34 kernel and stress-tested
> > it for many hours, definitely seems to improve the behaviour.
> >
> > I have left out your beautification suggestion for now, want to do more playing with
> > other aspects of the driver. There still seem to be issues when the device is unplugged
> > while in use and such.
> >
> > --- linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c.rz 2010-06-03 21:58:11.000000000 +0200
> > +++ linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c 2010-06-04 23:00:35.000000000 +0200
> > @@ -1100,31 +1100,26 @@
> > *
> > * @return pointer to descriptor on success, NULL on error.
> > */
> > -struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
> > +
> > +struct smscore_buffer_t *get_entry(void)
> > {
> > struct smscore_buffer_t *cb = NULL;
> > unsigned long flags;
> >
> > - DEFINE_WAIT(wait);
> > -
> > spin_lock_irqsave(&coredev->bufferslock, flags);
>
> Sorry, maybe I'm just blind, but where is 'coredev' defined in this
> scope? You probably forgot to pass it to get_entry?
>
> How could this be compiled? Is there coredev defined globally?
good catch. I think it failed and despite a different kernel id the old module was
loaded.
Here is the new version, this time lightly tested
--- linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c.rz 2010-06-03 21:58:11.000000000 +0200
+++ linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c 2010-06-07 14:32:06.000000000 +0200
@@ -1100,31 +1100,26 @@
*
* @return pointer to descriptor on success, NULL on error.
*/
-struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
+
+struct smscore_buffer_t *get_entry(struct smscore_device_t *coredev)
{
struct smscore_buffer_t *cb = NULL;
unsigned long flags;
- DEFINE_WAIT(wait);
-
spin_lock_irqsave(&coredev->bufferslock, flags);
-
- /* This function must return a valid buffer, since the buffer list is
- * finite, we check that there is an available buffer, if not, we wait
- * until such buffer become available.
- */
-
- prepare_to_wait(&coredev->buffer_mng_waitq, &wait, TASK_INTERRUPTIBLE);
-
- if (list_empty(&coredev->buffers))
- schedule();
-
- finish_wait(&coredev->buffer_mng_waitq, &wait);
-
+ if (!list_empty(&coredev->buffers)) {
cb = (struct smscore_buffer_t *) coredev->buffers.next;
list_del(&cb->entry);
-
+ }
spin_unlock_irqrestore(&coredev->bufferslock, flags);
+ return cb;
+}
+
+struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
+{
+ struct smscore_buffer_t *cb = NULL;
+
+ wait_event(coredev->buffer_mng_waitq, (cb = get_entry(coredev)));
return cb;
}
Richard
Em 07-06-2010 10:00, Richard Zidlicky escreveu:
> On Sun, Jun 06, 2010 at 07:51:56PM +0200, Jiri Slaby wrote:
>> On 06/06/2010 02:43 PM, Richard Zidlicky wrote:
>>> Hi,
>>>
>>> I have done a minimaly invasive patch for the stable 2.6.34 kernel and stress-tested
>>> it for many hours, definitely seems to improve the behaviour.
>>>
>>> I have left out your beautification suggestion for now, want to do more playing with
>>> other aspects of the driver. There still seem to be issues when the device is unplugged
>>> while in use and such.
>>>
>>> --- linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c.rz 2010-06-03 21:58:11.000000000 +0200
>>> +++ linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c 2010-06-04 23:00:35.000000000 +0200
>>> @@ -1100,31 +1100,26 @@
>>> *
>>> * @return pointer to descriptor on success, NULL on error.
>>> */
>>> -struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
>>> +
>>> +struct smscore_buffer_t *get_entry(void)
>>> {
>>> struct smscore_buffer_t *cb = NULL;
>>> unsigned long flags;
>>>
>>> - DEFINE_WAIT(wait);
>>> -
>>> spin_lock_irqsave(&coredev->bufferslock, flags);
>>
>> Sorry, maybe I'm just blind, but where is 'coredev' defined in this
>> scope? You probably forgot to pass it to get_entry?
>>
>> How could this be compiled? Is there coredev defined globally?
>
> good catch. I think it failed and despite a different kernel id the old module was
> loaded.
>
> Here is the new version, this time lightly tested
Could you please fix the indentation and send your SOB for this patch?
Cheers,
Mauro.
>
> --- linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c.rz 2010-06-03 21:58:11.000000000 +0200
> +++ linux-2.6.34/drivers/media/dvb/siano/smscoreapi.c 2010-06-07 14:32:06.000000000 +0200
> @@ -1100,31 +1100,26 @@
> *
> * @return pointer to descriptor on success, NULL on error.
> */
> -struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
> +
> +struct smscore_buffer_t *get_entry(struct smscore_device_t *coredev)
> {
> struct smscore_buffer_t *cb = NULL;
> unsigned long flags;
>
> - DEFINE_WAIT(wait);
> -
> spin_lock_irqsave(&coredev->bufferslock, flags);
> -
> - /* This function must return a valid buffer, since the buffer list is
> - * finite, we check that there is an available buffer, if not, we wait
> - * until such buffer become available.
> - */
> -
> - prepare_to_wait(&coredev->buffer_mng_waitq, &wait, TASK_INTERRUPTIBLE);
> -
> - if (list_empty(&coredev->buffers))
> - schedule();
> -
> - finish_wait(&coredev->buffer_mng_waitq, &wait);
> -
> + if (!list_empty(&coredev->buffers)) {
> cb = (struct smscore_buffer_t *) coredev->buffers.next;
> list_del(&cb->entry);
> -
> + }
> spin_unlock_irqrestore(&coredev->bufferslock, flags);
> + return cb;
> +}
> +
> +struct smscore_buffer_t *smscore_getbuffer(struct smscore_device_t *coredev)
> +{
> + struct smscore_buffer_t *cb = NULL;
> +
> + wait_event(coredev->buffer_mng_waitq, (cb = get_entry(coredev)));
>
> return cb;
> }
>
>
> Richard
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html