netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Abeni <pabeni@redhat.com>
To: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>, Eric Dumazet <edumazet@google.com>,
	Ivan Vecera <ivecera@redhat.com>
Subject: [PATCH net-next v2 0/5] net: sched: move back qlen to per CPU accounting
Date: Wed, 10 Apr 2019 14:32:36 +0200	[thread overview]
Message-ID: <cover.1554892007.git.pabeni@redhat.com> (raw)

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

Paolo Abeni (5):
  net: caif: avoid using qdisc_qlen()
  net: sched: prefer qdisc_is_empty() over direct qlen access
  net: sched: always do stats accounting according to TCQ_F_CPUSTATS
  net: sched: when clearing NOLOCK, clear TCQ_F_CPUSTATS, too
  Revert: "net: sched: put back q.qlen into a single location"

 include/net/sch_generic.h | 80 ++++++++++++++++++++++++++++-----------
 net/caif/caif_dev.c       | 12 ++++--
 net/core/gen_stats.c      |  2 +
 net/sched/sch_api.c       | 15 +++++++-
 net/sched/sch_generic.c   | 67 +++++++++++---------------------
 5 files changed, 105 insertions(+), 71 deletions(-)

-- 
2.20.1


             reply	other threads:[~2019-04-10 12:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-10 12:32 Paolo Abeni [this message]
2019-04-10 12:32 ` [PATCH net-next v2 1/5] net: caif: avoid using qdisc_qlen() Paolo Abeni
2019-04-10 12:32 ` [PATCH net-next v2 2/5] net: sched: prefer qdisc_is_empty() over direct qlen access Paolo Abeni
2019-04-10 12:32 ` [PATCH net-next v2 3/5] net: sched: always do stats accounting according to TCQ_F_CPUSTATS Paolo Abeni
2019-04-10 12:32 ` [PATCH net-next v2 4/5] net: sched: when clearing NOLOCK, clear TCQ_F_CPUSTATS, too Paolo Abeni
2019-04-10 12:32 ` [PATCH net-next v2 5/5] Revert: "net: sched: put back q.qlen into a single location" Paolo Abeni
2019-04-10 19:26 ` [PATCH net-next v2 0/5] net: sched: move back qlen to per CPU accounting David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1554892007.git.pabeni@redhat.com \
    --to=pabeni@redhat.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=ivecera@redhat.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).