Re: [Xen-devel] [Minios-devel] Double gnttab_end_access in mini-os netfront

[Marek Marczykowski-GÃrecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

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Ãrecki, on Tue 08 Dec 2015 12:46:31 +0100, wrote:
> > > > > http://xenbits.xen.org/gitweb/?p=mini-os.git;a=commit;h=7c8f3483906
> > > > > 52a67e
> > > > > 9356eec9cd2b0f76a9c7c72
> > > > > 
> > > > > With that commit reverted, issue vanishes.
> > > > > 
> > > > > I guess it's because before this commit, there was "if (rx->status
> > > > > ==
> > > > > NETIF_RSP_NULL) continue" before "gnttab_end_access(buf->gref)",
> > > > > but now
> > > > > that case is handled after gnttab_end_access (using "if (rx->status 
> > > > > >
> > > > > NETIF_RSP_NULL)"). I think the fix would be to restore that
> > > > > "continue"
> > > > > line.
> > > > 
> > > > That sounds pretty plausible to me (FWIW). Have you tried it?
> > > 
> > > I've tried moving gnttab_end_access into that if branch. And it didn't
> > > worked.
> > 
> > Which if branch?ÂÂPlease show the code, C is less ambiguous than english
> > :)
> 
> 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 @@
 
         buf = &dev->rx_buffers[id];
         page = (unsigned char*)buf->page;
-        gnttab_end_access(buf->gref);
 
         if (rx->status > NETIF_RSP_NULL)
         {
+        gnttab_end_access(buf->gref);
 #ifdef HAVE_LIBC
            if (dev->netif_rx == NETIF_SELECT_RX) {
                int len = 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.

-- 
Best Regards,
Marek Marczykowski-GÃrecki
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?

Attachment: pgpum9Y5spXVP.pgp
Description: PGP signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel