Re: [PATCH net-next v9 08/15] net: homa: create homa_pacer.h and homa_pacer.c

[Simon Horman <horms@kernel.org>]

On Mon, Jun 09, 2025 at 08:40:41AM -0700, John Ousterhout wrote:
> These files provide facilities to pace packet output in order to prevent
> queue buildup in the NIC. This functionality is needed to implement SRPT
> on output, so short messages don't get stuck in long NIC queues. Note: the
> pacer eventually needs to be replaced with a Homa-specific qdisc, which can
> better manage simultaneous transmissions by Homa and TCP. The current
> implementation can coexist with TCP and doesn't harm TCP, but
> Homa's latency suffers when TCP runs concurrently.
> 
> Signed-off-by: John Ousterhout <ouster@cs.stanford.edu>
> 
> ---
> Changes for v9:
> * Add support for homa_net objects
> * Use new homa_clock abstraction layer
> * Various name improvements (e.g. use "alloc" instead of "new" for functions
>   that allocate memory)
> 
> Changes for v8:
> * This file is new in v8 (functionality extracted from other files)
> ---
>  net/homa/homa_impl.h  |   1 +
>  net/homa/homa_pacer.c | 316 ++++++++++++++++++++++++++++++++++++++++++
>  net/homa/homa_pacer.h | 190 +++++++++++++++++++++++++
>  3 files changed, 507 insertions(+)
>  create mode 100644 net/homa/homa_pacer.c
>  create mode 100644 net/homa/homa_pacer.h
> 

> diff --git a/net/homa/homa_impl.h b/net/homa/homa_impl.h

...

> +/**
> + * homa_pacer_manage_rpc() - Arrange for the pacer to transmit packets
> + * from this RPC (make sure that an RPC is on the throttled list and wake up
> + * the pacer thread if necessary).
> + * @rpc:     RPC with outbound packets that have been granted but can't be
> + *           sent because of NIC queue restrictions. Must be locked by caller.
> + */
> +void homa_pacer_manage_rpc(struct homa_rpc *rpc)
> +	__must_hold(rpc_bucket_lock)
> +{
> +	struct homa_pacer *pacer = rpc->hsk->homa->pacer;
> +	struct homa_rpc *candidate;
> +	int bytes_left;
> +	int checks = 0;

Checks is set but otherwise unused in this function.
Probably it can be removed.

Flagged by Clang 20.1.4 as:

  .../homa_pacer.c:252:6: warning: variable 'checks' set but not used [-Wunused-but-set-variable]
    252 |         int checks = 0;
        |             ^
> +
> +	if (!list_empty(&rpc->throttled_links))
> +		return;
> +	bytes_left = rpc->msgout.length - rpc->msgout.next_xmit_offset;
> +	homa_pacer_throttle_lock(pacer);
> +	list_for_each_entry(candidate, &pacer->throttled_rpcs,
> +			    throttled_links) {
> +		int bytes_left_cand;
> +
> +		checks++;
> +
> +		/* Watch out: the pacer might have just transmitted the last
> +		 * packet from candidate.
> +		 */
> +		bytes_left_cand = candidate->msgout.length -
> +				candidate->msgout.next_xmit_offset;
> +		if (bytes_left_cand > bytes_left) {
> +			list_add_tail(&rpc->throttled_links,
> +				      &candidate->throttled_links);
> +			goto done;
> +		}
> +	}
> +	list_add_tail(&rpc->throttled_links, &pacer->throttled_rpcs);
> +done:
> +	homa_pacer_throttle_unlock(pacer);
> +	wake_up(&pacer->wait_queue);
> +}

...