On 06/07/2014 12:18 PM, Jonathan Cameron wrote:
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.
I'm not sure if we need to or should overload the POLLPRI/POLLRDBAND
semantics with a IIO specific meaning. In my opinion introducing a watershed
level should be enough. E.g. if it is set to 0 you get the current behavior.
It should not be to hard to implement this, the important part is to make
sure to not wakeup the pollqueue each time something is added to the FIFO,
but only if the FIFO level rises above the threshold. And when the buffer is
disabled the threshold is ignored and the pollqueue is woken unconditionally
to make it possible to read any residue that is left in the FIFO.
It is probably also worth looking at how other frameworks handle this. (I
think ALSA pretty much does what I just described).
- 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
