On 06/06/14 18:15, Josselin Costanzi wrote:
2014-06-06 17:40 GMT+02:00 Lars-Peter Clausen <lars@xxxxxxxxxx>:
On 06/06/2014 05:12 PM, Josselin Costanzi wrote:
2014-06-06 16:38 GMT+02:00 Lars-Peter Clausen <lars@xxxxxxxxxx>:
On 06/06/2014 04:30 PM, Josselin Costanzi wrote:
Currently the IIO buffer blocking read only wait until at least one
data element is available.
But that is how it is supposed to work. With this patch for example
you
won't be able to read the last data from the buffer after the buffer
has
been disabled.
I don't understand the usecase this patch breaks... If the buffer is
disabled then we return what was read alreay.
If there is less data in the buffer than the amount of requested data
you'll
keep looping forever.
We are working with an accelerometer and the only trigger is periodic so
we
didn't think of that since we are sure we end up with enough data.
Is there a way to know when if the trigger is done generating events and
no
more data is expected?
It should be done when the sysfs write to turn the buffer off is
complete.
Otherwise, no - there is is general no way to know when some triggers
will
fire.
Even with the patch as is, we can use a signal to interrupt the read so
it's
possible for the userspace to set a timeout (this works with our
userspace,
on a SIGINT we correctly get the data acquired so far).
We could, but see below...
Or if for example n is not aligned to the sample size you'll also
continue to loop forever.
If n isn't aligned to the sample size, wouldn't iio_read_first_n_kfifo
still return data multiple of samples size? In that case we would copy
complete elements until we try to do a short read which would fail at
n < kfifo_esize(&kf->kf) (in iio_read_first_n_kfifo).
The read_first_n() callback will make sure that it only returns full
samples, which means if n is smaller than the sample size it will
return 0.
In fact it won't return 0 but -EINVAL then read returns the size of
the data already
copied (this also works with our userspace program)
Maybe start with which problem you are trying to address with this
patch and
then we can work forward from there to a solution. The current form of
the
patch changes the semantics of read() in a way they shouldn't be
changed, so
this is not only about the implementation bugs.
Our use case is we continuously get data from an accelerometer. The
sample
frequency is relatively low (< 1KHz) so the userland ends up making one
syscall
per sample when we would prefer to process the data by batches to reduce
the cpu
load.
We would like to have the blocking read() semantics where we wait for
the
data
until the buffer is full unless there is an exceptional condition as
it's usually done for
ttys and sockets.
Be careful with your examples. This simply isn't true for sockets. Try
sending
a large amount of date over a tcp connection using blocking reads. You
frequently
get less data than requested as it tends to return when individual
packets come
in. Note the man page for read makes it absolutely clear that partial
data may
be returned and if so the userspace program is responsible for reading
again
until it gets what it wants.
I think the patch doesn't break anything except for the case where the
trigger generates
a finite and unknown number of data samples and the userspace requests
more than this
amount of samples.
A very common situation...
But in this scenario, without the patch, a blocking read does the same
as a poll followed
by a non blocking read which is the standard way to get a variable
amount of data.
Sorry, but we simply aren't going to change the semantics of this as this
is a major userspace ABI change. What we have at the moment conforms
entirely to what is permitted by the semantics of read.
Now having said that, we have had a number of discussions of how to
support
watershed type events on the buffer over the years. I think the current
consensus would be to use one of the more obscure POLL types to indicate
that there was more than a specified amount of data in a buffer ready
to be read. The most recent thread was related specifically to hardware
buffers, but as I point out later in the discussion any interface should
work equally well with software buffers.
http://marc.info/?l=linux-iio&m=139939531422385&w=2
If/when this gets implemented the semantics (which will need a man page
of their own given the 'unusual' use of POLL_PRI) will be
poll (POLLIN, POLLRDNORM) - wait for some data.
read non blocking - read anything that is there.
read blocking - read what is there or wait until something is then read
that.
poll (POLLPRI, POLLRDBAND) - wait for a sysfs defined watershed level of
data
be available.
Now to actually do this requires some additional functions for the
buffers
interface, and an implementation within the kfifo buffer + any others
people are using. As it stands, kfifo has the ability to query how
many elements are unused which gets us at least close to what we want...
I've cc'd Srinivas as he may have made some progress on an interface
for this.
