From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C1662C43381 for ; Thu, 28 Feb 2019 02:47:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 80BF32186A for ; Thu, 28 Feb 2019 02:47:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="g1Eov0za" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730577AbfB1CrA (ORCPT ); Wed, 27 Feb 2019 21:47:00 -0500 Received: from mail-pg1-f193.google.com ([209.85.215.193]:40440 "EHLO mail-pg1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730240AbfB1Cq7 (ORCPT ); Wed, 27 Feb 2019 21:46:59 -0500 Received: by mail-pg1-f193.google.com with SMTP id u9so8929167pgo.7 for ; Wed, 27 Feb 2019 18:46:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=qTqLcdAmIe6Y20KB+njxsdkWTPS9qPaWeJpN/hHKAOY=; b=g1Eov0za5JhiCqX7Nt0WtsaIxMQd9/wMq3w3dRJBgQaXbVaHp0/ArFIpwiMmBwQY+u nIjcOmJ55Ok9XJSRgEBAgZNqMeWpkvLblXExeTZReW0mKJNGIjq6UopGApoLLjYDwOEU NsconwLpOD8wLjJxltunfhY8TbVJiRZxQnm61mj8U2khVljWrW8g4w50EAlD7wuVJcxo DJuFdCbioiJcNAVda+CZr//bso1+4MNfDmR6hkhsn03pSv520PX7FPNHAoEaNtT3F6MX s5RehZW99IZ4Wkl1GSnRRCj94HkHHRMIux0AsJIcC2c+1x85f3D8jDyD0YJj0OVb1ESo ocbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qTqLcdAmIe6Y20KB+njxsdkWTPS9qPaWeJpN/hHKAOY=; b=Fm9/1GJurA5DhE3SIAIdUqyqa3Sh3aFNMDK2q3JXd4ITIcznwcBKFAXKbfZ25saWGy vcehYrClv3EOWLrUQH/5QbDsy92GFMbcMZz0vvx/CfU1vTOKPOCyeoARnG1BJfZ+2ic4 D8X53Mtn33CnvZLYeIgbSH8MW/r1cRjQpxF9xctBan9ylYsrB/txP0PrYm3PUBvT4x6p ataHjDLmTIDViXl5sKRZbD/7on1/+DMw/B2iMaI2LBip48+tvI9qo7Y6dshNUIWoulMU ufiyeMvh39j3tIsZ89ZF26zJqKHK+eSmj7p0ioBfBbKmpukjNZlL3R2FhxxrvThS5vgP gPdA== X-Gm-Message-State: AHQUAuZ9/7FQ/c3WJpotdpKtcdeu7hCFLTFg2B51dT0PTEU8FUTLVU1S zXD1kFYhYKBFt1HX+X222P5KZFevWFRa0vrV4HI= X-Google-Smtp-Source: AHgI3IatAytDZdBc8gI49CbM5NRxcNDQ6fG5Lg3DwYNp+GlgyI4tbvyGeX+6MORqqKDz2c6WTex3dxHJIG6xfH9wzgE= X-Received: by 2002:a62:70c9:: with SMTP id l192mr5011610pfc.207.1551322018730; Wed, 27 Feb 2019 18:46:58 -0800 (PST) MIME-Version: 1.0 References: <5fb87db3-d5bd-714b-213d-6821f4bf37da@gmail.com> In-Reply-To: From: Cong Wang Date: Wed, 27 Feb 2019 18:46:47 -0800 Message-ID: Subject: Re: [BUG] net/sched : qlen can not really be per cpu ? To: Eric Dumazet Cc: John Fastabend , Networking , Jamal Hadi Salim , Jiri Pirko Content-Type: text/plain; charset="UTF-8" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, Feb 26, 2019 at 4:56 PM Eric Dumazet wrote: > > > > On 02/26/2019 03:51 PM, Cong Wang wrote: > > On Tue, Feb 26, 2019 at 3:19 PM Eric Dumazet wrote: > >> > >> > >> > >> On 02/25/2019 10:42 PM, Eric Dumazet wrote: > >>> HTB + pfifo_fast as a leaf qdisc hits badly the following warning in htb_activate() : > >>> > >>> WARN_ON(cl->level || !cl->leaf.q || !cl->leaf.q->q.qlen); > >>> > >>> This is because pfifo_fast does not update sch->q.qlen, but per cpu counters. > >>> So cl->leaf.q->q.qlen is zero. > >>> > >>> HFSC, CBQ, DRR, QFQ have the same problem. > >>> > >>> Any ideas how we can fix this ? > >> > >> What about something simple for stable ? > >> ( I yet have to boot/test this ) > > > > Is merely updating qlen sufficient for fixing it? > > > > I thought it is because of the lack of qdisc_tree_reduce_backlog() > > in pfifo_fast. > > It does not seem to be the qdisc_tree_reduce_backlog() thing. > > HTB, HFSC, CBQ, DRR, QFQ only peek at their children 'qlen' to decide if there > is at least one packet in them. > > The backlog is only reported for dumps, but the actual backlog value is not used in data path. > Hmm, looking into this, do we really need to check cl->leaf.q->q.qlen in htb_activate() for pfifo_fast? htb_activate() is only called when qdisc_enqueue() returns NET_XMIT_SUCCESS, so for pfifo_fast that is always qlen!=0, right? So something like below? It is ugly but should be sufficient to shut up the warning. diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 30f9da7e1076..6d0182750f8b 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c @@ -555,7 +555,8 @@ htb_change_class_mode(struct htb_sched *q, struct htb_class *cl, s64 *diff) */ static inline void htb_activate(struct htb_sched *q, struct htb_class *cl) { - WARN_ON(cl->level || !cl->leaf.q || !cl->leaf.q->q.qlen); + WARN_ON(cl->level || !cl->leaf.q || + (!(cl->leaf.q->flags & TCQ_F_NOLOCK) && !cl->leaf.q->q.qlen)); if (!cl->prio_activity) { cl->prio_activity = 1 << cl->prio;