Re: [PATCH v20 net-next 1/6] sched: Struct definition and parsing of dualpi2 qdisc

[Jakub Kicinski <kuba@kernel.org>]

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.