drivers/serial/serial.c: only adapt readv api to avoid block on secound read in readv()

[anchao]

Summary

1. drivers/serial/serial.c: only adapt readv api to avoid block on secound read in readv()

2. fs/vfs: skip uio logic if only 1 iovcnt in slot

to improve the performance

Signed-off-by: chao an [email protected]

Impact

N/A

Testing

sim/nsh

[nuttxpr]

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a summary of the changes, it lacks crucial information.

Here's a breakdown of what's missing:

• Insufficient Summary: While the commit messages are present, they don't explain why the revert is necessary. What problem did the original commit introduce? What benefits does reverting bring? What was the motivation for the second change related to uio initialization? This section needs to clearly articulate the rationale behind the changes.

• Missing Impact Assessment: Stating "N/A" for impact is insufficient. Even reverts can have impact. Consider:

  • Impact on user: Did the original commit introduce a user-facing change that is now being reverted? Will reverting break any existing user applications or scripts?
  • Impact on compatibility: Does the revert affect compatibility with any specific hardware or software?
  • Impact on performance: Could reverting impact performance, either positively or negatively?

• Inadequate Testing: Simply stating "sim/nsh" is not enough. What specific tests were run? Provide actual log output before and after the change. Demonstrate that the revert fixes the problem it was intended to fix, and that it doesn't introduce new regressions. Ideally, the tests should exercise the specific code paths affected by the change. If the original commit caused a bug, show the bug occurring before the revert and then show the fix after the revert. For the uio change, demonstrate scenarios where readv/writev are and are not used, and show the expected behavior in both cases.

