Date: Tue, 7 May 2019 18:13:06 -0700 From: Conrad Meyer <cem@freebsd.org> To: Tycho Nightingale <tychon@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r346645 - in head/sys: compat/linuxkpi/common/include/linux compat/linuxkpi/common/src sys Message-ID: <CAG6CVpUeq0%2BckY2-r_K7Yj4e78T1WPjrtYUrbUC0NTkt%2Bg0Hiw@mail.gmail.com> In-Reply-To: <201904242030.x3OKUkgN073331@repo.freebsd.org> References: <201904242030.x3OKUkgN073331@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Tycho, On Wed, Apr 24, 2019 at 1:31 PM Tycho Nightingale <tychon@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. Thanks, Conrad
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpUeq0%2BckY2-r_K7Yj4e78T1WPjrtYUrbUC0NTkt%2Bg0Hiw>