On 6/7/19 10:55 PM, Linus Torvalds wrote:
> 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.
>
Thanks for the heads-up.
I actually had discussed the alignment problematic with our gcc folks,
and they claimed that the only portable way for specifying an abstract
array at the end of the structure was indeed to use 'char n[]'; they
claimed 'u64' would have different alignment issues etc and would only
work reliably on gcc.
So I might as well going back to the original style, as the offending
calls have been fixed with a different patch quite independently of this
issue.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@xxxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
