Re: Question on kzalloc and GFP_DMA32

[Ben Greear <greearb@candelatech.com>]

On 09/28/2016 02:11 PM, David Rientjes wrote:
> On Tue, 27 Sep 2016, Ben Greear wrote:
>
>>
>> I have been running this patch for a while:
>>
>>     ath10k:  Use GPF_DMA32 for firmware swap memory.
>>
>>     This fixes OS crash when using QCA 9984 NIC on x86-64 system
>>     without vt-d enabled.
>>
>>     Also tested on ea8500 with 9980, and x86-64 with 9980 and 9880.
>>
>>     All tests were with CT firmware.
>>
>>     Signed-off-by: Ben Greear <greearb@candelatech.com>
>>
>> -------------------- drivers/net/wireless/ath/ath10k/wmi.c
>> --------------------
>> index e20aa39..727b3aa 100644
>> @@ -4491,7 +4491,7 @@ static int ath10k_wmi_alloc_chunk(struct ath10k *ar, u32
>> req_id,
>>          if (!pool_size)
>>              return -EINVAL;
>>
>> -        vaddr = kzalloc(pool_size, GFP_KERNEL | __GFP_NOWARN);
>> +        vaddr = kzalloc(pool_size, GFP_KERNEL | __GFP_NOWARN | GFP_DMA32);
>>          if (!vaddr)
>>              num_units /= 2;
>>      }
>>
>>
>> It sometimes works, on some systems, but then it also often fails with the
>> splat
>> below, especially with several NICs in the system.
>>
>
> I suppose it's failing sometimes because the BUG() will trigger when
> trying to allocate new slab or CONFIG_ZONE_DMA32 isn't configured.  That
> shouldn't panic the kernel anymore since commit 72baeef0c271 ("slab: do
> not panic on invalid gfp_mask") in 4.8, but you shouldn't be doing
> kzalloc(..., ... | GFP_DMA32) anyway.

CONFIG_ZONE_DMA32 is enabled in my .config.

>> pool_size is relatively large (maybe 256k or so).
>>
>
> If it's 256k, why allocate through the slab allocator?  Why not
> alloc_pages(GFP_KERNEL | GFP_DMA32 | __GFP_ZERO | __GFP_NOWARN,
> get_order(pool_size))?

I really don't understand the (subtle?) difference between alloc_pages and kzalloc,
but I will give your suggestion a try and see if it works.  If you have time, maybe you could take
a look at drivers/net/wireless/ath/ath10k/wmi.c in the ath10k_wmi_alloc_chunk method
to see if you notice any problems with using alloc_pages there?

Thanks for the suggestion.


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com