clock_timekeeping: remove enter_critical_section in sched/clock/clock_timekeeping.c

[hujun260]

Summary

clock_timekeeping: remove enter_critical_section in sched/clock/clock_timekeeping.c
reason:
We would like to replace the critical section with a small lock.

Impact

clock_timekeeping

Testing

ci

[nuttxpr]

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements. While it provides a summary and mentions testing, it lacks crucial details.

Here's what's missing:

• Summary: Needs more detail. Why do you want to replace the critical section with a lock? What problem does this solve? What specific changes are being made (e.g., "Replacing enter_critical_section() and leave_critical_section() with calls to nxmutex_take() and nxmutex_give() on a newly introduced mutex")?
• Impact: The current entry is insufficient. Address all the impact points with "YES" or "NO" and provide descriptions where necessary. Consider:
  • Impact on user: Likely no, but explain why.
  • Impact on build: Possibly no, but state explicitly.
  • Impact on hardware: Probably no, but state explicitly.
  • Impact on documentation: If no documentation changes are needed, state that explicitly.
  • Impact on security: Does using a mutex instead of a critical section have any security implications? (e.g., priority inversion).
  • Impact on compatibility: Are there any backward compatibility concerns?
• Testing: "ci" is not sufficient. Provide specific details about the build host (OS, CPU architecture, compiler) and the target platform (architecture, board, configuration). Include actual testing logs demonstrating the issue before the change and the improved behavior after the change. Just saying "works as intended" is not enough; show evidence.

The PR needs to be significantly expanded to meet the requirements. Provide the missing information to ensure a thorough review.

[anchao]

Suggested change

	ret = up_timer_gettick(&counter);
	ret = up_timer_gettick(&counter); <---

same issue with #15129, how to protect lower half data

[hujun260]

I believe there is no clear specification that requires up_timer_gettick to be called within a critical_section. Currently, there are numerous calls to up_timer_gettick that are not within a critical section. For example, clock_systime_ticks calls up_timer_gettick, but clock_systime_ticks itself is not within a critical_section in many places.

[anchao]

The key issue is that the scope of impact of your code before and after modification is different. You cannot guarantee that the modification will not bring side effects within the visible range. If you insist on making changes, please ignore my comments，I will not spend time reviewing similar submissions in the future.

[anchao]

ditto