Re: [RFC] vtunerc: virtual DVB device - is it ok to NACK driverbecause of worrying about possible misusage?

[HoP]

Hi Andreas

[...]

>>> You don't need to wait for write-only operations. Basically all demux
>>> ioctls are write-only. Since vtunerc is using dvb-core's software demux
>>> *locally*, errors for invalid arguments etc. will be returned as usual.
>>>
>>> What's left is one call to FE_SET_FRONTEND for each frequency to tune
>>> to, and one FE_READ_STATUS for each time the lock status is queried.
>>> Note that one may use FE_GET_EVENT instead of FE_READ_STATUS to get
>>> notified of status changes asynchronously if desired.
>>>
>>> Btw.: FE_SET_FRONTEND doesn't block either, because the driver callback
>>> is called from a dvb_frontend's *local* kernel thread.
>>
>> Still, vtunerc waits for write operations:
>>
>> http://code.google.com/p/vtuner/source/browse/vtunerc_proxyfe.c?repo=linux-driver#285
>>
>> No matter if they are read or write, all of them call this function:
>>
>> http://code.google.com/p/vtuner/source/browse/vtunerc_ctrldev.c?repo=linux-driver#390
>>
>> That has a wait_event inside that function, as everything is directed to
>> the userspace.
>
> Please, stop writing such bullshit! Just before the call to wait_event
> there's code to return from the function if waiting has not been requested.
>
>> This is probably the way Florian found to return the errors returned by
>> the ioctls. This driver is synchronous, with simplifies it, at the lack of
>> performance.
>
> The fix is easy: set the third parameter to 0. A DVB application doesn't
> need to know whether SET_VOLTAGE etc. succeeded or not, because it won't
> get any feedback from the switch. If tuning fails, it has to retry
> anyway, even if all ioctls returned 0.
>
>> Ok, the driver could be smarter than that, and some heuristics could be
>> added into it, in order to foresee the likely error code, returning it
>> in advance, and then implementing some asynchronous mechanism that would
>> handle the error later, but that would be complex and may still introduce
>> some bad behaviors.
>
> There's no need to handle errors that don't occur.
>
> Nobody said that the implementation of vtunerc was perfect. I've already
> listed many things that need to be changed in order to even consider it
> for merging.

Exactly that was an intention for my first RFC - to get driver reviewed by
kernel hackers and enhance/fixe it to prepare it for merging.

I'm going to address all your findings during Xmass, so hopefully I will
spam here again after New Year :-)

I very appreciate your POV which helped me stay remembering
it is not way to the wall.

Thanks

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