On Thu, Jan 25, 2018 at 7:57 PM, Jason Wang <jasow...@redhat.com> wrote: > > > On 2018年01月26日 10:26, Cong Wang wrote: >> >> pfifo_fast used to drop based on qdisc_dev(qdisc)->tx_queue_len, >> so we have to resize skb array when we change tx_queue_len. >> >> Other qdiscs which read tx_queue_len are fine because they >> all save it to sch->limit or somewhere else in qdisc during init. >> They don't have to implement this, it is nicer if they do so >> that users don't have to re-configure qdisc after changing >> tx_queue_len. >> >> Cc: John Fastabend <john.fastab...@gmail.com> >> Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com> >> --- >> net/sched/sch_generic.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c >> index 08f9fa27e06e..190570f21b20 100644 >> --- a/net/sched/sch_generic.c >> +++ b/net/sched/sch_generic.c >> @@ -763,6 +763,23 @@ static void pfifo_fast_destroy(struct Qdisc *sch) >> } >> } >> +static int pfifo_fast_change_tx_queue_len(struct Qdisc *sch, >> + unsigned int new_len) >> +{ >> + struct pfifo_fast_priv *priv = qdisc_priv(sch); >> + struct skb_array *bands[PFIFO_FAST_BANDS]; >> + int prio; >> + >> + for (prio = 0; prio < PFIFO_FAST_BANDS; prio++) { >> + struct skb_array *q = band2list(priv, prio); >> + >> + bands[prio] = q; >> + } >> + >> + return skb_array_resize_multiple(bands, PFIFO_FAST_BANDS, new_len, >> + GFP_KERNEL); >> +} >> + >> struct Qdisc_ops pfifo_fast_ops __read_mostly = { >> .id = "pfifo_fast", >> .priv_size = sizeof(struct pfifo_fast_priv), >> @@ -773,6 +790,7 @@ struct Qdisc_ops pfifo_fast_ops __read_mostly = { >> .destroy = pfifo_fast_destroy, >> .reset = pfifo_fast_reset, >> .dump = pfifo_fast_dump, >> + .change_tx_queue_len = pfifo_fast_change_tx_queue_len, >> .owner = THIS_MODULE, >> .static_flags = TCQ_F_NOLOCK | TCQ_F_CPUSTATS, >> }; > > > Is __skb_array_empty() in pfifo_fast_dequeue() still safe after this change?
Yes, we sync with dequeue path before calling ->change_tx_queue_len(). I already mentioned this in patch 2/3.