wangchdo commented on PR #17573:
URL: https://github.com/apache/nuttx/pull/17573#issuecomment-3705451632

   @cederom  CI passed, could you please double check?
   
   @xiaoxiang781216 @Fix-Point
   I’ve addressed your comments—could you please take another look?
   
   I understand that your main concern with my previous implementation was 
about potential concurrency issues, or a possible violation of ownership 
invariants under SMP. This PR resolves those concerns using an approach that I 
believe is **reliable, simple, and efficient**. Below, I’ll explain the fix 
from a code-level perspective:
   
   1. In `hrtimer_process()`, I now copy the **hrtimer callback**, `expiration 
time`, and `argument` into local variables while holding the spinlock:
   
   
![img_v3_02ti_fc18d9f4-f5dd-4992-bd20-782b39ea245g](https://github.com/user-attachments/assets/dbe73bee-8da7-4afd-bf63-9f48d20e3ed1)
   
   2. Still in hrtimer_process(), the callback is invoked after the spinlock is 
released, using those local variables. This makes it safe for other cores to 
update the same hrtimer’s callback or expiration concurrently via 
`hrtimer_set()`, `hrtimer_cancel()`, or `hrtimer_start()`:
   
   
![img_v3_02ti_b4096580-0945-44f3-959c-6e254a350d6g](https://github.com/user-attachments/assets/67345bd8-9b21-4ac8-89e6-02863dfa36d6)
   
   3. In `hrtimer_cancel()`, `hrtimer->expired` is set to `UINT64_MAX` to 
explicitly mark the hrtimer as cancelled:
   
   
![img_v3_02ti_aa7370d2-2eb5-4cd4-8821-6b75795abbag](https://github.com/user-attachments/assets/662a03ad-6176-4380-8278-9772b3f46ba2)
   
   4 In `hrtimer_start()`, `hrtimer->expired` is updated directly with the new 
expiration value:
   
   
![img_v3_02ti_b6193a60-23f6-4e4b-a318-e067be9a99eg](https://github.com/user-attachments/assets/7d1f613e-a779-46ab-adcb-6e06ab579fc9)
   
   5. Also in `hrtimer_process()`, if period > 0, the hrtimer is only allowed 
to restart when `hrtimer->expired` has not been modified concurrently by other 
cores. Both `hrtimer_cancel() `(which sets expired to UINT64_MAX) and 
`hrtimer_start() `(which sets it to a new value) will cause this check to fail. 
This guarantees that the **hrtimer will not be restarted unexpectedly**:
   
   
![img_v3_02ti_b98e97a2-d688-4b1d-a072-86a4719be23g](https://github.com/user-attachments/assets/68e427a0-78ac-412e-b044-ec850056303b)
   
   6. Finally, in` hrtimer_process()`, `hrtimer->cpus `acts as a reference 
counter and is automatically incremented and decremented. This allows it to be 
safely used by `hrtimer_cancel_sync() `to ensure the hrtimer is fully quiesced 
before returning:
   
   
![img_v3_02ti_a2050e68-d10c-4298-868b-7977937ef5dg](https://github.com/user-attachments/assets/662cb092-5980-47be-9aa7-580b6f75b270)
   
![img_v3_02ti_8f12799e-ce5e-432f-8dec-1c50d96eeceg](https://github.com/user-attachments/assets/e128790e-74ef-4a4e-87f8-4ffbd000cbab)
   
   **Overall, this design ensures correct behavior under SMP while keeping the 
logic straightforward and efficient, and it integrates cleanly with the 
existing hrtimer model in NuttX.**
   
    
   
   
    


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to