| Message ID | 20160325183320.GB2792@var (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/25/2016 11:33 AM, Samuel Thibault wrote: > Wei Liu, on Fri 25 Mar 2016 13:09:07 +0000, wrote: >> CC Samuel > > Thanks. > >> On Wed, Mar 23, 2016 at 02:26:51PM -0700, Sarah Newman wrote: >>> 7c8f3483 introduced a break within a loop in netfront.c such that >>> cons and nr_consumed were no longer always being incremented. The >>> offset at cons will be processed multiple times with the break in >>> place. >>> >>> Remove the break and re-add "some !=0" in the loop for HAVE_LIBC. > > Mmm, right. > > That ifdef makes things even more difficult to understand then. That > however makes me think: how about the attached patch, which actually > simplifies the rest. > > Thanks! > Samuel > I have no objections to backing out additional changes made in 7c8f3483, but I wasn't the person who submitted that patch. The patch tests OK with GNT_DEBUG enabled. --Sarah
Sarah Newman, on Fri 25 Mar 2016 12:19:23 -0700, wrote: > I have no objections to backing out additional changes made in 7c8f3483, ? My patch doesn't really back out more than what you proposed actually. > The patch tests OK with GNT_DEBUG enabled. Good :) Samuel
On 03/25/2016 12:32 PM, Samuel Thibault wrote: > Sarah Newman, on Fri 25 Mar 2016 12:19:23 -0700, wrote: >> I have no objections to backing out additional changes made in 7c8f3483, > > ? My patch doesn't really back out more than what you proposed actually. It also backs out the #ifdef's on HAVE_LIBC, but yes it's functionally equivalent. --Sarah
On 03/25/2016 11:33 AM, Samuel Thibault wrote: >> On Wed, Mar 23, 2016 at 02:26:51PM -0700, Sarah Newman wrote: >>> 7c8f3483 introduced a break within a loop in netfront.c such that >>> cons and nr_consumed were no longer always being incremented. The >>> offset at cons will be processed multiple times with the break in >>> place. >>> >>> Remove the break and re-add "some !=0" in the loop for HAVE_LIBC. > > Mmm, right. > > That ifdef makes things even more difficult to understand then. That > however makes me think: how about the attached patch, which actually > simplifies the rest. > > Thanks! > Samuel > Does anything else need to happen for this patch to be used with xen master and stable 4.6? Thanks, Sarah
On Sat, Mar 26, 2016 at 01:53:28PM -0700, Sarah Newman wrote: > On 03/25/2016 11:33 AM, Samuel Thibault wrote: > >> On Wed, Mar 23, 2016 at 02:26:51PM -0700, Sarah Newman wrote: > >>> 7c8f3483 introduced a break within a loop in netfront.c such that > >>> cons and nr_consumed were no longer always being incremented. The > >>> offset at cons will be processed multiple times with the break in > >>> place. > >>> > >>> Remove the break and re-add "some !=0" in the loop for HAVE_LIBC. > > > > Mmm, right. > > > > That ifdef makes things even more difficult to understand then. That > > however makes me think: how about the attached patch, which actually > > simplifies the rest. > > > > Thanks! > > Samuel > > > > Does anything else need to happen for this patch to be used with xen master and stable 4.6? > I take it that you and Samuel have reached an agreement on how to proceed? Can either of you submit a proper final patch to xen-devel and minios-devel and CC me please? I will see to its backport. Note that we are near 4.7 freeze at the moment, so it might take some time for me to get to it. But I don't think I will miss it because I want the patch to be in for 4.7. Wei. > Thanks, Sarah
diff --git a/netfront.c b/netfront.c index 0eca5b5..b8fac62 100644 --- a/netfront.c +++ b/netfront.c @@ -97,19 +97,15 @@ void network_rx(struct netfront_dev *dev) { RING_IDX rp,cons,req_prod; int nr_consumed, more, i, notify; -#ifdef HAVE_LIBC - int some; -#endif + int dobreak; nr_consumed = 0; moretodo: rp = dev->rx.sring->rsp_prod; rmb(); /* Ensure we see queued responses up to 'rp'. */ -#ifdef HAVE_LIBC - some = 0; -#endif - for (cons = dev->rx.rsp_cons; cons != rp; nr_consumed++, cons++) + dobreak = 0; + for (cons = dev->rx.rsp_cons; cons != rp && !dobreak; nr_consumed++, cons++) { struct net_buffer* buf; unsigned char* page; @@ -134,8 +130,8 @@ moretodo: len = dev->len; memcpy(dev->data, page+rx->offset, len); dev->rlen = len; - some = 1; - break; + /* No need to receive the rest for now */ + dobreak = 1; } else #endif dev->netif_rx(page+rx->offset,rx->status); @@ -144,11 +140,7 @@ moretodo: dev->rx.rsp_cons=cons; RING_FINAL_CHECK_FOR_RESPONSES(&dev->rx,more); -#ifdef HAVE_LIBC - if(more && !some) goto moretodo; -#else - if(more) goto moretodo; -#endif + if(more && !dobreak) goto moretodo; req_prod = dev->rx.req_prod_pvt;