On Fri, Jan 26, 2018 at 11:57:59AM +0800, Jason Wang 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?
>
> Thanks
I think it isn't. If you want to use resize *and* use unlocked variants,
you must lock all producers and consumers when resizing yourself.
--
MST
