On 16-08-24 09:41 AM, Eric Dumazet wrote:
> On Tue, 2016-08-23 at 13:24 -0700, John Fastabend wrote:
>> Enable dflt qdisc support for per cpu stats before this patch a
>> dflt qdisc was required to use the global statistics qstats and
>> bstats.
>>
>> Signed-off-by: John Fastabend <john.r.fastab...@intel.com>
>> ---
>> net/sched/sch_generic.c | 24 ++++++++++++++++++++----
>> 1 file changed, 20 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
>> index 80544c2..910b4d15 100644
>> --- a/net/sched/sch_generic.c
>> +++ b/net/sched/sch_generic.c
>> @@ -646,18 +646,34 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue
>> *dev_queue,
>> struct Qdisc *sch;
>>
>> if (!try_module_get(ops->owner))
>> - goto errout;
>> + return NULL;
>>
>> sch = qdisc_alloc(dev_queue, ops);
>> if (IS_ERR(sch))
>> - goto errout;
>> + return NULL;
>> sch->parent = parentid;
>>
>> - if (!ops->init || ops->init(sch, NULL) == 0)
>> + if (!ops->init)
>> return sch;
>>
>> - qdisc_destroy(sch);
>> + if (ops->init(sch, NULL))
>> + goto errout;
>> +
>> + /* init() may have set percpu flags so init data structures */
>> + if (qdisc_is_percpu_stats(sch)) {
>> + sch->cpu_bstats =
>> + netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
>> + if (!sch->cpu_bstats)
>> + goto errout;
>> +
>> + sch->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
>> + if (!sch->cpu_qstats)
>> + goto errout;
>> + }
>> +
>
> Why are you attempting these allocations here instead of qdisc_alloc()
>
> This looks weird, I would expect base qdisc being fully allocated before
> ops->init() is attempted.
>
>
>
I could fully allocate it in qdisc_alloc() but we don't know if the
qdisc needs per cpu data structures until after the init call. So it
would sit unused in those cases if done from qdisc_alloc(). It seems
best to me at least to just avoid the allocation in qdisc_alloc() and
do it after init like I did here.
Perhaps it would be nice to pull these into a function call
post_init_qdisc_alloc() that does all this allocation?
.John
