Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 21 May 2014 19:03:35 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Scott Long <scottl@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r266481 - head/sys/x86/x86
Message-ID:  <20140521160335.GT74331@kib.kiev.ua>
In-Reply-To: <201405202243.s4KMhIRu033126@svn.freebsd.org>
References:  <201405202243.s4KMhIRu033126@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--uquXjW9+X8a4Yk6h
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, May 20, 2014 at 10:43:18PM +0000, Scott Long wrote:
> Author: scottl
> Date: Tue May 20 22:43:17 2014
> New Revision: 266481
> URL: http://svnweb.freebsd.org/changeset/base/266481
>=20
> Log:
>   Old PCIe implementations cannot allow a DMA transfer to cross a 4GB
>   boundary.  This was addressed several years ago by creating a parent
>   tag hierarchy for the root buses that set the boundary restriction
>   for appropriate buses and allowed child deviced to inherit it.
>   Somewhere along the way, this restriction was turned into a case for
>   marking the tag as a candidate for needing bounce buffers, instead
>   of just splitting the segment along the boundary line.  This flag
>   also causes all maps associated with this tag to be non-NULL, which
>   in turn causes bus_dmamap_sync() to take the slow path of function
>   pointer indirection to discover that there's no bouncing work to
>   do.  The end result is a lot of pages set aside in bounce pools
>   that will never be used, and a slow path for data buffers in nearly
>   every DMA-capable PCIe device.  For example, our workload at Netflix
>   was spending nearly 1% of all CPU time going through this slow path.
>  =20
>   Fix this problem by being more selective about when to set the
>   COULD_BOUNCE flag.  Only set it when the boundary restriction
>   exists and the consumer cannot do more than a single DMA segment
>   at once.  This fixes the case of dynamic buffers (mbufs, bio's)
>   but doesn't address static buffers allocated from bus_dmamem_alloc().
>   That case will be addressed in the future.
>  =20
>   For those interested, this was discovered thanks to Dtrace Flame
>   Graphs.
>  =20
>   Discussed with: jhb, kib
>   Obtained from:	Netflix, Inc.
>   MFC after:	3 days
>=20
> Modified:
>   head/sys/x86/x86/busdma_bounce.c
>=20
> Modified: head/sys/x86/x86/busdma_bounce.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/x86/x86/busdma_bounce.c	Tue May 20 22:11:52 2014	(r266480)
> +++ head/sys/x86/x86/busdma_bounce.c	Tue May 20 22:43:17 2014	(r266481)
> @@ -172,12 +172,35 @@ bounce_bus_dma_tag_create(bus_dma_tag_t=20
>  	newtag->map_count =3D 0;
>  	newtag->segments =3D NULL;
> =20
> +	/*
> +	 * Bouncing might be needed if there's a filter.
> +	 * XXX Filters are likely broken as there's no way to
> +	 *     guarantee that bounce pages will also satisfy the
> +	 *     filter requirement.
> +	 */
>  	if (parent !=3D NULL && ((newtag->common.filter !=3D NULL) ||
>  	    ((parent->common.flags & BUS_DMA_COULD_BOUNCE) !=3D 0)))
>  		newtag->common.flags |=3D BUS_DMA_COULD_BOUNCE;
> =20
> -	if (newtag->common.lowaddr < ptoa((vm_paddr_t)Maxmem) ||
> -	    newtag->common.alignment > 1)
> +	/*
> +	 * Bouncing might be needed if there's an upper memory
> +	 * restriction.
> +	 */
> +	if (newtag->common.lowaddr < ptoa((vm_paddr_t)Maxmem))
> +		newtag->common.flags |=3D BUS_DMA_COULD_BOUNCE;
> +
> +	/*
> +	 * Bouncing might be needed if there's an alignment
> +	 * restriction that can't be satisfied by breaking up
> +	 * the segment.
> +	 * XXX Need to consider non-natural alignment.
> +	 * XXX Static allocations that tie to bus_dmamem_alloc()
> +	 *     will likely pass this test and be penalized with
> +	 *     the COULD_BOUNCE flag.  Should probably have
> +	 *     bus_dmamem_alloc() clear this flag.
> +	 */
> +	if ((newtag->common.nsegments <=3D 1) &&
> +	    (newtag->common.alignment > 1))
>  		newtag->common.flags |=3D BUS_DMA_COULD_BOUNCE;
> =20
>  	if (((newtag->common.flags & BUS_DMA_COULD_BOUNCE) !=3D 0) &&
You changed the handling of the alignment, which is probably not correct.
The problematic parameter, if any, is boundary.

--uquXjW9+X8a4Yk6h
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (FreeBSD)

iQIcBAEBAgAGBQJTfM5WAAoJEJDCuSvBvK1BWSoP/1Xw9my9yPXgROku55QRi+Bi
Ccatu/hRWZN5Z6B/+3ZbAHAp+lUPKXlMsUjMJTvjqW9iJsoU/91Znffn8hglmEd7
OYtAcDH+blImaoB7ooidg5IUmiiuiuSHmJqTLCRzwQcGPPixhtwZNKRCABm5Sn2g
MTS74zPm8gJGZBcLhUG3r/pA9sWs4MEMqAgTeAyAfr1S4pgiu+e634e32mklx0nc
hZBjgp5ihTYntaF4Xf+2ylDC/R2rzjTTdpnasevwcQu57XUW0dXhvsG/9BUHpdOc
3jlJnY/Ysx/pJbxYbO47MdW0qZuW8DoV3okQkXoGiqMLJFVerbU8DVU1NDiqA9aU
llkVKrnLMIFY8qW2SgmEPqLxrjaTZ32U52rZ/rwjgLsQfyuziqpfzYg/1R7QYLkW
wV6WMIvEQxqujE4xkdJfWbsiSoplpUyIV65tKLacIDBMPKUWv/IyX7ayZte9dyAK
NTSCLmbW/CbWsh3YKT+joIjOk6NqE4s7cO/Q9+VLg1q0Hvf3V6acRkD8Vva56Ybf
O0NFzwLTqucb4SGr4/4byxONC9+q/kzM41LHii9BSvByBaLjRSqs4AYQHmD+ZqKH
OW1N0CDJyHsNOt2urLFlSQpBCimKDpv+FANCDIZiG3H6kAbL3M6z8y8SM2CaqvMN
t2ycXMRzYWfw5Dgt2qjZ
=i9mV
-----END PGP SIGNATURE-----

--uquXjW9+X8a4Yk6h--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140521160335.GT74331>