[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6a440baa-fd9b-4d00-a15e-1cdbfce52168@intel.com>
Date: Mon, 30 Sep 2024 14:33:45 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Joe Damato <jdamato@...tly.com>
CC: <netdev@...r.kernel.org>, Tony Nguyen <anthony.l.nguyen@...el.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "moderated list:INTEL
ETHERNET DRIVERS" <intel-wired-lan@...ts.osuosl.org>, open list
<linux-kernel@...r.kernel.org>
Subject: Re: [RFC net-next 1/1] idpf: Don't hard code napi_struct size
From: Joe Damato <jdamato@...tly.com>
Date: Wed, 25 Sep 2024 18:00:17 +0000
> The sizeof(struct napi_struct) can change. Don't hardcode the size to
> 400 bytes and instead use "sizeof(struct napi_struct)".
Just change this hardcode here when you submit your series.
I use sizeof() here only for structures which size can change depending
on .config, like ones containing spinlocks etc.
If you just add one field, it's easy to adjust the size here.
Otherwise, these assertions lose their sense. They're used to make sure
the structures are of *certain* *known* size, hardcoded inside them.
sizeof()s mean nothing, they don't give you the idea of how the
cachelines are organized after some code change.
>
> While fixing this, also move other calculations into compile time
> defines.
>
> Signed-off-by: Joe Damato <jdamato@...tly.com>
> ---
> drivers/net/ethernet/intel/idpf/idpf_txrx.h | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.h b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> index f0537826f840..d5e904ddcb6e 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.h
> @@ -437,9 +437,13 @@ struct idpf_q_vector {
> cpumask_var_t affinity_mask;
> __cacheline_group_end_aligned(cold);
> };
> -libeth_cacheline_set_assert(struct idpf_q_vector, 112,
> - 424 + 2 * sizeof(struct dim),
> - 8 + sizeof(cpumask_var_t));
> +
> +#define IDPF_Q_VECTOR_RO_SZ (112)
> +#define IDPF_Q_VECTOR_RW_SZ (sizeof(struct napi_struct) + 24 + \
> + 2 * sizeof(struct dim))
> +#define IDPF_Q_VECTOR_COLD_SZ (8 + sizeof(cpumask_var_t))
> +libeth_cacheline_set_assert(struct idpf_q_vector, IDPF_Q_VECTOR_RO_SZ,
> + IDPF_Q_VECTOR_RW_SZ, IDPF_Q_VECTOR_COLD_SZ);
>
> struct idpf_rx_queue_stats {
> u64_stats_t packets;
Thanks,
Olek
Powered by blists - more mailing lists