| Message ID | 1368783838-24187-1-git-send-email-wei.liu2@citrix.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Wei Liu <wei.liu2@citrix.com> Date: Fri, 17 May 2013 10:43:58 +0100 > > + /* If we fail to setup netfront, it is safe to just revoke access to granted > + * pages because backend is not accessing it at this point. > + */ > + bind_fail: > + xenbus_free_evtchn(dev, info->evtchn); > + alloc_evtchn_fail: > + gnttab_end_foreign_access_ref(info->rx_ring_ref, 0); > + grant_rx_ring_fail: > + free_page((unsigned long)rxs); > + alloc_rx_ring_fail: > + gnttab_end_foreign_access_ref(info->tx_ring_ref, 0); > + grant_tx_ring_fail: > + free_page((unsigned long)txs); > fail: > return err; > } This is not even close to adhering to proper coding style. The comment should be indented by one tab not one space: /* Like * this. */ And the code labels should appear at the first column, not with a leading space. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, May 19, 2013 at 11:28:33PM -0700, David Miller wrote: > From: Wei Liu <wei.liu2@citrix.com> > Date: Fri, 17 May 2013 10:43:58 +0100 > > > > > + /* If we fail to setup netfront, it is safe to just revoke access to granted > > + * pages because backend is not accessing it at this point. > > + */ > > + bind_fail: > > + xenbus_free_evtchn(dev, info->evtchn); > > + alloc_evtchn_fail: > > + gnttab_end_foreign_access_ref(info->rx_ring_ref, 0); > > + grant_rx_ring_fail: > > + free_page((unsigned long)rxs); > > + alloc_rx_ring_fail: > > + gnttab_end_foreign_access_ref(info->tx_ring_ref, 0); > > + grant_tx_ring_fail: > > + free_page((unsigned long)txs); > > fail: > > return err; > > } > > This is not even close to adhering to proper coding style. > > The comment should be indented by one tab not one space: > > /* Like > * this. > */ > > And the code labels should appear at the first column, not > with a leading space. Thanks, coding style problems are fixed now. For some reason some other labels in this file appear to have the same style problem, I will fix them when I touch other part of the file in the future. Wei. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 1db10141..2dff4eb 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1532,39 +1532,48 @@ static int setup_netfront(struct xenbus_device *dev, struct netfront_info *info) FRONT_RING_INIT(&info->tx, txs, PAGE_SIZE); err = xenbus_grant_ring(dev, virt_to_mfn(txs)); - if (err < 0) { - free_page((unsigned long)txs); - goto fail; - } + if (err < 0) + goto grant_tx_ring_fail; info->tx_ring_ref = err; rxs = (struct xen_netif_rx_sring *)get_zeroed_page(GFP_NOIO | __GFP_HIGH); if (!rxs) { err = -ENOMEM; xenbus_dev_fatal(dev, err, "allocating rx ring page"); - goto fail; + goto alloc_rx_ring_fail; } SHARED_RING_INIT(rxs); FRONT_RING_INIT(&info->rx, rxs, PAGE_SIZE); err = xenbus_grant_ring(dev, virt_to_mfn(rxs)); - if (err < 0) { - free_page((unsigned long)rxs); - goto fail; - } + if (err < 0) + goto grant_rx_ring_fail; info->rx_ring_ref = err; err = xenbus_alloc_evtchn(dev, &info->evtchn); if (err) - goto fail; + goto alloc_evtchn_fail; err = bind_evtchn_to_irqhandler(info->evtchn, xennet_interrupt, 0, netdev->name, netdev); if (err < 0) - goto fail; + goto bind_fail; netdev->irq = err; return 0; + /* If we fail to setup netfront, it is safe to just revoke access to granted + * pages because backend is not accessing it at this point. + */ + bind_fail: + xenbus_free_evtchn(dev, info->evtchn); + alloc_evtchn_fail: + gnttab_end_foreign_access_ref(info->rx_ring_ref, 0); + grant_rx_ring_fail: + free_page((unsigned long)rxs); + alloc_rx_ring_fail: + gnttab_end_foreign_access_ref(info->tx_ring_ref, 0); + grant_tx_ring_fail: + free_page((unsigned long)txs); fail: return err; }
We should correctly free related resources (grant ref, memory page, evtchn) when setup_netfront fails. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- drivers/net/xen-netfront.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-)