On Mon, 30 Jun 2025 17:04:56 +0000 Chia-Yu Chang (Nokia) wrote:
This still needs 2 WRITE_ONCE even "step_thresh" (NLA_U32) and "step_in_packtes" (NLA_FLAG) are replaced with "step_pkt_thresh" (NLA_U32) and "step_time_thresh" (NLA_U32) - which was proposed in my another email.
If you don't understand the question - ask for clarifications :/
You are right.
Could you elaborate on the orignal comment "And the block under which I'm responding is performing two dependent writes, one to ->step_in_packets and the other to ->step_thresh a change which is definitely not atomic.."?
I don't see we access the same atomic variable multiple times in a single expression, the 2 WRITE_ONCE() are in different expressions.
And, in the last WRITE_ONCE(), what we access are local variables: "step_pkt" "step_th", will it create problem?
Not really a problem, but what I'm saying is that I don't understand why all the writes are sprinkled with WRITE_ONCE(). You take
sch_tree_lock(sch);
to block data path and the control path is under rtnl_lock. So why the WRITE_ONCE()? WRITE_ONCE() is used to annotate writes which can be read concurrently without holding relevant locks.