Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1492203 > unrolled thread

Question on kzalloc and GFP_DMA32

Started byBen Greear <greearb@candelatech.com>
First post2016-09-28 01:10 +0200
Last post2016-09-29 01:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

Contents

  Question on kzalloc and GFP_DMA32 Ben Greear <greearb@candelatech.com> - 2016-09-28 01:10 +0200
    Re: Question on kzalloc and GFP_DMA32 David Rientjes <rientjes@google.com> - 2016-09-28 23:20 +0200
      Re: Question on kzalloc and GFP_DMA32 Ben Greear <greearb@candelatech.com> - 2016-09-29 01:40 +0200

#1492203 — Question on kzalloc and GFP_DMA32

FromBen Greear <greearb@candelatech.com>
Date2016-09-28 01:10 +0200
SubjectQuestion on kzalloc and GFP_DMA32
Message-ID<sm9FD-7lr-9@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1492959

FromDavid Rientjes <rientjes@google.com>
Date2016-09-28 23:20 +0200
Message-ID<smuqJ-3ym-15@gated-at.bofh.it>
In reply to#1492203
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))?

[toc] | [prev] | [next] | [standalone]

#1493032

FromBen Greear <greearb@candelatech.com>
Date2016-09-29 01:40 +0200
Message-ID<smwCd-4SR-3@gated-at.bofh.it>
In reply to#1492959
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web