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=-6.8 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,SIGNED_OFF_BY, 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 BBFFEC43381 for ; Thu, 21 Mar 2019 14:42:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 88113218B0 for ; Thu, 21 Mar 2019 14:42:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="fmluatoS" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728439AbfCUOmw (ORCPT ); Thu, 21 Mar 2019 10:42:52 -0400 Received: from mail-wm1-f66.google.com ([209.85.128.66]:40906 "EHLO mail-wm1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726551AbfCUOmw (ORCPT ); Thu, 21 Mar 2019 10:42:52 -0400 Received: by mail-wm1-f66.google.com with SMTP id u10so3016370wmj.5 for ; Thu, 21 Mar 2019 07:42:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=OW7Lp9gg/msdbf3n/DcYaUymqJvSZe2M96u3rSH82W4=; b=fmluatoSWuNarNKOqCKsVZYt4A65+IUvWDUVhIgKiDqxKGZLI2DVfJcY24IJabPFBJ nV5YyhcSNUTpfePe4qJmcE9/zltJ8gZP29s/HwpYUM6tqOoq0r1KmsUggs2OfN4M/RWi xqq62L5FV77pPi0DgAYOMxx3Z0qfrnEuWYkTniqSKemDAU3Wq69OdeHvNlHpKHuH93L7 k+3CkXPmOGm1kcL+MIMhNCMmeYp8JgU8UD8HtaKzd3uLDy30sj+DeSD5TlMypGVG6pBC P32SWl27G2mCKfZK51qWT3GDQ9IAqLSPVk+PqYJmQ5ySCVVXn739hFah7v4eLmK4mofv 1yhg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=OW7Lp9gg/msdbf3n/DcYaUymqJvSZe2M96u3rSH82W4=; b=LUQmdsyqfEWTV7wEUretG1dc6z3dhG5RWSovVNpyv7WJYi0yv+OkB8KB0EwdkuWvmY R+sevh2lN4NAKAJZECFEURQrRsRLJ1yHJwVsE9fOPOdSLBJEHi2YNy312nvmuWFFmLwl kB/oPaESQSMxOg6lhYd8561P8AkuJpAfjYRrCd7GubFNerWzJczpC6aYqXB2aD3lF8VQ LBnhQyrzmvpbSOFVK4vlkQy0craGnGOjBhym8VUBlv8Jp1S6S590OiX8JbpWT46f6hL3 HqvndXDHLdpbm6G993R08jaXeHwweKMAXlWS3FKF7YJ2zNa56Zola27m50sDyjptsUk2 ICew== X-Gm-Message-State: APjAAAWwOwztgrMmB9sCYn6+mrZ/lzM6k0F9Y9wpxP1jDvXvlk25SWbz N0rh7HaatZXt1dZJnyTTHI/4Pgwh X-Google-Smtp-Source: APXvYqyghLKYiVrx7Maw68JZPYd8/1vwSW8xJzLu9kB+yZUP3/lDwB0QX4vW4AM9/JYys3rIEeG2uw== X-Received: by 2002:a7b:c923:: with SMTP id h3mr2746974wml.34.1553179370375; Thu, 21 Mar 2019 07:42:50 -0700 (PDT) Received: from [172.31.96.190] ([195.39.71.253]) by smtp.gmail.com with ESMTPSA id 7sm13497646wrc.81.2019.03.21.07.42.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 21 Mar 2019 07:42:49 -0700 (PDT) Subject: Re: [PATCH net-next 1/2] net: sched: add empty status flag for NOLOCK qdisc To: Paolo Abeni , netdev@vger.kernel.org Cc: "David S. Miller" , Eric Dumazet , John Fastabend , Ivan Vecera References: From: Eric Dumazet Message-ID: Date: Thu, 21 Mar 2019 07:42:48 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 03/21/2019 03:14 AM, Paolo Abeni wrote: > The queue is marked not empty after acquiring the seqlock, > and it's up to the NOLOCK qdisc clearing such flag on dequeue. > Since the empty status lays on the same cache-line of the > seqlock, it's always hot on cache during the updates. > > This makes the empty flag update a little bit loosy. Given > the lack of synchronization between enqueue and dequeue, this > is unavoidable. > Seems nice ! > Signed-off-by: Paolo Abeni > --- > include/net/sch_generic.h | 17 +++++++++++++++++ > net/sched/sch_generic.c | 2 ++ > 2 files changed, 19 insertions(+) > > diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h > index 31284c078d06..1dc0aeec5d39 100644 > --- a/include/net/sch_generic.h > +++ b/include/net/sch_generic.h > @@ -113,6 +113,9 @@ struct Qdisc { > > spinlock_t busylock ____cacheline_aligned_in_smp; > spinlock_t seqlock; > + > + /* for NOLOCK qdisc, true if there are enqueued skbs */ > + bool not_empty; > struct rcu_head rcu; > }; > > @@ -143,11 +146,25 @@ static inline bool qdisc_is_running(struct Qdisc *qdisc) > return (raw_read_seqcount(&qdisc->running) & 1) ? true : false; > } > > +static inline bool qdisc_is_empty(struct Qdisc *qdisc) > +{ > + if (qdisc->flags & TCQ_F_NOLOCK) > + return !qdisc->not_empty; double negations are not very readable IMO ... Why not using qdisc->empty instead ? > + return !qdisc->q.qlen; > +} > + > +/* for NOLOCK qdisc, caller must held seqlock */ > +static inline void qdisc_set_empty(struct Qdisc *qdisc) > +{ > + qdisc->not_empty = false; > +} > + I guess the helper is not really needed ? > static inline bool qdisc_run_begin(struct Qdisc *qdisc) > { > if (qdisc->flags & TCQ_F_NOLOCK) { > if (!spin_trylock(&qdisc->seqlock)) > return false; > + qdisc->not_empty = true; Not clear why you have a qdisc_set_empty() helper only for one case. > } else if (qdisc_is_running(qdisc)) { > return false; > } > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > index a117d9260558..3b03c6b9be1e 100644 > --- a/net/sched/sch_generic.c > +++ b/net/sched/sch_generic.c > @@ -671,6 +671,8 @@ static struct sk_buff *pfifo_fast_dequeue(struct Qdisc *qdisc) > qdisc_qstats_cpu_backlog_dec(qdisc, skb); > qdisc_bstats_cpu_update(qdisc, skb); > qdisc_qstats_atomic_qlen_dec(qdisc); > + } else { > + qdisc_set_empty(qdisc); > } > > return skb; >