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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 76D56C4345F for ; Tue, 30 Apr 2024 18:22:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=8IFhPj7DIippGq7U3GQ3KEs/pbeKBLLraKavhBTf2eY=; b=LFzNhDwHvQ0LwA HIt3jF6ts1KWDkUyKtNr78Rti1bfxLd7To2tE2uzYOCu2HStMqSjv420Pbt7vgGzbXW9TH64Bn4nS lJuirNX4tJpxS6PHPXfV9r++8TVS4ZrxHM1m7qEH/uQeiOMqjkNBytjdjLQdX4xXcGSTzNwNJlgsK oukzAcxVbI/SVscOuRCDEQqpE5zzn7oDqwy90OzyxuGi3aJK68K3s4WfKCNxb/aytqlgchDD6Jrtb p6dQkOuB6G6C8w1+QWX00nXGIyKA2tlR2sFwhbBMtNJBFvS11dkKnr2/Be+kyKnbIQ666N0Q0aMMY HiYmBy8+CIZk6JG8YUEg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s1s70-00000007ZJm-3ZAn; Tue, 30 Apr 2024 18:21:54 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s1s6x-00000007ZIz-0mlk for linux-arm-kernel@lists.infradead.org; Tue, 30 Apr 2024 18:21:52 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id DB1D4CE114D; Tue, 30 Apr 2024 18:21:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6C8DEC2BBFC; Tue, 30 Apr 2024 18:21:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1714501308; bh=e8R5pNN2IsLACAwBDVyNGSn3zTMo3zNZ2dNsVU58FAY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=uoKFBS0oWnJcn5AdrjfuWyIkjg2MAAkA4VXWGlkuJpeO3isRw7tzGSQCr8k0jK/s/ j51yGThqNL/UTuSjLfQuERbZIA6/RR462aK9UR49PvH14iimEeBtauE97B+Rn144tA EAiYurlFqXopiuoOtIr/WqoIvx6xgAlvAwyXQMZ2KQ28kmbdzS+VpG+/c8D16SA84U Phw2RgTDYvjJWtggVTwYSUYuxKUtKuhAez8zUnAGm04qEp1b2tbX8zNIhvFkprsKnP I6ZydNCTiPdlowBbxVR8j0AXv5ugZVa2/YciVsAusdd5mKFFpEbFm/y0TZt//24ZDs 1tNh0FQNdrhDQ== Date: Tue, 30 Apr 2024 19:21:42 +0100 From: Simon Horman To: MD Danish Anwar Cc: Dan Carpenter , Heiner Kallweit , Andrew Lunn , Jan Kiszka , Diogo Ivo , Paolo Abeni , Jakub Kicinski , Eric Dumazet , "David S. Miller" , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org, srk@ti.com, Vignesh Raghavendra , r-gunasekaran@ti.com, Roger Quadros Subject: Re: [PATCH net-next v2] net: ti: icssg_prueth: Add SW TX / RX Coalescing based on hrtimers Message-ID: <20240430182142.GC2575892@kernel.org> References: <20240429071501.547680-1-danishanwar@ti.com> <20240429183034.GG516117@kernel.org> <5b7cf22a-ca91-4975-bd26-c76a16781ad7@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5b7cf22a-ca91-4975-bd26-c76a16781ad7@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240430_112151_784475_42D9BDF8 X-CRM114-Status: GOOD ( 24.32 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Apr 30, 2024 at 03:12:58PM +0530, MD Danish Anwar wrote: > Simon, > > On 30/04/24 12:00 am, Simon Horman wrote: > > On Mon, Apr 29, 2024 at 12:45:01PM +0530, MD Danish Anwar wrote: > >> Add SW IRQ coalescing based on hrtimers for RX and TX data path for ICSSG > >> driver, which can be enabled by ethtool commands: > >> > >> - RX coalescing > >> ethtool -C eth1 rx-usecs 50 > >> > >> - TX coalescing can be enabled per TX queue > >> > >> - by default enables coalesing for TX0 > > > > nit: coalescing > > > > Please consider running patches through ./checkpatch --codespell > > > >> ethtool -C eth1 tx-usecs 50 > >> - configure TX0 > >> ethtool -Q eth0 queue_mask 1 --coalesce tx-usecs 100 > >> - configure TX1 > >> ethtool -Q eth0 queue_mask 2 --coalesce tx-usecs 100 > >> - configure TX0 and TX1 > >> ethtool -Q eth0 queue_mask 3 --coalesce tx-usecs 100 --coalesce > >> tx-usecs 100 > >> > >> Minimum value for both rx-usecs and tx-usecs is 20us. > >> > >> Compared to gro_flush_timeout and napi_defer_hard_irqs this patch allows > >> to enable IRQ coalescing for RX path separately. > >> > >> Benchmarking numbers: > >> =============================================================== > >> | Method | Tput_TX | CPU_TX | Tput_RX | CPU_RX | > >> | ============================================================== > >> | Default Driver 943 Mbps 31% 517 Mbps 38% | > >> | IRQ Coalescing (Patch) 943 Mbps 28% 518 Mbps 25% | > >> =============================================================== > >> > >> Signed-off-by: MD Danish Anwar > >> --- > > [ ... ] > >> if (num_tx_packets >= budget) > >> return budget; > >> > >> - if (napi_complete_done(napi_tx, num_tx_packets)) > >> - enable_irq(tx_chn->irq); > >> + if (napi_complete_done(napi_tx, num_tx_packets)) { > >> + if (unlikely(tx_chn->tx_pace_timeout_ns && !tdown)) { > >> + hrtimer_start(&tx_chn->tx_hrtimer, > >> + ns_to_ktime(tx_chn->tx_pace_timeout_ns), > >> + HRTIMER_MODE_REL_PINNED); > >> + } else { > >> + enable_irq(tx_chn->irq); > >> + } > > > > This compiles with gcc-13 and clang-18 W=1 > > (although the inner {} are unnecessary). > > > >> + } > >> > >> return num_tx_packets; > >> } > > > > ... > > > >> @@ -872,7 +894,13 @@ int emac_napi_rx_poll(struct napi_struct *napi_rx, int budget) > >> } > >> > >> if (num_rx < budget && napi_complete_done(napi_rx, num_rx)) > >> - enable_irq(emac->rx_chns.irq[rx_flow]); > >> + if (unlikely(emac->rx_pace_timeout_ns)) { > >> + hrtimer_start(&emac->rx_hrtimer, > >> + ns_to_ktime(emac->rx_pace_timeout_ns), > >> + HRTIMER_MODE_REL_PINNED); > >> + } else { > >> + enable_irq(emac->rx_chns.irq[rx_flow]); > >> + } > > > > But this does not; I think outer (but not inner) {} are needed. > > > > For both of these if checks, by having {} for outer if I am not seeing > the warnings anymore. The braces don't seem to be neccessary for inner if. > > For both of these ifs I'll keep both inner and outer ifs in braces as > this will help readablity as Dan pointed out. > > I will post v3 with this change. Thanks, sounds good to me. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel