RE: [net-next v2 1/1] tipc: avoid unnecessary copying of bundled messages
> -Original Message- > From: David Laight [mailto:david.lai...@aculab.com] > Sent: Monday, February 19, 2018 14:30 > To: Jon Maloy > Cc: netdev@vger.kernel.org; Mohan Krishna Ghanta Krishnamurthy > ; Tung Quang Nguyen > ; Hoang Huu Le > ; Canh Duc Luu > ; Ying Xue ; tipc- > discuss...@lists.sourceforge.net > Subject: RE: [net-next v2 1/1] tipc: avoid unnecessary copying of bundled > messages > > From: Jon Maloy > Date: Thu, 15 Feb 2018 14:14:37 +0100 > > > 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 plus a > > calculated fraction of the base buffer's overhead. This change > > eliminates the overaccounting and at least mitigates the occurrence of > > unnecessary buffer copying. > > Have you actually checked that copying the data when you extract the > messages isn't faster than cloning and trying to avoid the copy? > Copying at the point is probably cheaper because it leads to a simpler > message structure. Yes, that is probably what I'll end up doing, if copying is unavoidable anyway. ///jon > > David
RE: [net-next v2 1/1] tipc: avoid unnecessary copying of bundled messages
From: Jon Maloy Date: Thu, 15 Feb 2018 14:14:37 +0100 > 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 plus a > calculated fraction of the base buffer's overhead. This change > eliminates the overaccounting and at least mitigates the occurrence > of unnecessary buffer copying. Have you actually checked that copying the data when you extract the messages isn't faster than cloning and trying to avoid the copy? Copying at the point is probably cheaper because it leads to a simpler message structure. David
RE: [net-next v2 1/1] tipc: avoid unnecessary copying of bundled messages
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Friday, February 16, 2018 21:33 > To: Jon Maloy > Cc: netdev@vger.kernel.org; Mohan Krishna Ghanta Krishnamurthy > ; Tung Quang Nguyen > ; Hoang Huu Le > ; Canh Duc Luu > ; Ying Xue ; tipc- > discuss...@lists.sourceforge.net > Subject: Re: [net-next v2 1/1] tipc: avoid unnecessary copying of bundled > messages > > From: Jon Maloy > Date: Thu, 15 Feb 2018 14:14:37 +0100 > > > 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 plus a > > calculated fraction of the base buffer's overhead. This change > > eliminates the overaccounting and at least mitigates the occurrence of > > unnecessary buffer copying. > > > > Reported-by: Hoang Le > > Acked-by: Ying Xue > > Signed-off-by: Jon Maloy > > As I explained in my previous two emails, I don't think this method of > accounting is appropriate. > > All of your clones must use the same skb->truesize as the original SKB > because each and every one of them keeps the full buffer from being > liberated until they are released. I understand what you are saying, although I am not happy with its consequences in this case. I guess I will just leave it the way it is until I can come up with something smarter. ///jon
Re: [net-next v2 1/1] tipc: avoid unnecessary copying of bundled messages
From: Jon Maloy Date: Thu, 15 Feb 2018 14:14:37 +0100 > 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 plus a > calculated fraction of the base buffer's overhead. This change > eliminates the overaccounting and at least mitigates the occurrence > of unnecessary buffer copying. > > Reported-by: Hoang Le > Acked-by: Ying Xue > Signed-off-by: Jon Maloy As I explained in my previous two emails, I don't think this method of accounting is appropriate. All of your clones must use the same skb->truesize as the original SKB because each and every one of them keeps the full buffer from being liberated until they are released.
