All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: Question on kzalloc and GFP_DMA32
@ 2016-10-06 16:41 Adrian Chadd
  0 siblings, 0 replies; 4+ messages in thread
From: Adrian Chadd @ 2016-10-06 16:41 UTC (permalink / raw)
  To: Ben Greear; +Cc: David Rientjes, Linux Kernel Mailing List

On 28 September 2016 at 16:37, Ben Greear <greearb@candelatech.com> wrote:
> On 09/28/2016 02:11 PM, David Rientjes wrote:

[snip]

>>
>> 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.

Hi, to follow up (since I was the original suggester of DMA32 for
fixing this WMI specific issue) :

The underlying reason is that ath10k is looking for some memory that's
in the device physical memory range (which is 32 bit) so the on-chip
SoC (target CPU) can use the host memory for doing, well, what's
effectively swapping code/data segments in and out.

When using kzalloc, it looks like it's allocating memory from whatever
is available, and then it has to find contiguous DMA32 pages when it's
asked to get a physical mapping to that range. Normally the failure
experienced on 64 bit machines w/ ath10k is when the DMA32 region is
either too fragmented, or just plain full.

I've pointed out to a few ath10k people that because of how the code
kzalloc's a hundred k or so of memory and then wants a physically
contig region to be mapped, they're likely to eventually fail. My
guess (since I'm mainly a FreeBSD developer) is that the way ath10k is
getting physically contig bounce-buffer style memory after a kzalloc
is really designed for small, ephemeral allocations, and not the large
allocation that ath10k does every time the device is brought up.

So, what's the "blessed" way in Linux to allocate device DMA aware
contiguous memory to let the target device play in? Is it alloc_pages,
or is there some other API that ath10k should be using to allocate
physical memory within the bus dma constraints of a specific device?

Thanks!


-adrian

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Question on kzalloc and GFP_DMA32
  2016-09-28 21:11 ` David Rientjes
@ 2016-09-28 23:37   ` Ben Greear
  0 siblings, 0 replies; 4+ messages in thread
From: Ben Greear @ 2016-09-28 23:37 UTC (permalink / raw)
  To: David Rientjes; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Question on kzalloc and GFP_DMA32
  2016-09-27 23:07 Ben Greear
@ 2016-09-28 21:11 ` David Rientjes
  2016-09-28 23:37   ` Ben Greear
  0 siblings, 1 reply; 4+ messages in thread
From: David Rientjes @ 2016-09-28 21:11 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-kernel

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.

> 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))?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Question on kzalloc and GFP_DMA32
@ 2016-09-27 23:07 Ben Greear
  2016-09-28 21:11 ` David Rientjes
  0 siblings, 1 reply; 4+ messages in thread
From: Ben Greear @ 2016-09-27 23:07 UTC (permalink / raw)
  To: Linux Kernel Mailing List


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.

pool_size is relatively large (maybe 256k or so).

Any idea for a more proper way to do this?


gfp: 4
------------[ cut here ]------------
kernel BUG at /home/greearb/git/linux-4.7.dev.y/mm/slub.c:1508!
invalid opcode: 0000 [#1] PREEMPT SMP
Modules linked in: coretemp hwmon ath9k intel_rapl ath10k_pci x86_pkg_temp_thermal ath9k_common ath10k_core intel_powerclamp ath9k_hw ath kvm iTCO_wdt mac80211 
iTCO_vendor_support irqbypass snd_hda_codec_hdmi 6
CPU: 2 PID: 268 Comm: kworker/u8:5 Not tainted 4.7.2+ #16
Hardware name: To be filled by O.E.M. To be filled by O.E.M./ChiefRiver, BIOS 4.6.5 06/07/2013
Workqueue: ath10k_aux_wq ath10k_wmi_event_service_ready_work [ath10k_core]
task: ffff880036433a00 ti: ffff880036440000 task.ti: ffff880036440000
RIP: 0010:[<ffffffff8124592a>]  [<ffffffff8124592a>] new_slab+0x39a/0x410
RSP: 0018:ffff880036443b58  EFLAGS: 00010092
RAX: 0000000000000006 RBX: 00000000024082c4 RCX: 0000000000000000
RDX: 0000000000000006 RSI: ffff88021e30dd08 RDI: ffff88021e30dd08
RBP: ffff880036443b90 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000372 R12: ffff88021dc01200
R13: ffff88021dc00cc0 R14: ffff88021dc01200 R15: 0000000000000001
FS:  0000000000000000(0000) GS:ffff88021e300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f3e65c1c730 CR3: 0000000001e06000 CR4: 00000000001406e0
Stack:
  ffffffff8127a4fc ffff0a01ffffff10 00000000024082c4 ffff88021dc01200
  ffff88021dc00cc0 ffff88021dc01200 0000000000000001 ffff880036443c58
  ffffffff81247ac6 ffff88021e31b360 ffff880036433a00 ffff880036433a00
Call Trace:
  [<ffffffff8127a4fc>] ? __d_lookup+0x9c/0x160
  [<ffffffff81247ac6>] ___slab_alloc+0x396/0x4a0
  [<ffffffffa0f8e14d>] ? ath10k_wmi_event_service_ready_work+0x5ad/0x800 [ath10k_core]
  [<ffffffff811f5279>] ? alloc_kmem_pages+0x9/0x10
  [<ffffffff8120f203>] ? kmalloc_order+0x13/0x40
  [<ffffffffa0f8e14d>] ? ath10k_wmi_event_service_ready_work+0x5ad/0x800 [ath10k_core]
  [<ffffffff81247bf6>] __slab_alloc.isra.72+0x26/0x40
  [<ffffffff81248767>] __kmalloc+0x147/0x1b0
  [<ffffffffa0f8e14d>] ath10k_wmi_event_service_ready_work+0x5ad/0x800 [ath10k_core]
  [<ffffffff811370a1>] ? dequeue_entity+0x261/0xac0
  [<ffffffff8111c2d8>] process_one_work+0x148/0x420
  [<ffffffff8111c929>] worker_thread+0x49/0x480
  [<ffffffff8111c8e0>] ? rescuer_thread+0x330/0x330
  [<ffffffff81121984>] kthread+0xc4/0xe0
  [<ffffffff8184d75f>] ret_from_fork+0x1f/0x40
  [<ffffffff811218c0>] ? kthread_create_on_node+0x170/0x170
Code: e9 65 fd ff ff 49 8b 57 20 48 8d 42 ff 83 e2 01 49 0f 44 c7 f0 80 08 40 e9 6f fd ff ff 89 c6 48 c7 c7 01 36 c7 81 e8 e8 40 fa ff <0f> 0b ba 00 10 00 00 be 
5a 00 00 00 48 89 c7 48 d3 e2 e8 bf 18
RIP  [<ffffffff8124592a>] new_slab+0x39a/0x410
  RSP <ffff880036443b58>
---[ end trace ea3b0043b2911d93 ]---


static struct page *new_slab(struct kmem_cache *s, gfp_t flags, int node)
{
         if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
                 pr_emerg("gfp: %u\n", flags & GFP_SLAB_BUG_MASK);
                 BUG();
         }

         return allocate_slab(s,
                 flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node);
}


Thanks,
Ben

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-10-06 16:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-06 16:41 Question on kzalloc and GFP_DMA32 Adrian Chadd
  -- strict thread matches above, loose matches on Subject: below --
2016-09-27 23:07 Ben Greear
2016-09-28 21:11 ` David Rientjes
2016-09-28 23:37   ` Ben Greear

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.