From owner-svn-src-all@freebsd.org Wed May 8 01:25:06 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 3173B15996DF; Wed, 8 May 2019 01:25:06 +0000 (UTC) (envelope-from tychon@freebsd.org) Received: from pb-smtp1.pobox.com (pb-smtp1.pobox.com [64.147.108.70]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C6CA285102; Wed, 8 May 2019 01:25:05 +0000 (UTC) (envelope-from tychon@freebsd.org) Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 9A84C14E159; Tue, 7 May 2019 21:24:59 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=content-type :mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=sasl; bh= htkg6BhtvsGYdOgDXZmA1a8dSiY=; b=SjxRq3UzMZOuqT+x+c6Yne+xE4e8cVZH zcEhWAnFoZ88KMDDLenUv/TEtUf1FLjJF0q9WTQ0AM0gprsK2SXNTzDZJUA1fTE0 9ZoKgS9JgxJbav8HGxqfyQZpKn950B6XqQZiVRGsMdyJsIjvkPl/vuOPZJgbYuT6 tHmbmVWzv9c= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 916E914E158; Tue, 7 May 2019 21:24:59 -0400 (EDT) Received: from [10.0.1.195] (unknown [146.115.68.244]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id BC32E14E155; Tue, 7 May 2019 21:24:58 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Subject: Re: svn commit: r346645 - in head/sys: compat/linuxkpi/common/include/linux compat/linuxkpi/common/src sys From: Tycho Nightingale In-Reply-To: Date: Tue, 7 May 2019 21:24:57 -0400 Cc: src-committers , svn-src-all , svn-src-head Content-Transfer-Encoding: quoted-printable Message-Id: <6991C64C-D105-423F-B66A-4E2B2924565F@freebsd.org> References: <201904242030.x3OKUkgN073331@repo.freebsd.org> To: cem@freebsd.org X-Mailer: Apple Mail (2.3445.9.1) X-Pobox-Relay-ID: 18212434-7130-11E9-A6BF-46F8B7964D18-09779102!pb-smtp1.pobox.com X-Rspamd-Queue-Id: C6CA285102 X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.95 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.95)[-0.948,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 May 2019 01:25:06 -0000 Hi, > On May 7, 2019, at 9:13 PM, Conrad Meyer wrote: >=20 > Hi Tycho, >=20 > On Wed, Apr 24, 2019 at 1:31 PM Tycho Nightingale = wrote: >>=20 >> Author: tychon >> Date: Wed Apr 24 20:30:45 2019 >> New Revision: 346645 >> URL: https://svnweb.freebsd.org/changeset/base/346645 >>=20 >> Log: >> LinuxKPI should use bus_dma(9) to be compatible with an IOMMU >>=20 >> 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 >> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> --- 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 =3D -1; >> + mtx_lock(&priv->dma_lock); >> + if (_bus_dmamap_load_phys(priv->dmat, obj->dmamap, phys, len, >> + BUS_DMA_NOWAIT, &seg, &nseg) !=3D 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 =3D=3D 1, ("More than one segment (nseg=3D%d)", = nseg)); >=20 > 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: >=20 > KASSERT(nseg =3D=3D 0, ("More than one segment (nseg=3D%d)", nseg + = 1)); >=20 > 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=E2=80=99t any reason for it and in hindsight I prefer your version = =E2=80=94 especially if it eliminates this Coverity issue. Tycho=