On Wed, 12 Jun 2024 12:07:05 +0200 Przemek Kitszel wrote:
> Given that it will be a generic solution (would fix the [1] above),
> and be also easier to use, like:
> 
>   CACHELINE_STRUCT_GROUP(idpf_q_vector,
>       CACHELINE_STRUCT_GROUP_RD(/* read mostly */
>               struct idpf_vport *vport;
>               u16 num_rxq;
>               u16 num_txq;
>               u16 num_bufq;
>               u16 num_complq;
>               struct idpf_rx_queue **rx;
>               struct idpf_tx_queue **tx;
>               struct idpf_buf_queue **bufq;
>               struct idpf_compl_queue **complq;
>               struct idpf_intr_reg intr_reg;
>       ),
>       CACHELINE_STRUCT_GROUP_RW(
>               struct napi_struct napi;
>               u16 total_events;
>               struct dim tx_dim;
>               u16 tx_itr_value;
>               bool tx_intr_mode;
>               u32 tx_itr_idx;
>               struct dim rx_dim;
>               u16 rx_itr_value;
>               bool rx_intr_mode;
>               u32 rx_itr_idx;
>       ),
>       CACHELINE_STRUCT_GROUP_COLD(
>               u16 v_idx;
>               cpumask_var_t affinity_mask;
>       )
> );
> 
> Note that those three inner macros have distinct meaningful names not to
> have this working, but to aid human reader, then checkpatch/check-kdoc.
> Technically could be all the same CACHELINE_GROUP().
> 
> I'm not sure if (at most) 3 cacheline groups are fine for the general
> case, but it would be best to have just one variant of the
> CACHELINE_STRUCT_GROUP(), perhaps as a vararg.

I almost want to CC Linus on this because I think it's mostly about
personal preferences. I dislike the struct_group()-style macros. They
don't scale (imagine having to define two partially overlapping groups)
and don't look like C to my eyes. Kees really had to do this for his
memory safety work because we need to communicate a "real struct" type
to the compiler, but if you're just doing this so fail the build and
make the developer stop to think - it's not worth the ugliness.

Can we not extend __cacheline_group_begin() and __cacheline_group_end()
-style markings?

Reply via email to