In summary: The PR needs to provide a more thorough explanation of the why, a detailed impact assessment (even if it's minimal), and significantly more robust testing information including actual log output. Without these details, it's difficult to assess the correctness and impact of the changes.

[yamt]

does it actually improve the performance much? otherwise, i'm against it.
as far as we want to support readv/writev, it's simpler always to use iovecs.

the original plan was that eventually deprecate read/write in file_operations.
this change would make it impossible.

while nonblock trick might be ok for the serial driver, it is not so generic.
eg. it won't work for files which needs to preserve data boundaries or files
which needs to provide read/write atomicity.

[anchao]

does it actually improve the performance much? otherwise, i'm against it. as far as we want to support readv/writev, it's simpler always to use iovecs.

the original plan was that eventually deprecate read/write in file_operations. this change would make it impossible.

while nonblock trick might be ok for the serial driver, it is not so generic. eg. it won't work for files which needs to preserve data boundaries or files which needs to provide read/write atomicity.

This patch provides 2 optimizations

1. For scenarios that do not support writev/readv and iovcnt == 1, use simpler read and write operations. Not all read and write operations for vfs need to apply uio overhead
2. Serial port devices can maintain single-byte read and write, and uio is too complicated

The advantage of nuttx rtos is that which could run on a chip with a very low frequency (50MHz). On the critical path, we need to further reduce the interference of complex frameworks. You will find that many people in the community are working on inline function and simplifying the function call process. Each cycle is very important for MCU devices

[anchao]

Linux experts don't understand the hard real-time requirements, which is why key scheduling performance of nuttx is better than ThreadX.

[yamt]

Linux experts don't understand the hard real-time requirements, which is why key scheduling performance of nuttx is better than ThreadX.

i guess we should avoid talking too much bad about other projects in a forum like this.

[yamt]

does it actually improve the performance much? otherwise, i'm against it. as far as we want to support readv/writev, it's simpler always to use iovecs.
the original plan was that eventually deprecate read/write in file_operations. this change would make it impossible.
while nonblock trick might be ok for the serial driver, it is not so generic. eg. it won't work for files which needs to preserve data boundaries or files which needs to provide read/write atomicity.

This patch provides 2 optimizations

1. For scenarios that do not support writev/readv and iovcnt == 1, use simpler read and write operations. Not all read and write operations for vfs need to apply uio overhead

2. Serial port devices can maintain single-byte read and write, and uio is too complicated

The advantage of nuttx rtos is that which could run on a chip with a very low frequency (50MHz). On the critical path, we need to further reduce the interference of complex frameworks. You will find that many people in the community are working on inline function and simplifying the function call process. Each cycle is very important for MCU devices

correctness, simplicity, code size, maintainability, etc are also important.
i'm just not convinced this PR provides a better trade-off.
maybe you can convince me eg. by providing some numbers.

also, i'm not sure about your "too complicated" claim.
having a separate fast path seems to make it more complicated to me.

[anchao]

Linux experts don't understand the hard real-time requirements, which is why key scheduling performance of nuttx is better than ThreadX.

i guess we should avoid talking too much bad about other projects in a forum like this.

To be honest, this is also why it took 20 years for PREEMPT_RT to be merged into the master branch, and even then, PREEMPT_RT still fails to meet the requirements of determinism and hard real time system.

[anchao]

The advantage of nuttx rtos is that which could run on a chip with a very low frequency (50MHz). On the critical path, we need to further reduce the interference of complex frameworks. You will find that many people in the community are working on inline function and simplifying the function call process. Each cycle is very important for MCU devices

correctness, simplicity, code size, maintainability, etc are also important. i'm just not convinced this PR provides a better trade-off. maybe you can convince me eg. by providing some numbers.

also, i'm not sure about your "too complicated" claim. having a separate fast path seems to make it more complicated to me.

I don't have a device and time. I just analyzed the code path and found that the current read/write scenario has additional overhead (uio) compared to the previous one.

90% of the business code will not use interfaces like readv/writev. Why do everyone need to use uio to generate additional function calls?
(Even iov_iter in Linux read/write path is also unreasonable)

[yamt]

The advantage of nuttx rtos is that which could run on a chip with a very low frequency (50MHz). On the critical path, we need to further reduce the interference of complex frameworks. You will find that many people in the community are working on inline function and simplifying the function call process. Each cycle is very important for MCU devices

correctness, simplicity, code size, maintainability, etc are also important. i'm just not convinced this PR provides a better trade-off. maybe you can convince me eg. by providing some numbers.
also, i'm not sure about your "too complicated" claim. having a separate fast path seems to make it more complicated to me.

I don't have a device and time. I just analyzed the code path and found that the current read/write scenario has additional overhead (uio) compared to the previous one.

90% of the business code will not use interfaces like readv/writev.

while it might be true fro you business code, it depends.
some code always use readv/writev.

Why do everyone need to use uio to generate additional function calls? (Even iov_iter in Linux read/write path is also unreasonable)

to have uniform kernel api and eventually reduce code footprint.

[yamt]

Linux experts don't understand the hard real-time requirements, which is why key scheduling performance of nuttx is better than ThreadX.

i guess we should avoid talking too much bad about other projects in a forum like this.

To be honest, this is also why it took 20 years for PREEMPT_RT to be merged into the master branch, and even then, PREEMPT_RT still fails to meet the requirements of determinism and hard real time system.

just curious; are you using read/write in a hard-realtime logic?
how do you estimate the upper cost of a read/write call?
disable caches and count cpu-cycles?

[anchao]

The advantage of nuttx rtos is that which could run on a chip with a very low frequency (50MHz). On the critical path, we need to further reduce the interference of complex frameworks. You will find that many people in the community are working on inline function and simplifying the function call process. Each cycle is very important for MCU devices

correctness, simplicity, code size, maintainability, etc are also important. i'm just not convinced this PR provides a better trade-off. maybe you can convince me eg. by providing some numbers.
also, i'm not sure about your "too complicated" claim. having a separate fast path seems to make it more complicated to me.

I don't have a device and time. I just analyzed the code path and found that the current read/write scenario has additional overhead (uio) compared to the previous one.
90% of the business code will not use interfaces like readv/writev.

while it might be true fro you business code, it depends. some code always use readv/writev.

Why do everyone need to use uio to generate additional function calls? (Even iov_iter in Linux read/write path is also unreasonable)

to have uniform kernel api and eventually reduce code footprint.

Let's first discuss #15603 , which has better branch for write/read and writev/readv

[anchao]

Linux experts don't understand the hard real-time requirements, which is why key scheduling performance of nuttx is better than ThreadX.

i guess we should avoid talking too much bad about other projects in a forum like this.

To be honest, this is also why it took 20 years for PREEMPT_RT to be merged into the master branch, and even then, PREEMPT_RT still fails to meet the requirements of determinism and hard real time system.

just curious; are you using read/write in a hard-realtime logic? how do you estimate the upper cost of a read/write call? disable caches and count cpu-cycles?

Hard real-time CPUs do not use vfs, only debugging and diagnostics case.

[yamt]

Linux experts don't understand the hard real-time requirements, which is why key scheduling performance of nuttx is better than ThreadX.

i guess we should avoid talking too much bad about other projects in a forum like this.

To be honest, this is also why it took 20 years for PREEMPT_RT to be merged into the master branch, and even then, PREEMPT_RT still fails to meet the requirements of determinism and hard real time system.

just curious; are you using read/write in a hard-realtime logic? how do you estimate the upper cost of a read/write call? disable caches and count cpu-cycles?

Hard real-time CPUs do not use vfs, only debugging and diagnostics case.

ok. so, your comments about hard realtime are not directly related to this PR. what a relief.

[anchao]

Hard real-time CPUs do not use vfs, only debugging and diagnostics case.

ok. so, your comments about hard realtime are not directly related to this PR. what a relief.

No, we should try to reduce the CPU usage in debug mode as much as possible.

[yamt]

Hard real-time CPUs do not use vfs, only debugging and diagnostics case.

ok. so, your comments about hard realtime are not directly related to this PR. what a relief.

No, we should try to reduce the CPU usage in debug mode as much as possible.

it's about general performance improvement, not hard-realtime, isn't it?

[anchao]

No, we should try to reduce the CPU usage in debug mode as much as possible.

it's about general performance improvement, not hard-realtime, isn't it?

Real-time performance is the result of optimization of each subsystem

[yamt]

No, we should try to reduce the CPU usage in debug mode as much as possible.

it's about general performance improvement, not hard-realtime, isn't it?

Real-time performance is the result of optimization of each subsystem

ok. i guess we are just using different definitions of hard-realtime.

[xiaoxiang781216]

@yamt do you have more suggestion for this patch?

[Laczen]

I have seen that there is quite some discussion about the use of read/readv and write/writev. Why does NuttX use uio for readv/writev instead of the classic iovec and iovcnt ?

[yamt]

doesn't this break BS/DEL on iov boundaries?

[yamt]

I have seen that there is quite some discussion about the use of read/readv and write/writev. Why does NuttX use uio for readv/writev instead of the classic iovec and iovcnt ?

because IMO raw iovec array is quite complex to deal with correctly, especially when it needs to be done in every drivers.

[Laczen]

I have seen that there is quite some discussion about the use of read/readv and write/writev. Why does NuttX use uio for readv/writev instead of the classic iovec and iovcnt ?

because IMO raw iovec array is quite complex to deal with correctly, especially when it needs to be done in every drivers.

I can't understand why it is complex to deal with raw iovec correctly.

But I can imagine that there are drivers where it is easier/better to write readv/writev on top of read/write, while there are others where it is easier/better to write read/write on top of readv/writev.

Maybe all drivers should provide read/write AND readv/writev and the driver implementation can decide what the best approach is to provide the functionality.