Skip to content

Commit 7bbde83

Browse files
jrfastabdavem330
authored andcommitted
net: sched: drop qdisc_reset from dev_graft_qdisc
In qdisc_graft_qdisc a "new" qdisc is attached and the 'qdisc_destroy' operation is called on the old qdisc. The destroy operation will wait a rcu grace period and call qdisc_rcu_free(). At which point gso_cpu_skb is free'd along with all stats so no need to zero stats and gso_cpu_skb from the graft operation itself. Further after dropping the qdisc locks we can not continue to call qdisc_reset before waiting an rcu grace period so that the qdisc is detached from all cpus. By removing the qdisc_reset() here we get the correct property of waiting an rcu grace period and letting the qdisc_destroy operation clean up the qdisc correctly. Note, a refcnt greater than 1 would cause the destroy operation to be aborted however if this ever happened the reference to the qdisc would be lost and we would have a memory leak. Signed-off-by: John Fastabend <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent a53851e commit 7bbde83

File tree

1 file changed

+19
-9
lines changed

1 file changed

+19
-9
lines changed

net/sched/sch_generic.c

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -819,10 +819,6 @@ struct Qdisc *dev_graft_qdisc(struct netdev_queue *dev_queue,
819819
root_lock = qdisc_lock(oqdisc);
820820
spin_lock_bh(root_lock);
821821

822-
/* Prune old scheduler */
823-
if (oqdisc && refcount_read(&oqdisc->refcnt) <= 1)
824-
qdisc_reset(oqdisc);
825-
826822
/* ... and graft new one */
827823
if (qdisc == NULL)
828824
qdisc = &noop_qdisc;
@@ -977,6 +973,16 @@ static bool some_qdisc_is_busy(struct net_device *dev)
977973
return false;
978974
}
979975

976+
static void dev_qdisc_reset(struct net_device *dev,
977+
struct netdev_queue *dev_queue,
978+
void *none)
979+
{
980+
struct Qdisc *qdisc = dev_queue->qdisc_sleeping;
981+
982+
if (qdisc)
983+
qdisc_reset(qdisc);
984+
}
985+
980986
/**
981987
* dev_deactivate_many - deactivate transmissions on several devices
982988
* @head: list of devices to deactivate
@@ -987,7 +993,6 @@ static bool some_qdisc_is_busy(struct net_device *dev)
987993
void dev_deactivate_many(struct list_head *head)
988994
{
989995
struct net_device *dev;
990-
bool sync_needed = false;
991996

992997
list_for_each_entry(dev, head, close_list) {
993998
netdev_for_each_tx_queue(dev, dev_deactivate_queue,
@@ -997,20 +1002,25 @@ void dev_deactivate_many(struct list_head *head)
9971002
&noop_qdisc);
9981003

9991004
dev_watchdog_down(dev);
1000-
sync_needed |= !dev->dismantle;
10011005
}
10021006

10031007
/* Wait for outstanding qdisc-less dev_queue_xmit calls.
10041008
* This is avoided if all devices are in dismantle phase :
10051009
* Caller will call synchronize_net() for us
10061010
*/
1007-
if (sync_needed)
1008-
synchronize_net();
1011+
synchronize_net();
10091012

10101013
/* Wait for outstanding qdisc_run calls. */
1011-
list_for_each_entry(dev, head, close_list)
1014+
list_for_each_entry(dev, head, close_list) {
10121015
while (some_qdisc_is_busy(dev))
10131016
yield();
1017+
/* The new qdisc is assigned at this point so we can safely
1018+
* unwind stale skb lists and qdisc statistics
1019+
*/
1020+
netdev_for_each_tx_queue(dev, dev_qdisc_reset, NULL);
1021+
if (dev_ingress_queue(dev))
1022+
dev_qdisc_reset(dev, dev_ingress_queue(dev), NULL);
1023+
}
10141024
}
10151025

10161026
void dev_deactivate(struct net_device *dev)

0 commit comments

Comments
 (0)