Re: [Xen-devel] [RFC PATCH V3 04/16] netback: switch to per-cpu scratch space.

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, 2012-01-30 at 16:49 +0000, Viral Mehta wrote:
> Hi,
> 
> On Mon, Jan 30, 2012 at 9:45 AM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> >
> >        skb_queue_head_init(&rxq);
> > @@ -534,13 +527,16 @@ void xen_netbk_rx_action(struct xen_netbk *netbk)
> >                        break;
> >        }
> >
> > -       BUG_ON(npo.meta_prod > ARRAY_SIZE(netbk->meta));
> > +       BUG_ON(npo.meta_prod > MAX_PENDING_REQS);
> 
> While you are already here,
> how about having WARN_ON() ?
> 
> >
> > -       if (!npo.copy_prod)
> > +       if (!npo.copy_prod) {
> > +               put_cpu_ptr(gco);
> > +               put_cpu_ptr(m);
> >                return;
> > +       }
> >
> > -       BUG_ON(npo.copy_prod > ARRAY_SIZE(netbk->grant_copy_op));
> > -       ret = HYPERVISOR_grant_table_op(GNTTABOP_copy, 
> > &netbk->grant_copy_op,
> > +       BUG_ON(npo.copy_prod > (2 * XEN_NETIF_RX_RING_SIZE));
> 
> And may be here, too...
> 
> If there is serious bug, may be system will crash at a later point
> But, IMHO, WARN_ON() is the correct function for drivers at least.
> 

I don't agree. Here BUG_ON means code logic has defects that we haven't
discovered. I won't take the risk of any undefined behavior.

Further more, there are a bunch of drivers use BUG_ON. A simple grep
will show you hundreds (even thousands?) of results.


Wei.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel