Skip to content

Comments

fix(std_common): handle NUM_READY_WORKER correctly#270

Merged
ytakano merged 4 commits intomainfrom
fix/std_properly_handle_NUM_READY_WORKER
Jan 14, 2025
Merged

fix(std_common): handle NUM_READY_WORKER correctly#270
ytakano merged 4 commits intomainfrom
fix/std_properly_handle_NUM_READY_WORKER

Conversation

@Koichi98
Copy link
Contributor

@Koichi98 Koichi98 commented Jan 12, 2025

Description

The following three points were fixed in kernel/src/main.rs because the shell would not start when executing std_common with make run-std:

  1. num_cpu() is only available after each CPU has called increment_num_cpu(). If we don't wait for all CPUs to increment, NUM_READY_WORKER underflows and the following while loop never terminates.
while NUM_READY_WORKER.load(Ordering::SeqCst) > 0 {
    awkernel_lib::delay::wait_microsec(10);
}

I added wait_microsec(100) before num_cpu() to avoid this. Ideally, we'd reference the CPU count obtained in each kernel/src/arch/ARCH/kernel_main.rs, but since this would require significant changes, I've decided not to include it in this PR.

  1. Non-primary CPUs should only decrement NUM_READY_WORKER after PRIMARY_READY becomes true in std-common, as it is in other architectures. Therefore, I modified std_common to also check that PRIMARY_READY is TRUE before decrementing.

  2. Replaced fetch_sub with fetch_update to prevent underflow when num_cpu is less than the actual number of CPU. Added a panic when NUM_READY_WORKER can't be decremented (already 0) as this indicates an unexpected state.

Related links

How was this PR tested?

Verified that the shell correctly enters the wait state by executing 'make run-std RELEASE=1' more than 10 times

Notes for reviewers

Signed-off-by: koichiimai <kotty.0704@gmail.com>
Signed-off-by: koichiimai <kotty.0704@gmail.com>
Signed-off-by: koichiimai <kotty.0704@gmail.com>
Signed-off-by: koichiimai <kotty.0704@gmail.com>
@Koichi98 Koichi98 requested a review from ytakano January 14, 2025 00:49
@ytakano ytakano merged commit 1f9f6a9 into main Jan 14, 2025
1 check passed
@Koichi98 Koichi98 deleted the fix/std_properly_handle_NUM_READY_WORKER branch January 14, 2025 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants