From: Simon Horman <horms@kernel•org>
To: Eric Dumazet <edumazet@google•com>
Cc: "David S. Miller" <davem@davemloft•net>,
Jakub Kicinski <kuba@kernel•org>, Paolo Abeni <pabeni@redhat•com>,
Jamal Hadi Salim <jhs@mojatatu•com>,
Cong Wang <xiyou.wangcong@gmail•com>,
Jiri Pirko <jiri@resnulli•us>,
netdev@vger•kernel.org, eric.dumazet@gmail•com
Subject: Re: [PATCH net-next 04/14] net_sched: sch_choke: implement lockless choke_dump()
Date: Wed, 17 Apr 2024 15:44:21 +0100 [thread overview]
Message-ID: <20240417144421.GY2320920@kernel.org> (raw)
In-Reply-To: <CANn89iJkEzcU1-8yJF6AvYUqiE1U8-4oUcLOe73EtV=sHHnjZw@mail.gmail.com>
On Wed, Apr 17, 2024 at 03:41:36PM +0200, Eric Dumazet wrote:
> On Wed, Apr 17, 2024 at 3:14 PM Simon Horman <horms@kernel•org> wrote:
> >
> > On Mon, Apr 15, 2024 at 01:20:44PM +0000, Eric Dumazet wrote:
> > > Instead of relying on RTNL, choke_dump() can use READ_ONCE()
> > > annotations, paired with WRITE_ONCE() ones in choke_change().
> > >
> > > Also use READ_ONCE(q->limit) in choke_enqueue() as the value
> > > could change from choke_change().
> >
> > Hi Eric,
> >
> > I'm wondering if you could expand on why q->limit needs this treatment
> > but not other fields, f.e. q->parms.qth_min (aka p->qth_min).
>
> Other fields got their WRITE_ONCE() in red_set_parms()
>
> + WRITE_ONCE(p->qth_min, qth_min << Wlog);
> + WRITE_ONCE(p->qth_max, qth_max << Wlog);
> + WRITE_ONCE(p->Wlog, Wlog);
> + WRITE_ONCE(p->Plog, Plog);
Thanks, understood.
I do wonder if other schedulers may need WRITE_ONCE() in
their enqueue() function. But I have not analysed this yet.
> > > Signed-off-by: Eric Dumazet <edumazet@google•com>
> > > ---
> > > include/net/red.h | 10 +++++-----
> > > net/sched/sch_choke.c | 23 ++++++++++++-----------
> > > 2 files changed, 17 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/include/net/red.h b/include/net/red.h
> >
> > ...
> >
> > > @@ -244,7 +244,7 @@ static inline void red_set_parms(struct red_parms *p,
> > > max_P = red_maxp(Plog);
> > > max_P *= delta; /* max_P = (qth_max - qth_min)/2^Plog */
> > > }
> > > - p->max_P = max_P;
> > > + WRITE_ONCE(p->max_P, max_P);
> > > max_p_delta = max_P / delta;
> > > max_p_delta = max(max_p_delta, 1U);
> > > p->max_P_reciprocal = reciprocal_value(max_p_delta);
> >
> > A little further down in this function p->Scell_log is set.
> > I think it also needs the WRITE_ONCE() treatment as it
> > is read in choke_dump().
> >
>
> I will squash in v2 the missing WRITE_ONCE(), thanks !
Likewise, thanks.
Your proposed update looks good to me.
> diff --git a/include/net/red.h b/include/net/red.h
> index 7daf7cf6130aeccf3d81a77600f4445759f174b7..802287d52c9e37e76ba9154539f511629e4b9780
> 100644
> --- a/include/net/red.h
> +++ b/include/net/red.h
> @@ -257,7 +257,7 @@ static inline void red_set_parms(struct red_parms *p,
> p->target_min = qth_min + 2*delta;
> p->target_max = qth_min + 3*delta;
>
> - p->Scell_log = Scell_log;
> + WRITE_ONCE(p->Scell_log, Scell_log);
> p->Scell_max = (255 << Scell_log);
>
> if (stab)
>
next prev parent reply other threads:[~2024-04-17 14:44 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-15 13:20 [PATCH net-next 00/14] net_sched: first series for RTNL-less qdisc dumps Eric Dumazet
2024-04-15 13:20 ` [PATCH net-next 01/14] net_sched: sch_fq: implement lockless fq_dump() Eric Dumazet
2024-04-16 18:19 ` Simon Horman
2024-04-16 18:33 ` Eric Dumazet
2024-04-17 8:45 ` Eric Dumazet
2024-04-17 9:00 ` Simon Horman
2024-04-17 9:02 ` Eric Dumazet
2024-04-17 9:23 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 02/14] net_sched: cake: implement lockless cake_dump() Eric Dumazet
2024-04-17 8:35 ` Simon Horman
2024-04-17 8:54 ` Eric Dumazet
2024-04-17 9:24 ` Simon Horman
2024-04-17 12:25 ` Toke Høiland-Jørgensen
2024-04-15 13:20 ` [PATCH net-next 03/14] net_sched: sch_cbs: implement lockless cbs_dump() Eric Dumazet
2024-04-17 9:27 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 04/14] net_sched: sch_choke: implement lockless choke_dump() Eric Dumazet
2024-04-17 13:14 ` Simon Horman
2024-04-17 13:41 ` Eric Dumazet
2024-04-17 14:44 ` Simon Horman [this message]
2024-04-15 13:20 ` [PATCH net-next 05/14] net_sched: sch_codel: implement lockless codel_dump() Eric Dumazet
2024-04-17 15:59 ` Simon Horman
2024-04-17 16:05 ` Eric Dumazet
2024-04-17 16:21 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 06/14] net_sched: sch_tfs: implement lockless etf_dump() Eric Dumazet
2024-04-17 16:27 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 07/14] net_sched: sch_ets: implement lockless ets_dump() Eric Dumazet
2024-04-17 16:54 ` Simon Horman
2024-04-17 17:08 ` Eric Dumazet
2024-04-17 17:17 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 08/14] net_sched: sch_fifo: implement lockless __fifo_dump() Eric Dumazet
2024-04-15 13:20 ` [PATCH net-next 09/14] net_sched: sch_fq_codel: implement lockless fq_codel_dump() Eric Dumazet
2024-04-17 17:07 ` Simon Horman
2024-04-17 17:14 ` Eric Dumazet
2024-04-17 17:22 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 10/14] net_sched: sch_fq_pie: implement lockless fq_pie_dump() Eric Dumazet
2024-04-17 17:13 ` Simon Horman
2024-04-17 17:15 ` Eric Dumazet
2024-04-17 17:23 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 11/14] net_sched: sch_hfsc: implement lockless accesses to q->defcls Eric Dumazet
2024-04-15 13:20 ` [PATCH net-next 12/14] net_sched: sch_hhf: implement lockless hhf_dump() Eric Dumazet
2024-04-17 17:26 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 13/14] net_sched: sch_pie: implement lockless pie_dump() Eric Dumazet
2024-04-17 17:28 ` Simon Horman
2024-04-15 13:20 ` [PATCH net-next 14/14] net_sched: sch_skbprio: implement lockless skbprio_dump() Eric Dumazet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240417144421.GY2320920@kernel.org \
--to=horms@kernel$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=jhs@mojatatu$(echo .)com \
--cc=jiri@resnulli$(echo .)us \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox