rpmsg_virtio.c: replace metal_cpu_yield to metal_yield #613
Conversation
wyr-7
force-pushed
the
replace_metal_cpu_yield
branch
2 times, most recently
from
fd55a2d
to
01ed340
Compare
There was a problem hiding this comment.
This PR seems valid to me. We should not have the CPU yield notion in OpenAMP. Waiting 1 ms is more generic, and seems enough.
Please rework your commit message to something more generic.
Commit proposal:
The CPU yield is not supported by all OSes/processors. If not supported, this causes
a busy loop that monopolizes the CPU. Replace it with metal_sleep_usec, which is
OS-dependent and allows the OS scheduler to switch to another thread.
3e2d89a
to
66b0ca4
Compare
There was a problem hiding this comment.
I disagree with this change. It overrides decisions made in
- lib/processor/cpu/hosted.h
- lib/processor/cpu/x86.h
- lib/processor/cpu/aarch64.h
which could surprise current users of the library.
I think changing the implementation of metal_cpu_yield() in lib/processor/generic/cpu.h to use metal_sleep_usec() would solve the same problem, with fewer surprises to our current users.
|
We discussed this with @edmooring. He is right; it is more flexible to manage this in libmetal. As
|
wyr-7
force-pushed
the
replace_metal_cpu_yield
branch
from
66b0ca4
to
75dc14f
Compare
Ok, add metal_yield() in pr:OpenAMP/libmetal#311, this pr is denpends on it. |
arnopo
changed the title
wyr-7
force-pushed
the
replace_metal_cpu_yield
branch
from
75dc14f
to
31aa1f9
Compare
The CPU yield is not supported by all OSes/processors. If not supported, this causes a busy loop that monopolizes the CPU. Replace it with the new metal_yield, it would be managed at the OS level and dispatched to metal_cpu_yield, metal_sleep_usec, or others. Signed-off-by: Yongrong Wang <[email protected]>
wyr-7
force-pushed
the
replace_metal_cpu_yield
branch
from
31aa1f9
to
600b25b
Compare
Ok, thanks, done. |
There was a problem hiding this comment.
LGTM to rebase after OpenAMP/libmetal#311 integration to pass the CI
The CPU yield is not supported by all OSes/processors. If not supported, this causes a busy loop that monopolizes the CPU. Replace it with the new metal_yield, it would be managed at the OS level and dispatched to metal_cpu_yield, metal_sleep_usec, or others.