netif_release_rx_bufs
PATCH 6/6 netif_release_rx_bufs
Hi,
This patch adds a netif_release_rx_bufs() function to the netfront driver. It intends to fix the rx buffer page leak. Unfortunaly it doesn't work perfectly, the reason is that reclaiming the pages granted to the backend driver works only if the backend driver gave them back already, filled with network data. Reclaiming unfilled rx buffers does NOT work.
I think we need either a way to get back pages with transfer grants without cooperation from the backend driver, or we need some way to say "pretty pretty please, give me back my rx buffers" to the netback driver.
comments?
Gerd
Signature · 2 lines
Gerd Hoffmann <[email protected]> http://www.suse.de/~kraxel/julika-dora.jpeg
drivers/xen/netfront/netfront.c +73 -0
@@ -494,6 +494,7 @@ static int network_open(struct net_devic {struct netfront_info *np =netdev_priv (dev); +DPRINTK ("%s\n" , np->xbdev->nodename);memset (&np->stats,0 ,sizeof (np->stats));network_alloc_rx_buffers (dev);
@@ -1285,10 +1286,80 @@ err:return more_to_do; } +static void netif_release_rx_bufs (struct netfront_info *np) +{ +struct mmu_update *mmu = np->rx_mmu; +struct multicall_entry *mcl = np->rx_mcl; +struct sk_buff *skb; +unsigned long mfn; +int bufs =0 , gnttab =0 , unused =0 ; +int id, ref; + +spin_lock (&np->rx_lock); + +for (id =0 ; id <NET_RX_RING_SIZE ; id++) { +if ((ref = np->grant_rx_ref[id]) ==GRANT_INVALID_REF ) { + unused++; +continue ; + } +if ((mfn =gnttab_end_foreign_transfer_ref (ref)) ==0 ) { + gnttab++; +continue ; + } + +gnttab_release_grant_reference (&np->gref_rx_head, ref); + np->grant_rx_ref[id] =GRANT_INVALID_REF ; + + skb = np->rx_skbs[id]; +add_id_to_freelist (np->rx_skbs, id); + +if (!xen_feature (XENFEAT_auto_translated_physmap)) { +/* Remap the page. */ +MULTI_update_va_mapping (mcl, (unsigned long )skb->head, +pfn_pte_ma (mfn,PAGE_KERNEL ), +0 ); + mcl++; + mmu->ptr = ((maddr_t )mfn <<PAGE_SHIFT ) + |MMU_MACHPHYS_UPDATE ; + mmu->val =__pa (skb->head) >>PAGE_SHIFT ; + mmu++; + +set_phys_to_machine (__pa (skb->head) >>PAGE_SHIFT , + mfn); + } + bufs++; + +#if 0 /* FIXME */ +dev_kfree_skb (skb); +#endif + } + +printk ("%s: %d released ok, %d gnttab errs, %d unused slots\n" , + __FUNCTION__, bufs, gnttab, unused); +if (0 == bufs) +return ; + +/* Some pages are no longer absent... */ +balloon_update_driver_allowance (-bufs); + +if (!xen_feature (XENFEAT_auto_translated_physmap)) { +/* Do all the remapping work, and M2P updates, in one big hypercall. */ + mcl->op = __HYPERVISOR_mmu_update; + mcl->args[0 ] = (unsigned long )np->rx_mmu; + mcl->args[1 ] = mmu - np->rx_mmu; + mcl->args[2 ] =0 ; + mcl->args[3 ] =DOMID_SELF ; + mcl++; +HYPERVISOR_multicall (np->rx_mcl, mcl - np->rx_mcl); + } + +spin_unlock (&np->rx_lock); +}static int network_close (struct net_device *dev) {struct netfront_info *np =netdev_priv (dev); +DPRINTK ("%s\n" , np->xbdev->nodename);netif_stop_queue (np->netdev);return 0 ; }
@@ -1427,6 +1498,8 @@ static void network_connect(struct net_dstatic void netif_uninit (struct net_device *dev) {struct netfront_info *np =netdev_priv (dev); +DPRINTK ("%s\n" , np->xbdev->nodename); +netif_release_rx_bufs (np);gnttab_free_grant_references (np->gref_tx_head);gnttab_free_grant_references (np->gref_rx_head); }
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 10 lines · Gerd Hoffmann
This patch adds a
netif_release_rx_bufs()function to the netfront driver. It intends to fix the rx buffer page leak. Unfortunaly it doesn't work perfectly, the reason is that reclaiming the pages granted to the backend driver works only if the backend driver gave them back already, filled with network data. Reclaiming unfilled rx buffers does NOT work.I think we need either a way to get back pages with transfer grants without cooperation from the backend driver, or we need some way to say "pretty pretty please, give me back my rx buffers" to the netback driver.
Unlike a map grant, you can reclaim a transfer grant at any time. The only question is whether it has been used to transfer a page, or not.
-- Keir
Quoted 6 lines · Keir Fraser
Quoted 3 lines
I think we need either a way to get back pages with transfer grants without cooperation from the backend driver, or we need some way to say "pretty pretty please, give me back my rx buffers" to the netback driver.
Unlike a map grant, you can reclaim a transfer grant at any time. The only question is whether it has been used to transfer a page, or not.
How can I do that? gnttab_end_foreign_transfer_ref() doesn't cut it, it will not succeed unconditionally ...
cheers,
Gerd
Signature · 2 lines
Gerd Hoffmann <[email protected]> http://www.suse.de/~kraxel/julika-dora.jpeg
Quoted 9 lines · Gerd Hoffmann
Quoted 6 lines
Quoted 3 lines
I think we need either a way to get back pages with transfer grants without cooperation from the backend driver, or we need some way to say "pretty pretty please, give me back my rx buffers" to the netback driver.
Unlike a map grant, you can reclaim a transfer grant at any time. The only question is whether it has been used to transfer a page, or not.
How can I do that?
gnttab_end_foreign_transfer_ref()doesn't cut it, it will not succeed unconditionally ...
The function will not spin unboundedly and, when it returns, the grant is definitely no longer active. If any page was transferred via the grant, it is returned to the caller (else zero is returned). It does just what you need.
-- Keir
Quoted 8 lines · Keir Fraser
Quoted 3 lines
How can I do that?
gnttab_end_foreign_transfer_ref()doesn't cut it, it will not succeed unconditionally ...The function will not spin unboundedly and, when it returns, the grant is definitely no longer active. If any page was transferred via the grant, it is returned to the caller (else zero is returned). It does just what you need.
Oh, ok. I assumed zero return means it failed. Guess I just have to keep track of the original mfns so I can map them back into the address space in case no page was transfered, right?
cheers,
Gerd
Signature · 2 lines
Gerd Hoffmann <[email protected]> http://www.suse.de/~kraxel/julika-dora.jpeg
Quoted 11 lines · Gerd Hoffmann
Quoted 7 lines
Quoted 2 lines
How can I do that?
gnttab_end_foreign_transfer_ref()doesn't cut it, it will not succeed unconditionally ...The function will not spin unboundedly and, when it returns, the grant is definitely no longer active. If any page was transferred via the grant, it is returned to the caller (else zero is returned). It does just what you need.
Oh, ok. I assumed zero return means it failed. Guess I just have to keep track of the original mfns so I can map them back into the address space in case no page was transfered, right?
You've given up the original mfns at this point, to ensure you have enough reservation headroom for the transfer. You'll be wanting to give the 'empty mfn' to the balloon driver, which can simply stick the page on its ballooned-out list. You might need to add a new API function to balloon driver to do this.
-- Keir
Quoted 5 lines · Keir Fraser
You've given up the original mfns at this point, to ensure you have enough reservation headroom for the transfer. You'll be wanting to give the 'empty mfn' to the balloon driver, which can simply stick the page on its ballooned-out list. You might need to add a new API function to balloon driver to do this.
Passing the page to the balloon driver works ok, but then I have trouble releasing the skb because shinfo(skb)->frags[0].page isn't valid any more. Guess I better aquire a page from xen in netfront instead of letting the ballon driver do that ...
cheers,
Gerd
Signature · 2 lines
Gerd Hoffmann <[email protected]> http://www.suse.de/~kraxel/julika-dora.jpeg
Quoted 11 lines · Gerd Hoffmann
Quoted 5 lines · Keir Fraser
You've given up the original mfns at this point, to ensure you have enough reservation headroom for the transfer. You'll be wanting to give the 'empty mfn' to the balloon driver, which can simply stick the page on its ballooned-out list. You might need to add a new API function to balloon driver to do this.
Passing the page to the balloon driver works ok, but then I have trouble releasing the skb because shinfo(skb)->frags[0].page isn't valid any more. Guess I better aquire a page from xen in netfront instead of letting the ballon driver do that ...
Best bet is to free the page to the balloon driver, and then set nr_frags to zero. This is perfectly valid -- it was netfront that set it to non-zero in the first place.
-- Keir
Hi,
Quoted 3 lines
Best bet is to free the page to the balloon driver, and then set
nr_fragsto zero. This is perfectly valid -- it was netfront that set it to non-zero in the first place.
Perfect. New version of the patch is attached below.
cheers,
Gerd
Signature · 2 lines
Gerd Hoffmann <[email protected]> http://www.suse.de/~kraxel/julika-dora.jpeg
drivers/xen/netfront/netfront.c +79 -0
@@ -494,6 +494,7 @@ static int network_open(struct net_devic {struct netfront_info *np =netdev_priv (dev); +DPRINTK ("%s\n" , np->xbdev->nodename);memset (&np->stats,0 ,sizeof (np->stats));network_alloc_rx_buffers (dev);
@@ -1285,10 +1286,86 @@ err:return more_to_do; } +static void netif_release_rx_bufs (struct netfront_info *np) +{ +struct mmu_update *mmu = np->rx_mmu; +struct multicall_entry *mcl = np->rx_mcl; +struct sk_buff *skb; +unsigned long mfn; +int xfer =0 , noxfer =0 , unused =0 ; +int id, ref, rc; + +printk ("%s: enter\n" , __FUNCTION__); + +spin_lock (&np->rx_lock); + +for (id =0 ; id <NET_RX_RING_SIZE ; id++) { +if ((ref = np->grant_rx_ref[id]) ==GRANT_INVALID_REF ) { + unused++; +continue ; + } + + skb = np->rx_skbs[id]; + mfn =gnttab_end_foreign_transfer_ref (ref); +gnttab_release_grant_reference (&np->gref_rx_head, ref); + np->grant_rx_ref[id] =GRANT_INVALID_REF ; +add_id_to_freelist (np->rx_skbs, id); + +if (0 == mfn) { +struct page *page =skb_shinfo (skb)->frags[0 ].page; +balloon_release_driver_page (page); +skb_shinfo (skb)->nr_frags =0 ; +dev_kfree_skb (skb); + noxfer++; +continue ; + } + +if (!xen_feature (XENFEAT_auto_translated_physmap)) { +/* Remap the page. */ +struct page *page =skb_shinfo (skb)->frags[0 ].page; +unsigned long pfn =page_to_pfn (page); +void *vaddr =page_address (page); + +MULTI_update_va_mapping (mcl, (unsigned long )vaddr, +pfn_pte_ma (mfn,PAGE_KERNEL ), +0 ); + mcl++; + mmu->ptr = ((maddr_t )mfn <<PAGE_SHIFT ) + |MMU_MACHPHYS_UPDATE ; + mmu->val = pfn; + mmu++; + +set_phys_to_machine (pfn, mfn); + } +dev_kfree_skb (skb); + xfer++; + } +printk ("%s: %d xfer, %d noxfer, %d unused\n" , + __FUNCTION__, xfer, noxfer, unused); + +if (xfer) { +/* Some pages are no longer absent... */ +balloon_update_driver_allowance (-xfer); + +if (!xen_feature (XENFEAT_auto_translated_physmap)) { +/* Do all the remapping work, and M2P updates, in one big hypercall. */ + mcl->op = __HYPERVISOR_mmu_update; + mcl->args[0 ] = (unsigned long )np->rx_mmu; + mcl->args[1 ] = mmu - np->rx_mmu; + mcl->args[2 ] =0 ; + mcl->args[3 ] =DOMID_SELF ; + mcl++; +HYPERVISOR_multicall (np->rx_mcl, mcl - np->rx_mcl); + } + } + +spin_unlock (&np->rx_lock); +}static int network_close (struct net_device *dev) {struct netfront_info *np =netdev_priv (dev); +DPRINTK ("%s\n" , np->xbdev->nodename);netif_stop_queue (np->netdev);return 0 ; }
@@ -1427,6 +1504,8 @@ static void network_connect(struct net_dstatic void netif_uninit (struct net_device *dev) {struct netfront_info *np =netdev_priv (dev); +DPRINTK ("%s\n" , np->xbdev->nodename); +netif_release_rx_bufs (np);gnttab_free_grant_references (np->gref_tx_head);gnttab_free_grant_references (np->gref_rx_head); }
drivers/xen/balloon/balloon.c +13 -0
@@ -612,8 +612,21 @@ void balloon_dealloc_empty_page_range(schedule_work (&balloon_worker); } +void balloon_release_driver_page (struct page *page) +{ +unsigned long flags; + +balloon_lock (flags); +balloon_append (page); + driver_pages--; +balloon_unlock (flags); + +schedule_work (&balloon_worker); +} +EXPORT_SYMBOL_GPL (balloon_update_driver_allowance);EXPORT_SYMBOL_GPL (balloon_alloc_empty_page_range);EXPORT_SYMBOL_GPL (balloon_dealloc_empty_page_range); +EXPORT_SYMBOL_GPL (balloon_release_driver_page);MODULE_LICENSE ("Dual BSD/GPL" );
drivers/xen/netfront/Makefile +1 -0
@@ -1,3 +1,4 @@ +EXTRA_CFLAGS += -DDEBUG=1 obj-$(CONFIG_XEN_NETDEV_FRONTEND) := xennet.o
include/xen/balloon.h +2 -0
@@ -52,6 +52,8 @@ extern voidballoon_dealloc_empty_page_range ( struct page *page, unsigned long nr_pages); +void balloon_release_driver_page (struct page *page); +/* * Prevent the balloon driver from changing the memory reservation during * a driver critical region.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel