Subject: Re: i40e: Kernel unaligned access due to 'struct i40e_dma_mem' being 'packed'

From: tndave <[email-masked]>
Date: Sun, 14 Feb 2016 05:52:40 +0530

On 01/30/2016 04:17 AM, tndave wrote:
>
>
> On 01/27/2016 10:56 PM, David Miller wrote:
>> From: tndave <[email-masked]>
>> Date: Wed, 27 Jan 2016 17:50:14 -0800
>>
>>> Hi,
>>>
>>> i40e driver has 'struct i40e_dma_mem' defined with 'packed' directive
>>> causing kernel unaligned errors on sparc (when
>>> 40e_allocate_dma_mem_d()
>>> is being called)
>>>
>>> log_unaligned: 1031 callbacks suppressed
>>> Kernel unaligned access at TPC[448ae8]
>>> dma_4v_alloc_coherent+0x188/0x2e0
>>> Kernel unaligned access at TPC[448ae8]
>>> dma_4v_alloc_coherent+0x188/0x2e0
>>> Kernel unaligned access at TPC[448ae8]
>>> dma_4v_alloc_coherent+0x188/0x2e0
>>> Kernel unaligned access at TPC[448ae8]
>>> dma_4v_alloc_coherent+0x188/0x2e0
>>>
>>> This can be fixed with get_unaligned/put_unaligned(). However I don't
>>> see 'struct i40e_dma_mem' is being directly shoved into NIC hardware.
>>> But instead fields of the struct are being read and used for hardware
>>> (e.g. dma_addr_t pa). For the test, I remove __packed, and i40e driver
>>> and HW works fine. (of course kernel unaligned errors are gone too).
>>> My question is, does 'struct i40e_dma_mem' required to be __packed?
>>
>> People get overzealoud with __packed.
>>
>> And even if it doesn't cause unaligned accesses like this, it generates
>> terrible code (byte at a time accesses to words) on several
>> architectures.
> True. For the same reason I want to clarify if __packed is actually
> needed? instead of fixing it with get_unaligned/put_unaligned()!

We are having this issue on multiple sparc servers. It would be really
helpful to have feedback from i40e driver folks.

Thanks.

-Tushar