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=-7.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=no 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 7C522C2D0A3 for ; Wed, 4 Nov 2020 00:01:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 24A3C2074F for ; Wed, 4 Nov 2020 00:01:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604448101; bh=rNsRX9qw/7MQXiB05tFOfeqYO7iNiqFy9zeKQ9JUp4c=; h=Subject:From:To:Date:In-Reply-To:References:List-ID:From; b=vRQY+d1GICo9xBVDONUV90ixp1A6nlk7FTJc7HO6q4clJaj2mtfb3OpeHHmGyreei K7opjaZQ9Tn6IeUfmBDRQhlbmMFHxD1eH8yfxBhYncRWpZqqdDvCNhmmilZPtqSC9B NvxuisESormC3hoMRLxPOo85IrKWMVgcPwGy8tK0= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729526AbgKDABk (ORCPT ); Tue, 3 Nov 2020 19:01:40 -0500 Received: from mail.kernel.org ([198.145.29.99]:60770 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726709AbgKDABj (ORCPT ); Tue, 3 Nov 2020 19:01:39 -0500 Received: from sx1.lan (c-24-6-56-119.hsd1.ca.comcast.net [24.6.56.119]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 411DA20735; Wed, 4 Nov 2020 00:01:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1604448099; bh=rNsRX9qw/7MQXiB05tFOfeqYO7iNiqFy9zeKQ9JUp4c=; h=Subject:From:To:Date:In-Reply-To:References:From; b=Wz+zIIAlXXDQWkc2kPljTjcpQxQdcjeLuK6YZZA2MdJQE5NBr1fjKgJthd5eA/Xbc LUOgejCVgV/COh1MWGsg2J3OfZxEU8M/ZdsRdHZJ57zgru83nPkAYHNV8UsPMUvdmg Z0IbVwwCO2bGubHmMz+t6XgPZlQPrM1A2/6xGrNw= Message-ID: <02019e49d43ba71d86f9caed761dbfe64a77331b.camel@kernel.org> Subject: Re: [PATCH 3/4] gve: Rx Buffer Recycling From: Saeed Mahameed To: David Awogbemila , netdev@vger.kernel.org Date: Tue, 03 Nov 2020 16:01:38 -0800 In-Reply-To: <20201103174651.590586-4-awogbemila@google.com> References: <20201103174651.590586-1-awogbemila@google.com> <20201103174651.590586-4-awogbemila@google.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5 (3.36.5-1.fc32) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, 2020-11-03 at 09:46 -0800, David Awogbemila wrote: > This patch lets the driver reuse buffers that have been freed by the > networking stack. > > In the raw addressing case, this allows the driver avoid allocating > new > buffers. > In the qpl case, the driver can avoid copies. > > Signed-off-by: David Awogbemila > --- > drivers/net/ethernet/google/gve/gve.h | 10 +- > drivers/net/ethernet/google/gve/gve_rx.c | 194 +++++++++++++++---- > ---- > + if (len <= priv->rx_copybreak) { > + /* Just copy small packets */ > + skb = gve_rx_copy(dev, napi, page_info, len); > + u64_stats_update_begin(&rx->statss); > + rx->rx_copied_pkt++; > + rx->rx_copybreak_pkt++; > + u64_stats_update_end(&rx->statss); > + } else { > + bool can_flip = gve_rx_can_flip_buffers(dev); > + int recycle = 0; > + > + if (can_flip) { > + recycle = gve_rx_can_recycle_buffer(page_info- > >page); > + if (recycle < 0) { > + gve_schedule_reset(priv); How would a reset solve anything if your driver is handling pages with "bad" refcount, i don't agree here that reset is the best course of action, all you can do here is warn and leak the page ... this is a critical driver bug and not something that user should expect. > + > + } else { > + /* It is possible that the networking stack has > already > + * finished processing all outstanding packets > in the buffer > + * and it can be reused. > + * Flipping is unnecessary here - if the > networking stack still > + * owns half the page it is impossible to tell > which half. Either > + * the whole page is free or it needs to be > replaced. > + */ > + int recycle = > gve_rx_can_recycle_buffer(page_info->page); > + > + if (recycle < 0) { > + gve_schedule_reset(priv); > + return false; Same.