On 2019-05-08 03:24, Tycho Nightingale wrote:
Hi,
On May 7, 2019, at 9:13 PM, Conrad Meyer <c...@freebsd.org> wrote:
Hi Tycho,
On Wed, Apr 24, 2019 at 1:31 PM Tycho Nightingale <tyc...@freebsd.org> wrote:
Author: tychon
Date: Wed Apr 24 20:30:45 2019
New Revision: 346645
URL: https://svnweb.freebsd.org/changeset/base/346645
Log:
LinuxKPI should use bus_dma(9) to be compatible with an IOMMU
Reviewed by: hselasky, kib
Tested by: greg@unrelenting.technology
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D19845
...
Modified: head/sys/compat/linuxkpi/common/src/linux_pci.c
==============================================================================
--- head/sys/compat/linuxkpi/common/src/linux_pci.c Wed Apr 24 19:56:02
2019 (r346644)
+++ head/sys/compat/linuxkpi/common/src/linux_pci.c Wed Apr 24 20:30:45
2019 (r346645)
...
+linux_dma_map_phys(struct device *dev, vm_paddr_t phys, size_t len)
+{
...
+ nseg = -1;
+ mtx_lock(&priv->dma_lock);
+ if (_bus_dmamap_load_phys(priv->dmat, obj->dmamap, phys, len,
+ BUS_DMA_NOWAIT, &seg, &nseg) != 0) {
+ bus_dmamap_destroy(priv->dmat, obj->dmamap);
+ mtx_unlock(&priv->dma_lock);
+ uma_zfree(linux_dma_obj_zone, obj);
+ return (0);
+ }
+ mtx_unlock(&priv->dma_lock);
+
+ KASSERT(++nseg == 1, ("More than one segment (nseg=%d)", nseg));
This construct is a bit odd. Coverity produces the (perhaps spurious)
warning (CID 1401319) that the KASSERT (which can be compiled out in
!INVARIANTS builds) has a side effect (++nseg). While true, nseg is
never used afterwards, so perhaps we can use the equivalent expression
with no side effect instead? I.e., something like:
KASSERT(nseg == 0, ("More than one segment (nseg=%d)", nseg + 1));
Does that make sense? It is a false positive of sorts, but performing
side effects in compiled-out assert is a pretty strong antipattern so
I'd just as soon "fix" the warning.
The construct is indeed a little odd and mimics how other callers of
_bus_dmamap_load_phys() handle the bizarre way nseg is treated. There isn’t
any reason for it and in hindsight I prefer your version — especially if it
eliminates this Coverity issue.
Tycho
I believe I already changed those asserts to what was suggested. See
later commits on the same file.
--HPS
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
