From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from shards.monkeyblade.net (shards.monkeyblade.net [184.105.139.130]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 5B5423F35C for ; Mon, 14 May 2018 11:06:02 -0400 (EDT) Received: from localhost (pool-173-77-163-54.nycmny.fios.verizon.net [173.77.163.54]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) (Authenticated sender: davem-davemloft) by shards.monkeyblade.net (Postfix) with ESMTPSA id 2FFB513CD6E23; Mon, 14 May 2018 08:06:01 -0700 (PDT) Date: Mon, 14 May 2018 11:05:58 -0400 (EDT) Message-Id: <20180514.110558.392362320880673775.davem@davemloft.net> To: toke@toke.dk Cc: netdev@vger.kernel.org, cake@lists.bufferbloat.net From: David Miller In-Reply-To: <87h8nawqnn.fsf@toke.dk> References: <152579005972.4805.2514975750552805066.stgit@alrua-kau> <20180513.200948.1658763899383204062.davem@davemloft.net> <87h8nawqnn.fsf@toke.dk> X-Mailer: Mew version 6.7 on Emacs 25.3 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Text/Plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.5.12 (shards.monkeyblade.net [149.20.54.216]); Mon, 14 May 2018 08:06:01 -0700 (PDT) Subject: Re: [Cake] [PATCH net-next v9 1/7] sched: Add Common Applications Kept Enhanced (cake) qdisc X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 May 2018 15:06:02 -0000 From: Toke H=F8iland-J=F8rgensen Date: Mon, 14 May 2018 11:08:28 +0200 > David Miller writes: > = >> From: Toke H=F8iland-J=F8rgensen >> Date: Tue, 08 May 2018 16:34:19 +0200 >> >>> +struct cake_flow { >>> + /* this stuff is all needed per-flow at dequeue time */ >>> + struct sk_buff *head; >>> + struct sk_buff *tail; >> >> Please do not invent your own SKB list handling mechanism. > = > We didn't invent it, we inherited it from fq_codel. I was actually ab= out > to fix that, but then I noticed it was still around in fq_codel, and = so > let it be. I can certainly fix it anyway, but, erm, why is it accepta= ble > in fq_codel but not in cake? struct sk_buff_head is not that new, is = it? I guess one argument has to do with the amount of memory consumed by th= is per-flow or per-queue information, right? Because the skb queue head h= as a qlen and a spinlock regardless of whether they are used or not. Furthermore, if you use the __skb_insert() et al. helpers, even though = it won't use the lock it will adjust the qlen counter. And that's useless= work since you have no use for the qlen value. Taken together, it seems that what you and fq_codel are doing is not such a bad idea after all. So please leave it alone. On-and-off again, I've looked into converting skbs to using list_head but it's a non-trivial set of work. All over the tree the different layers use the next/prev pointers in different ways. Some use it for a doubly linked list. Some use it for a singly linked list. Some encode state in the prev pointer. You name it, it's out there. I'll try to get back to that task because obviously it'll be useful to have code like cake and fq_codel use common helpers instead of custom stuff. Thanks.