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.
