linux-kernel - Re: [PATCH 2/2] media: usb: pwc: Don't use coherent DMA buffers for ISO transfer

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:   Tue, 31 Jul 2018 15:06:45 +0900
From:   Tomasz Figa <tfiga@...omium.org>
To:     Mauro Carvalho Chehab <mchehab+samsung@...nel.org>
Cc:     Ezequiel Garcia <ezequiel@...labora.com>,
        "Matwey V. Kornilov" <matwey@....msu.ru>,
        Alan Stern <stern@...land.harvard.edu>, hdegoede@...hat.com,
        Hans Verkuil <hverkuil@...all.nl>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        rostedt@...dmis.org, mingo@...hat.com,
        Mike Isely <isely@...ox.com>,
        Bhumika Goyal <bhumirks@...il.com>,
        Colin King <colin.king@...onical.com>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] media: usb: pwc: Don't use coherent DMA buffers for
 ISO transfer

On Tue, Jul 31, 2018 at 1:07 AM Mauro Carvalho Chehab
<mchehab+samsung@...nel.org> wrote:
>
> Em Tue, 17 Jul 2018 17:10:22 -0300
> Ezequiel Garcia <ezequiel@...labora.com> escreveu:
>
> > Yeah, and not setting URB_NO_TRANSFER_DMA_MAP makes the USB core
> > create DMA mappings and use the streaming API. Which makes more
> > sense in hardware without hardware coherency.
> >
> > The only thing that bothers me with this patch is that it's not
> > really something specific to this driver. If this fix is valid
> > for pwc, then it's valid for all the drivers allocating coherent
> > memory.
>
> We're actually doing this change on other drivers:
>         https://git.linuxtv.org/media_tree.git/commit/?id=d571b592c6206
>
> I suspect that the reason why all USB media drivers were using
> URB_NO_TRANSFER_DMA_MAP is just because the first media USB driver
> upstream used it.
>
> On that time, I remember I tried once to not use this flag, but there
> was something that broke (perhaps I just didn't know enough about the
> USB layer - or perhaps some fixes happened at USB core - allowing it
> to be used with ISOC transfers).
>
> Anyway, nowadays, I fail to see a reason why not let the USB core
> do the DMA maps. On my tests after this patch, at the boards I tested
> (arm and x86), I was unable to see any regressions.

I can see one reason:

usb_alloc_coherent() would use dma_pool_alloc() with a fallback to
dma_alloc_coherent() to do the allocation. I'm not sure what a typical
size for an URB buffer is, but I assume that it's definitely more than
1 page. Order >0 allocations with page allocator (and SLAB, which
eventually just falls back to page allocator for such large
allocations) are generally considered costly. With allocation through
DMA API, mechanisms such as CMA or IOMMU can be used (if available),
making it much more likely to have the allocation satisfied on heavy
load / long uptime systems.

Best regards,
Tomasz