lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 26 Jul 2019 14:18:26 +0800
From:   Jian-Hong Pan <jian-hong@...lessm.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     Yan-Hsuan Chuang <yhchuang@...ltek.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S . Miller" <davem@...emloft.net>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux@...lessm.com" <linux@...lessm.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH] rtw88: pci: Use general byte arrays as the elements of RX ring

David Laight <David.Laight@...lab.com> 於 2019年7月25日 週四 下午5:21寫道:
>
> From: Jian-Hong Pan
> > Sent: 25 July 2019 09:09
> > Each skb as the element in RX ring was expected with sized buffer 8216
> > (RTK_PCI_RX_BUF_SIZE) bytes. However, the skb buffer's true size is
> > 16640 bytes for alignment after allocated, x86_64 for example. And, the
> > difference will be enlarged 512 times (RTK_MAX_RX_DESC_NUM).
> > To prevent that much wasted memory, this patch follows David's
> > suggestion [1] and uses general buffer arrays, instead of skbs as the
> > elements in RX ring.
> ...
> >       for (i = 0; i < len; i++) {
> > -             skb = dev_alloc_skb(buf_sz);
> > -             if (!skb) {
> > +             buf = devm_kzalloc(rtwdev->dev, buf_sz, GFP_ATOMIC);
>
> You should do this allocation somewhere than can sleep.
> So you don't need GFP_ATOMIC, making the allocate (and dma map)
> much less likely to fail.
> If they do fail using a smaller ring might be better than failing
> completely.

Ok, I can tweak and try it.

> I suspect that buf_sz gets rounded up somewhat.
> Also you almost certainly want 'buf' to be cache-line aligned.
> I don't think devm_kzalloc() guarantees that at all.

Sure

> While allocating all 512 buffers in one block (just over 4MB)
> is probably not a good idea, you may need to allocated (and dma map)
> then in groups.

Thanks for reviewing.  But got questions here to double confirm the idea.
According to original code, it allocates 512 skbs for RX ring and dma
mapping one by one.  So, the new code allocates memory buffer 512
times to get 512 buffer arrays.  Will the 512 buffers arrays be in one
block?  Do you mean aggregate the buffers as a scatterlist and use
dma_map_sg?

Thank you,
Jain-Hong Pan

Powered by blists - more mailing lists