Re: [Xen-devel] [PATCH net-next V2 2/3] xen-netback: avoid allocating variable size array on stack

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-05-02 at 13:04 +0100, Jan Beulich wrote:
> >>> On 02.05.13 at 12:43, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> > @@ -934,11 +934,14 @@ static int netbk_count_requests(struct xenvif *vif,
> >     RING_IDX cons = vif->tx.req_cons;
> >     int slots = 0;
> >     int drop_err = 0;
> > +   int more_data;
> >  
> >     if (!(first->flags & XEN_NETTXF_more_data))
> >             return 0;
> >  
> >     do {
> > +           struct xen_netif_tx_request dropped_tx = { 0 };
> > +
> 
> No need for an initializer here.
> 
> >             if (slots >= work_to_do) {
> >                     netdev_err(vif->dev,
> >                                "Asked for %d slots but exceeds this 
> > limit\n",
> > @@ -972,6 +975,9 @@ static int netbk_count_requests(struct xenvif *vif,
> >                     drop_err = -E2BIG;
> >             }
> >  
> > +           if (drop_err)
> > +                   txp = &dropped_tx;
> > +
> >             memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + slots),
> >                    sizeof(*txp));
> >  
> > @@ -1001,7 +1007,13 @@ static int netbk_count_requests(struct xenvif *vif,
> >                     netbk_fatal_tx_err(vif);
> >                     return -EINVAL;
> >             }
> > -   } while ((txp++)->flags & XEN_NETTXF_more_data);
> > +
> > +           more_data = txp->flags & XEN_NETTXF_more_data;
> > +
> > +           if (!drop_err)
> > +                   txp++;
> 
> And no need for the conditional here afaict.

I think it is needed, in the case where you've assigned txp =
&dropped_tx you don't want to increment txp.

Or perhaps it just gets reassigned back to &dropped_tx at the top of the
next loop, so the increment doesn't matter. Subtle! I'm happy with
whichever way Wei prefers, but it is probably worthy of a comment

> 
> Jan
> 
> > +
> > +   } while (more_data);
> >  
> >     if (drop_err) {
> >             netbk_tx_err(vif, first, cons + slots);
> 
> 



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