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/[email protected]/ 
> 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/[email protected]/)
>  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.

Reply via email to