Hi,
在 2025/09/03 10:55, Han Guangjiang 写道:
Hi Kuai,
Instead of add checking from hot path, do you consider delaying setting q->td
until policy is activated from the slow path? I think this is better solution.
Thank you for your review. You're absolutely right that performance
considerations in the hot path are important.
We actually considered delaying the setting of q->td until after policy
activation, but we found that q->td is needed by blkcg_activate_policy()
during its execution, so it has to be set before calling
blkcg_activate_policy().
That's not hard to bypass, q->td is used to initialze tg->td in
throtl_pd_init(), actually you can just remove it, and add a helper
tg_to_td() to replace it;
struct throtl_data *tg_to_td(struct throtl_grp *tg)
{
return tg_to_blkg(tg)->q->td;
}
Meanwhile, please remove the comment about freeze queue, turns out it
can't protect blk_throtl_bio() becasue q_usage_coutner is not grabbed
yet while issuing bio.
Thanks,
Kuai
We explored several alternative approaches:
1) Adding a dedicated flag like 'throttle_ready' to struct request_queue:
- Set this flag at the end of blk_throtl_init()
- Check this flag in blk_throtl_activated() to determine if policy
loading is complete
- However, this requires adding a new bool variable to the struct
2) Reusing the q->td pointer with low-order bit flags:
- Use pointer low-order bits to mark initialization completion status
- This would avoid adding new fields but requires careful handling
and additional processing
Given these constraints, we chose the current approach of checking the
policy bit in blk_throtl_activated() as it:
- Doesn't require struct changes
- Provides a clean, atomic check
- Aligns with the existing policy activation mechanism
We would appreciate your suggestions on how to better handle this
initialization race condition.
Thanks,
Han Guangjiang
.
