kernel: pipe: should return if >= min_xfer bytes transferred and timeout is K_FOREVER

[cfriedt]

Describe the bug
The following code hangs indefinitely at k_pipe_get(), but it should not, according to the documentation:

unsigned char buf[64];
struct k_pipe p;
size_t bytes_written = 0;
size_t bytes_read = 0;
char rxbuf[32] = {};

k_pipe_init(&p, buf, sizeof(buf));

k_pipe_put(&p, "Hi!", 3, &bytes_written, 3 /* min_xfer */, K_FOREVER);
// wrote 3 bytes
printf("Wrote Hi!\n");

k_pipe_get(&p, rxbuf, sizeof(rxbuf), &bytes_read, 1 /* min_xfer */, K_FOREVER);
// bytes_read should equal 3
// k_pipe_get() should return immediately since min_xfer is 1
// but hangs indefinitely
printf("Read '%s'\n", rxbuf);

To Reproduce
Steps to reproduce the behavior:

1. mkdir build; cd build
2. cmake -DBOARD=qemu_cortex_m3
3. make
4. See error

Expected behavior

As per the documentation

Data can be synchronously received from a pipe by a thread. If the specified minimum number of bytes can not be immediately satisfied, then the operation will either fail immediately or attempt to receive as many bytes as possible and then pend in the hope that the receive can be completed later. Accepted data is either copied from the pipe’s ring buffer or directly from the waiting sender(s).

Since "the specified minimum number of bytes can be immediately satisfied", the call to k_pipe_get() should return immediately.

Impact
This is a showstopper for me if I would like to continue using the k_pipe API.

Screenshots or console output

Environment (please complete the following information):

• OS: Linux
• Toolchain: cross-compile
• Commit SHA or Version used: 2833d01

Additional context

I can make a PR. Have a patch for it already. I wonder though if k_pipe_put() has the same problem. Will investigate.

[cfriedt]

Yep. k_pipe_write() has the same problem. Will include a fix for that as well.

[pabigot]

While I agree the documentation could be interpreted the way you do, and that it might be useful if it worked that way, it seems the implementation respects "immediately satisfied" and "fail immediately" only in the case of K_NO_WAIT.

If your interpretation is to be supported I don't think your proposed solution in #24486 will work because at the point you discover that the minimum transfer hasn't been satisfied immediately some bytes may have been transferred, and they can't be clawed back, so failure will cause lost data.

I suspect a proper solution would be to modify pipe_xfer_prepare(), which checks for how much is available immediately but also has comments that are inconsistent with your desired interpretation. Additional modifications in the other functions will be required to complete early without pending when min_xfer is satisfied.

But first it has to be confirmed that this is a bug, rather than misleading documentation.

[cfriedt]

I guess technically, if K_NO_WAIT is specified, then the documentation is consistent.

However, I see no way to perform blocking I/O without entirely pre-determined data lengths. To be completely transparent, it is #24426 that is blocked.

[cfriedt]

I interpreted the API documentation to mean that if I called k_pipe_get() with min_xfer = 1 and timeout = K_FOREVER, then I would be able to mimic the same behaviour as read(2), and perform an actual blocking call that would return as soon as min_xfer was received or an error occurred.

Technically speaking, if an error occurs and some bytes have been read, the the number of bytes read is still available in bytes_read, so they don't need to necessarily be "clawed back".

[carlescufi]

@andyross could you take a look?

[andyross]

Yeah, that "min_xfer" is an ugly wart. But I think I agree with @cfriedt that it can only reasonably be interpreted as having higher precedence than the timeout. That is, what we want is:

"Here is some data (or a buffer to receive data), please transfer at least this many bytes of it, if necessary waiting this long to do it before returning"

But what we've implemented is:

"Here is some data (or buffer), please transfer all of it, if necessary waiting this long before returning, but as a special case it's OK to return early if you can synchronously move this smaller amount instead".

Yeah, we're wrong, even if you can squint and make us legal per our vague docs. The PR looks good, I'll put comments there.

[cfriedt]

Thanks for having a look @andyross @carlescufi