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

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2077 bytes --]

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 #1.2: Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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