From: Paolo Abeni <pab...@redhat.com>
Date: Wed, 10 Apr 2019 14:32:36 +0200

> The commit 46b1c18f9deb ("net: sched: put back q.qlen into a single location")
> introduced some measurable regression in the contended scenarios for
> lock qdisc.
> 
> As Eric suggested we could replace q.qlen access with calls to 
> qdisc_is_empty()
> in the datapath and revert the above commit. The TC subsystem updates 
> qdisc->is_empty in a somewhat loose way: notably 'is_empty' is set only when
> the qdisc dequeue() calls return a NULL ptr. That is, the invocation after
> the last packet is dequeued.
> 
> The above is good enough for BYPASS implementation - the only downside is that
> we end up avoiding the optimization for a very small time-frame - but will
> break hard things when internal structures consistency for classful qdisc
> relies on child qdisc_is_empty().
> 
> A more strict 'is_empty' update adds a relevant complexity to its life-cycle, 
> so
> this series takes a different approach: we allow lockless qdisc to switch from
> per CPU accounting to global stats accounting when the NOLOCK bit is cleared.
> Since most pieces of infrastructure are already in place, this requires very
> little changes to the pfifo_fast qdisc, and any later NOLOCK qdisc can hook
> there with little effort - no need to maintain two different implementations.
> 
> The first 2 patches removes direct qlen access from non core TC code, the 3rd
> and 4th patches place and use the infrastructure to allow stats account
> switching and the 5th patch is the actual revert.
> 
>  v1 -> v2:
>   - fixed build issues
>   - more descriptive commit message for patch 5/5

I really like these changes, series applied, thanks Paolo.

Reply via email to