Dear All,
Thanks to your comments, I found out that the function in my kernel
iio_buffer_read_first_n_outer is different to the one that you
mentioned.
I was debugging a kernel version 3.10, that's include another
implementation of the function that we are talking. After some extra
effort, I could update to version 3.14 and effectively, the behavior
is as expected.
Very sorry for this misunderstanding.
BTW, in case of future problems, would be fine
Thanks
Julio
kernel 3.10:
---------------
ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
size_t n, loff_t *f_ps)
{
struct iio_dev *indio_dev = filp->private_data;
struct iio_buffer *rb = indio_dev->buffer;
i if (!rb || !rb->access->read_first_n)
return -EINVAL;
return rb->access->read_first_n(rb, n, buf);
}
kernel 3.14:
---------------
ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf,
size_t n, loff_t *f_ps)
{
struct iio_dev *indio_dev = filp->private_data;
struct iio_buffer *rb = indio_dev->buffer;
int ret;
if (!indio_dev->info)
return -ENODEV;
if (!rb || !rb->access->read_first_n)
return -EINVAL;
do {
if (!iio_buffer_data_available(rb)) {
if (filp->f_flags & O_NONBLOCK)
return -EAGAIN;
ret = wait_event_interruptible(rb->pollq,
iio_buffer_data_available(rb) ||
indio_dev->info == NULL);
if (ret)
return ret;
if (indio_dev->info == NULL)
return -ENODEV;
}
ret = rb->access->read_first_n(rb, n, buf);
if (ret == 0 && (filp->f_flags & O_NONBLOCK))
ret = -EAGAIN;
} while (ret == 0);
return ret;
}
On Tue, Jan 5, 2016 at 2:22 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On 04/01/16 12:46, Lars-Peter Clausen wrote:
>> On 01/04/2016 12:34 PM, Jonathan Cameron wrote:
>>> On 04/01/16 04:59, Julio Cruz wrote:
>>>> Hi Jonathan,
>>>>
>>>> Previously, you help me about an issue related with data loss. You suggest
>>>> me to debug deep in the core elements. I will try to summarize the results
>>>> below for future reference.
>>>>
>>>> When there is not data available in the buffer (kfifo), and the application
>>>> try to read data (using "read" function), it return zero (0).
>>>>
>>>> If libiio will be used to read the data, there is a problem (detailed at
>>>> https://github.com/analogdevicesinc/libiio/issues/23). In brief, Paul
>>>> (pcercuei) suggest me that this issue must be manage by the driver, in this
>>>> case, return -EAGAIN when there is not data available [Resource temporarily
>>>> unavailable (POSIX.1)].
>>>>
>>>> After review the core elements as suggested, I changed the line (in
>>>> function iio_read_first_n_kfifo of kfifo_buf.c) as below:
>>>>
>>>> - return copied;
>>>> + return copied == 0 ? -EAGAIN: copied;
>>>>
>>>> Do you think will be OK like this?
>>> Hmm.. This is an interesting one (thanks for tracking it down)
>>>
>>> The man page for read indeed allows for this to occur.
>>>
>>> When attempting to read a file (other than a pipe or FIFO) that sup‐
>>> ports non-blocking reads and has no data currently available:
>>>
>>> * If O_NONBLOCK is set, read() shall return −1 and set errno to
>>> [EAGAIN].
>>>
>>>
>>> However the issue here is that this is an ABI change and there may
>>> unfortunately be code out there relying on it returning 0.
>>
>> We never propagate 0 to userspace though. The referenced function is
>> iio_read_first_n_kfifo() which is an internal function. The function that
>> handles the userspace ABI is iio_buffer_read_first_n_outer() and here, as
>> Daniel pointed out, there are two things that can happen.
>>
>> We are in non-blocking mode and iio_read_first_n_kfifo() returns 0. In that
>> case we'll return -EAGAIN as mandated by the specification.
>>
>> We are in blocking mode and iio_read_first_n_kfifo() returns 0. In that case
>> we'll go back to waiting for more data and we'll only return if either data
>> was received or the application was interrupted by a signal. In the former
>> case we'll return the number of received bytes in the later case -ERESTARTSYS.
>>
>> So either way we should never return 0, something else must be going on.
>>
>>
>> Btw. letting iio_read_first_n_kfifo() return -EAGAIN will break blocking mode.
> That's what I get for thinking I remembered how this code works ;)
> Completely forgot the outer function did anything non trivial.
>
> Thanks Daniel / Lars for picking up on this!
>
> Oops.
>
> Jonathan
>>
>> - Lars
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
