linux-kernel - Re: [PATCH 42/44] powerpc/cell: use the dma

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]

Message-ID: <20170618071344.GB18526@infradead.org>
Date:   Sun, 18 Jun 2017 00:13:44 -0700
From:   Christoph Hellwig <hch@...radead.org>
To:     Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc:     Christoph Hellwig <hch@....de>, x86@...nel.org,
        linux-arm-kernel@...ts.infradead.org,
        xen-devel@...ts.xenproject.org, linux-c6x-dev@...ux-c6x.org,
        linux-hexagon@...r.kernel.org, linux-ia64@...r.kernel.org,
        linux-mips@...ux-mips.org, openrisc@...ts.librecores.org,
        linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org,
        linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
        linux-xtensa@...ux-xtensa.org, dmaengine@...r.kernel.org,
        linux-tegra@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-samsung-soc@...r.kernel.org,
        iommu@...ts.linux-foundation.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 42/44] powerpc/cell: use the dma_supported method for ops
 switching

On Sun, Jun 18, 2017 at 06:50:27AM +1000, Benjamin Herrenschmidt wrote:
> What is your rationale here ? (I have missed patch 0 it seems).

Less code duplication, more modular dma_map_ops insteance.

> dma_supported() was supposed to be pretty much a "const" function
> simply informing whether a given setup is possible. Having it perform
> an actual switch of ops seems to be pushing it...

dma_supported() is already gone from the public DMA API as it doesn't
make sense to be called separately from set_dma_mask.  It will be
entirely gone in the next series after this one.

> What if a driver wants to test various dma masks and then pick one ?
> 
> Where does the API documents that if a driver calls dma_supported() it
> then *must* set the corresponding mask and use that ?

Where is the API document for _any_ of the dma routines? (A: work in
progress by me, but I need to clean up the mess of arch hooks before
it can make any sense)

> I don't like a function that is a "boolean query" like this one to have
> such a major side effect.
> 
> >From an API standpoint, dma_set_mask() is when the mask is established,
> and thus when the ops switch should happen.

And that's exactly what happens at the driver API level.  It just turns
out the dma_capable method is they way better place to actually
implement it, as the ->set_dma_mask method requires lots of code
duplication while not offering any actual benefit over ->dma_capable.
And because of that it's gone after this series.

In theory we could rename ->dma_capable now, but it would require a
_lot_ of churn.  Give me another merge window or two and we should
be down to be about 2 handful of dma_map_ops instance, at which point
we could do all this gratious renaming a lot more easily :)