From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [net-next 1/1] tipc: avoid unnecessary copying of bundled messages Date: Wed, 14 Feb 2018 05:42:36 -0800 Message-ID: <1518615756.3715.198.camel@gmail.com> References: <1518612631-21171-1-git-send-email-jon.maloy@ericsson.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: mohan.krishna.ghanta.krishnamurthy@ericsson.com, tung.q.nguyen@dektech.com.au, hoang.h.le@dektech.com.au, canh.d.luu@dektech.com.au, ying.xue@windriver.com, tipc-discussion@lists.sourceforge.net To: Jon Maloy , davem@davemloft.net, netdev@vger.kernel.org Return-path: Received: from mail-pf0-f196.google.com ([209.85.192.196]:45857 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030202AbeBNNmj (ORCPT ); Wed, 14 Feb 2018 08:42:39 -0500 Received: by mail-pf0-f196.google.com with SMTP id w83so3939763pfi.12 for ; Wed, 14 Feb 2018 05:42:39 -0800 (PST) In-Reply-To: <1518612631-21171-1-git-send-email-jon.maloy@ericsson.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 2018-02-14 at 13:50 +0100, Jon Maloy wrote: > A received sk buffer may contain dozens of smaller 'bundled' messages > which after extraction go each in their own direction. > > Unfortunately, when we extract those messages using skb_clone() each > of the extracted buffers inherit the truesize value of the original > buffer. Apart from causing massive overaccounting of the base buffer's > memory, this often causes tipc_msg_validate() to come to the false > conclusion that the ratio truesize/datasize > 4, and perform an > unnecessary copying of the extracted buffer. > > We now fix this problem by explicitly correcting the truesize value of > the buffer clones to be the truesize of the clone itself. This change > eliminates both the overaccounting and the unnecessary buffer copying. > > Reported-by: Hoang Le > Acked-by: Ying Xue > Signed-off-by: Jon Maloy > --- > net/tipc/msg.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/tipc/msg.c b/net/tipc/msg.c > index 4e1c6f6..a368fa8 100644 > --- a/net/tipc/msg.c > +++ b/net/tipc/msg.c > @@ -434,6 +434,9 @@ bool tipc_msg_extract(struct sk_buff *skb, struct sk_buff **iskb, int *pos) > skb_pull(*iskb, offset); > imsz = msg_size(buf_msg(*iskb)); > skb_trim(*iskb, imsz); > + > + /* Scale extracted buffer's truesize to avoid double accounting */ > + (*iskb)->truesize = SKB_TRUESIZE(imsz); How do you guarantee that under accounting wont happen here ? Copying data to avoid OOM is not necessarily bad. TCP stack does this under stress (this is called collapsing), and this definitely can happen. skb_clone() will also clones frags, and you absolutely do not know what memory each frag can hold (that could be 64KB on arches with 64KB pages)