-----Original Message----- From: Jakub Kicinski kuba@kernel.org Sent: Monday, June 30, 2025 5:14 PM To: Chia-Yu Chang (Nokia) chia-yu.chang@nokia-bell-labs.com Cc: alok.a.tiwari@oracle.com; pctammela@mojatatu.com; horms@kernel.org; donald.hunter@gmail.com; xandfury@gmail.com; netdev@vger.kernel.org; dave.taht@gmail.com; pabeni@redhat.com; jhs@mojatatu.com; stephen@networkplumber.org; xiyou.wangcong@gmail.com; jiri@resnulli.us; davem@davemloft.net; edumazet@google.com; andrew+netdev@lunn.ch; ast@fiberby.net; liuhangbin@gmail.com; shuah@kernel.org; linux-kselftest@vger.kernel.org; ij@kernel.org; ncardwell@google.com; Koen De Schepper (Nokia) koen.de_schepper@nokia-bell-labs.com; g.white@cablelabs.com; ingemar.s.johansson@ericsson.com; mirja.kuehlewind@ericsson.com; cheshire@apple.com; rs.ietf@gmx.at; Jason_Livingood@comcast.com; vidhi_goel@apple.com Subject: Re: [PATCH v20 net-next 1/6] sched: Struct definition and parsing of dualpi2 qdisc
CAUTION: This is an external email. Please be very careful when clicking links or opening attachments. See the URL nok.it/ext for additional information.
On Sun, 29 Jun 2025 22:49:24 +0000 Chia-Yu Chang (Nokia) wrote:
I don't get the reason for all these WRITE_ONCE()s. You lock the qdisc to make modifications, right? 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..
Thanks again for other comments again, and I will take actions in the next version.
As there is only one step marking in L-queue, so we still need two WRITE_ONCE even two attributes (one for threshold in packets and one in time) are used.
When applying the step marking, we need to know either the computation is based on the sojourn time or queue length.
Let me ask again - why do you use WRITE_ONCE() at all if the modification takes the qdisc lock? Which reader are you afraid of racing with?
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.
Because we need to know which kind of AQM is applied, either based on # of packets or sojourn time in the L-queue.
And the corresponding reader will be at the dualpi2_dump.