From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47415) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aggkR-0004SC-P1 for qemu-devel@nongnu.org; Thu, 17 Mar 2016 18:50:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aggkQ-0008SF-H3 for qemu-devel@nongnu.org; Thu, 17 Mar 2016 18:50:31 -0400 Date: Fri, 18 Mar 2016 09:33:31 +1100 From: David Gibson Message-ID: <20160317223331.GY9032@voom> References: <1458130611-17304-1-git-send-email-thuth@redhat.com> <1458130611-17304-3-git-send-email-thuth@redhat.com> <20160317062307.GO9032@voom> <56EA5D07.3000706@redhat.com> <56EACA1A.9090104@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="E0VqhFbWSXkyZBiT" Content-Disposition: inline In-Reply-To: <56EACA1A.9090104@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/3] hw/net/spapr_llan: Fix receive buffer handling for better performance List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: lvivier@redhat.com, Alexey Kardashevskiy , Jason Wang , qemu-devel@nongnu.org, Alexander Graf , qemu-ppc@nongnu.org, Anton Blanchard --E0VqhFbWSXkyZBiT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Mar 17, 2016 at 04:15:38PM +0100, Thomas Huth wrote: > On 17.03.2016 08:30, Thomas Huth wrote: > > On 17.03.2016 07:23, David Gibson wrote: > >> On Wed, Mar 16, 2016 at 01:16:50PM +0100, Thomas Huth wrote: > >>> > >>> This patch introduces an alternate way of handling the receive > >>> buffers of the spapr-vlan device, resulting in much better > >>> receive performance for the guest. > [...] > >>> +/** > >>> + * Enqueuing receive buffer by adding it to one of our receive buffe= r pools > >>> + */ > >>> +static target_long spapr_vlan_add_rxbuf_to_pool(VIOsPAPRVLANDevice *= dev, > >>> + target_ulong buf) > >>> +{ > >>> + int size =3D VLAN_BD_LEN(buf); > >>> + int pool; > >>> + > >>> + pool =3D spapr_vlan_get_rx_pool_id(dev, size); > >>> + > >>> + /* No matching pool found? Try to create a new one */ > >>> + if (pool < 0) { > >>> + for (pool =3D RX_MAX_POOLS - 1; pool >=3D 0 ; pool--) { > >> > >> I don't think this loop actually accomplishes anything. Either the > >> last slot is free, in which case you use it, then sort into place, or > >> it's not, in which case you've hit the maximum number of buffer pools. > >=20 > > Oh, you're right. Well spotted! I'll rework my patch to do it without > > that loop. >=20 > Wait, no, there was a case where this loop is actually really required: >=20 > 1) All pools are in use and filled with at least one BD > 2) User in the guest suddenly decides to change the buffer size of > one of the pools in the /sys fs of the guest. > 3) Guest driver tries to add buffers with a new size that do not > match any size of one of the pools in the host > 4) After the pool on the host runs empty which contained the BDs with > the size that is not in use anymore, we should recycle that pool > for the buffers with the new size instead. Since that buffer pool > might not be at the end of the list, we've got to scan all buffers > here to make sure we find it. >=20 > So I think the for-loop should stay as it is. Ah, good point. I think I was assuming that the pools got sorted when one was emptied as well, but they're not and I suspect it's not a good idea to do so. Hmm.. I wonder if there's a brief way of explaining the above to put in the comment. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --E0VqhFbWSXkyZBiT Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJW6zC7AAoJEGw4ysog2bOSRyMQAOBRQrtdA1yGj00Yyk//sz3V gnT3ATtbgeqMEksHhn8jOPwxaexAZCSBbI52C06n61vvsv+VVFLFUB01UbcV5nSv HVepNEpycZmNrvqKoo3wjo+oxX3gVeW2c9RwzUz1sLFIx+EyKTFGPILjhSG5pc3d C6etIuUBzKP5a5n0IGZIdC6oQ8EkTcldw1I2waNnGCvhAotRX7Dbj/L93JOFC9yu aW41EeJYOGSUppUBaI064aLDmTAueKjMVKlUykd4s9VhByQ8MS/DHkVBPuxM/8Y+ RaZfRE5K6JcVG7s/UMrTaaxUeRsU3d2h+v6w0Pdb2swMYx8x77CBTyNmtZEnnSHM NeDqcPgqMWCo6iz7gADs6V6DmLjiN8nQq05mUqnPOYJKGWGbN5Cu5Oz4mg9rod1x XPN9vhDR4Ufit/50wslTYWxsSl63vsIVt3pFpmWQ9Rw/CVINviOpRjom6O8W2aGA zkU7r5XGQjN5qavQSMqqIMLQI9sx5g3V997VdPWXxRVMe56QZu7iYqd6jjbONSUE BtKWjMXnq+OT0DLA3mstC7XhJ8E543tHK6qBTnOQhdDRTyCQLeOSlBjO9tz5xacv pPYHYEhmf12ASI/HSM8TSrvVuzbRmoUsc7VSx1Ws9PqoVT79Z19Atu6hzpqKZ2Jr AMHK7yD87Jwn4UwCnVsb =VE6P -----END PGP SIGNATURE----- --E0VqhFbWSXkyZBiT--