On Tue, Jun 4, 2019 at 2:30 AM Hannes Reinecke <hare@xxxxxxx> wrote:
>
> Gcc-9 complains for a memset across pointer boundaries, which happens
> as the code tries to allocate a flexible array on the stack.
> Turns out we cannot do this without relying on gcc-isms, so
> this patch converts the stack allocation in proper kzalloc() calls.
Getting back to this - maybe you already fixed this in your bigger
patch series, but I noted a problem with this:
> static inline struct fcoe_rport *fcoe_ctlr_rport(struct fc_rport_priv *rdata)
> {
> - return (struct fcoe_rport *)(rdata + 1);
> + return (struct fcoe_rport *)(&rdata->rpriv);
> }
...
> @@ -212,6 +212,7 @@ struct fc_rport_priv {
> struct rcu_head rcu;
> u16 sp_features;
> u8 spp_type;
> + char rpriv[];
> };
The above does not work at all on machines that have alignment
constraints, because now your fcoe_rport pointer will be very much
mis-aligned.
The old "(rdata + 1)" thing was also potentially mis-aligned: the size
of "struct fc_rport_priv" is aligned, but it's aligned to the
alignment of fc_rport_priv, not to the alignment of struct fcoe_rport.
But in practice the old alignment was probably "good enough".
But the "char rpriv[]" model definitely has horrendous explicit
mix-alignment, with that previous single-byte spp_type member. It's
almost certainly at a 3-byte offset.
On x86, you won't notice. It won't even perform all that badly. But on
other architectures it might not work at all, or perform horribly
badly.
Using at least "u64 rpriv[]" might be better. I didn't look at your
other patch version.
Linus
