From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek =?utf-8?Q?Marczykowski-G=C3=B3recki?= Subject: Re: [Minios-devel] Double gnttab_end_access in mini-os netfront Date: Tue, 8 Dec 2015 13:32:53 +0100 Message-ID: <20151208123253.GP960@mail-itl> References: <20151206021957.GB15506@mail-itl> <1449574429.16124.56.camel@citrix.com> <20151208114631.GM960@mail-itl> <20151208120031.GC2916@var.bordeaux.inria.fr> <1449576694.16124.72.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5820298936242738441==" Return-path: In-Reply-To: <1449576694.16124.72.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: minios-devel@lists.xenproject.org, Samuel Thibault , Martin Lucina , xen-devel List-Id: xen-devel@lists.xenproject.org --===============5820298936242738441== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="WYfJCIN5rqlfy3K0" Content-Disposition: inline --WYfJCIN5rqlfy3K0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 08, 2015 at 12:11:34PM +0000, Ian Campbell wrote: > On Tue, 2015-12-08 at 13:00 +0100, Samuel Thibault wrote: > > Marek Marczykowski-G=C3=B3recki, on Tue 08 Dec 2015 12:46:31 +0100, wro= te: > > > > > http://xenbits.xen.org/gitweb/?p=3Dmini-os.git;a=3Dcommit;h=3D7c8= f3483906 > > > > > 52a67e > > > > > 9356eec9cd2b0f76a9c7c72 > > > > >=20 > > > > > With that commit reverted, issue vanishes. > > > > >=20 > > > > > I guess it's because before this commit, there was "if (rx->status > > > > > =3D=3D > > > > > NETIF_RSP_NULL) continue" before "gnttab_end_access(buf->gref)", > > > > > but now > > > > > that case is handled after gnttab_end_access (using "if (rx->stat= us=20 > > > > > > > > > > > NETIF_RSP_NULL)"). I think the fix would be to restore that > > > > > "continue" > > > > > line. > > > >=20 > > > > That sounds pretty plausible to me (FWIW). Have you tried it? > > >=20 > > > I've tried moving gnttab_end_access into that if branch. And it didn't > > > worked. > >=20 > > Which if branch?=C2=A0=C2=A0Please show the code, C is less ambiguous t= han english > > :) >=20 > Details of in which way it didn't work would also be useful, I expect. I don't have my test environment handy, but the change was: --- netfront.c.orig 2015-12-08 13:29:04.913000000 +0100 +++ netfront.c 2015-12-08 13:29:24.060000000 +0100 @@ -122,10 +122,10 @@ =20 buf =3D &dev->rx_buffers[id]; page =3D (unsigned char*)buf->page; - gnttab_end_access(buf->gref); =20 if (rx->status > NETIF_RSP_NULL) { + gnttab_end_access(buf->gref); #ifdef HAVE_LIBC if (dev->netif_rx =3D=3D NETIF_SELECT_RX) { int len =3D rx->status; But to be frank, I'm not entirely sure that the tested version was really with this patch, as stubdom building code is quite convolved... The crash was the same, but I'll test it again to be sure. --=20 Best Regards, Marek Marczykowski-G=C3=B3recki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? --WYfJCIN5rqlfy3K0 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWZs32AAoJENuP0xzK19cspJ0H/0oTwClwhJugRleKi8fuPRJg /Dl1bOP5lt3WeDODnMUE8mqcVMA1sfmIEzgrIhriq60aIrpamMaENrQ1BYTGcda/ AS//AHuAxyiv14lpNtA3+kGDz5N2Vu76qoGAZwb6A9mcNHCbsrBpKld3hK27lJUU ppHRovsj/3BxIhXoBpjOTikgfUG6wLnNdusBojYNz6ec+iD0Ok41D897Kff9S57o Ah9zsQxQCYiB2DcBz0/IDvRq/EdS7seX4AFec/R8H80L6gPJhf0GrLxMPX5+w99g kTf2skbblGRaTZT8P8LjUuzNhy4+JH0XrBaisqwFnsG1EFdBY+gHnj+/Rf3uqUA= =xh+L -----END PGP SIGNATURE----- --WYfJCIN5rqlfy3K0-- --===============5820298936242738441== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============5820298936242738441==--