On Tue, 1 Jul 2025 05:41:53 +0000 Chia-Yu Chang (Nokia) wrote:
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.
This follows series https://lore.kernel.org/netdev/20240415132054.3822230-1-edumazet@google.com/ to use READ_ONCE in dual2_dump() and WRITE_ONCE in dualpi2_change.
And because of one previous chnage request (https://lore.kernel.org/all/26f2f366-aa14-4879-978a-58d46f9d83a4@redhat.com/) to split single dualpi2.c into 3 patches, so you can find them in the 2nd patch of this series.
What I can do is either move these WRITE_ONCE also to the next patch or merge these patches.
I would prefer the first approach to make it still 3 patches, any other suggestion?
I see. With the change to use two separate fields for packets and time I think this should be fine. We will hopefully report one or the other, but not one misinterpreted as the other.