Skip to content

修复时间溢出,内核提速,修复随机丢失任务 #10328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

htl5241
Copy link
Contributor

@htl5241 htl5241 commented May 27, 2025

拉取/合并请求描述:(PR description)

[

为什么提交这份PR (why to submit this PR)

你的解决方案是什么 (what is your solution)

请提供验证的bsp和config (provide the config and bsp)

  • BSP:
  • .config:
  • action:

]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到.github/workflows/bsp_buildings.yml 详细请参考链接BSP自查

@github-actions github-actions bot added the tools label May 27, 2025
@htl5241
Copy link
Contributor Author

htl5241 commented May 27, 2025

人那,快点通过主线,我在下个pr

@github-actions github-actions bot added the Kernel PR has src relate code label May 28, 2025
@aozima
Copy link
Member

aozima commented May 28, 2025

不同类型的PR,提交到不同的分支,然后分别发PR,这样更好些。

@supperthomas supperthomas requested a review from Copilot May 29, 2025 01:52
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds llvm-arm as a supported CMake platform, refines tuple-to-define string formatting in cmake.py, and replaces many manual rt_schedule() calls with wrap-aware tick deltas and inlined scheduler updates.

  • Extend CMake platform checks to include llvm-arm and sanitize tuple defines
  • Replace multiple rt_tick_get() uses with rt_tick_get_delta(), handling wrap-around
  • Refactor scheduler in scheduler_up.c to inline priority updates and minimize direct list operations

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/cmake.py Add llvm-arm to platform lists; sanitize tuple CPPDEFINES
src/timer.c Combine flag checks; use wrap-aware timeout comparison
src/thread.c Remove redundant rt_schedule() calls; use rt_tick_get_delta()
src/signal.c Eliminate spuriously re-scheduling in signal delivery
src/scheduler_up.c Inline priority update/get functions; remove duplicated scheduling logic
src/mempool.c Replace simple tick subtraction with rt_tick_get_delta(); remove in-place scheduling
src/ipc.c Remove ad-hoc rt_schedule() invocations across semaphores, mutexes, events, mailboxes, and message queues
Comments suppressed due to low confidence (4)

tools/cmake.py:189

  • [nitpick] Sanitizing tuple strings by brute-force replace can be fragile. Consider explicitly handling tuple or list defines, e.g. join elements with spaces when i is a sequence, and fallback to str(i) otherwise.
cm_file.write("\t-D" + str(i).replace("(", "").replace(")","\").replace(",", " ") + "\n")

src/thread.c:135

  • Removing the explicit rt_schedule() in _thread_exit may prevent the scheduler from switching away from an exited thread. Ensure that context switching still occurs via another mechanism after exit.
    rt_schedule();

src/mempool.c:329

  • The removed rt_schedule() after releasing the spinlock may prevent newly awakened threads from running. Verify that a scheduling point is still reached when a block is allocated or freed.
rt_schedule();

src/ipc.c:728

  • All explicit rt_schedule() calls were removed from semaphore release logic, potentially delaying wake-up of waiting threads. Confirm that the scheduler is triggered elsewhere or restore the scheduling call here.
if (need_schedule == RT_TRUE)

@@ -762,7 +758,7 @@ void rt_timer_check(void)
rt_tick_t next_timeout;

ret = _timer_list_next_timeout(_soft_timer_list, &next_timeout);
if ((ret == RT_EOK) && (next_timeout <= rt_tick_get()))
if ((ret == RT_EOK) && ((rt_tick_get() - next_timeout) < RT_TICK_MAX / 2))
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Using RT_TICK_MAX / 2 for wrap-around tolerance is a magic constant. Add a brief comment or create a helper function to clarify why half the range is used for timeout comparisons.

Suggested change
if ((ret == RT_EOK) && ((rt_tick_get() - next_timeout) < RT_TICK_MAX / 2))
if ((ret == RT_EOK) && ((rt_tick_get() - next_timeout) < rt_timer_wraparound_tolerance()))

Copilot uses AI. Check for mistakes.


*tick += inc_tick;
left_tick = *tick - cur_tick;
if (left_tick > target_tick)
Copy link
Preview

Copilot AI May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left_tick > target_tick compares leftover ticks to an absolute timeout, which may be incorrect after wrap-around. Revisit this condition to ensure it handles wrap correctly (e.g., compare to half the tick range or use rt_tick_get_delta).

Copilot uses AI. Check for mistakes.

@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

不同类型的PR,提交到不同的分支,然后分别发PR,这样更好些。

我没想吧kenel的pr,我只是同步到我自己的分支,不知道咋整个都pr了。

@aozima
Copy link
Member

aozima commented May 29, 2025

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。
不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。
你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

  • 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。
  • 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。
  • git push orgin master1:master --force-with-lease 用本地的master1,强行覆盖远端的master分支。

@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master分支。

这次不小心pr的这个 ,提升内核性能最低15%,修复丢失任务bug(一个任务 不停关闭新建,就有丢失的问题)。

@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master分支。

这次不小心pr的这个 ,提升内核性能最低15%,修复丢失任务bug(一个任务 不停关闭新建,就有丢失的问题)。

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master分支。

我已经验证稳定性,你可以拉下这个pr试试就知道了。

@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master分支。

我测试网络任务,之前 at32f407 100mh最大网速只能达到550kB cpu占用率90%多,打完我的补丁最高达640kB CPU占用%84.

3 similar comments
@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master分支。

我测试网络任务,之前 at32f407 100mh最大网速只能达到550kB cpu占用率90%多,打完我的补丁最高达640kB CPU占用%84.

@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master分支。

我测试网络任务,之前 at32f407 100mh最大网速只能达到550kB cpu占用率90%多,打完我的补丁最高达640kB CPU占用%84.

@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master分支。

我测试网络任务,之前 at32f407 100mh最大网速只能达到550kB cpu占用率90%多,打完我的补丁最高达640kB CPU占用%84.

@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

test.zip
测试丢失任务代码

@htl5241
Copy link
Contributor Author

htl5241 commented May 29, 2025

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master

需要多练习下git使用。

因为你本地就直接用的master,发PR最好创建1个新的分支。 不管是否用master,已PR过还没合并的分支,不是有必要的更新的话,就不要再向这个分支推送了。 你在此之上,还有后续开发,你推送新的分支名即可,

你现在这种情况,比较合理的操作是

* 把你更新了kernal之后的master,取个新的分支名,如master-kernal,是否要push随你。

* 在你想要PR的commit上面(也就是你本次PR的第1个commit),建立1个新的分支名,如master1。

* `git push orgin master1:master --force-with-lease` 用本地的master1,强行覆盖远端的master分支。

测试完就帮我把这个pr拉入主线吧。

@htl5241 htl5241 changed the title 修复cmake 的 tuple错误 与 llvm-arm编译错误 修复时间溢出,内核提速,修复随机丢失任务 May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kernel PR has src relate code tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants