Re: [PATCH net-next 1/5] net/sched: taprio: don't overwrite "sch" variable in taprio_dump_class_stats()
From: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Date: 2023-05-30 21:15:15
Also in:
intel-wired-lan, lkml
Hi, Vladimir Oltean [off-list ref] writes:
In taprio_dump_class_stats() we don't need a reference to the root Qdisc once we get the reference to the child corresponding to this traffic class, so it's okay to overwrite "sch". But in a future patch we will need the root Qdisc too, so create a dedicated "child" pointer variable to hold the child reference. This also makes the code adhere to a more conventional coding style. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> ---
The patch looks good: Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com> But I have a suggestion, this "taprio_queue_get() -> dev_queue->qdisc_sleeping()" dance should have the same result as calling 'taprio_leaf()'. I am thinking of using taprio_leaf() here and in taprio_dump_class(). Could be a separate commit.
quoted hunk ↗ jump to hunk
net/sched/sch_taprio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c index 76db9a10ef50..d29e6785854d 100644 --- a/net/sched/sch_taprio.c +++ b/net/sched/sch_taprio.c@@ -2388,10 +2388,10 @@ static int taprio_dump_class_stats(struct Qdisc *sch, unsigned long cl, __acquires(d->lock) { struct netdev_queue *dev_queue = taprio_queue_get(sch, cl); + struct Qdisc *child = dev_queue->qdisc_sleeping; - sch = dev_queue->qdisc_sleeping; - if (gnet_stats_copy_basic(d, NULL, &sch->bstats, true) < 0 || - qdisc_qstats_copy(d, sch) < 0) + if (gnet_stats_copy_basic(d, NULL, &child->bstats, true) < 0 || + qdisc_qstats_copy(d, child) < 0) return -1; return 0; }-- 2.34.1
-- Vinicius