lkml.org 
[lkml]   [2024]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net-next] net: ti: icssg_prueth: Add SW TX / RX Coalescing based on hrtimers
On Wed, Apr 24, 2024 at 02:48:23PM +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
> 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.
>
> Comapared to gro_flush_timeout and napi_defer_hard_irqs this patch

nit: Compared

> allows to enable IRQ coalescing for RX path separately.
>
> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>

..

> @@ -190,19 +191,35 @@ int emac_tx_complete_packets(struct prueth_emac *emac, int chn,
> return num_tx;
> }
>
> +static enum hrtimer_restart emac_tx_timer_callback(struct hrtimer *timer)
> +{
> + struct prueth_tx_chn *tx_chns =
> + container_of(timer, struct prueth_tx_chn, tx_hrtimer);
> +
> + enable_irq(tx_chns->irq);
> + return HRTIMER_NORESTART;
> +}
> +
> static int emac_napi_tx_poll(struct napi_struct *napi_tx, int budget)
> {
> struct prueth_tx_chn *tx_chn = prueth_napi_to_tx_chn(napi_tx);
> struct prueth_emac *emac = tx_chn->emac;
> + bool tdown = false;
> int num_tx_packets;
>
> - num_tx_packets = emac_tx_complete_packets(emac, tx_chn->id, budget);
> + num_tx_packets = emac_tx_complete_packets(emac, tx_chn->id, budget, &tdown);

Please consider limiting lines to 80 columns wide in Networking code.

>
> 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);
> + }
>
> return num_tx_packets;
> }

..

> @@ -870,7 +889,12 @@ 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]);

clang-18 and gcc-13 both complain about the if/else logic above.
I think it would be best to add {} to the outer if statement.

>
> return num_rx;
> }

..

\
 
 \ /
  Last update: 2024-05-27 18:06    [W:0.074 / U:1.688 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site