From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Fitzhardinge Subject: Re: [PV-ops][PATCH] Netback: Fix PV network issue for netback multiple threads patchset Date: Thu, 01 Jul 2010 17:29:07 +0200 Message-ID: <4C2CB443.1060907@goop.org> References: <1276248930.19091.2870.camel@zakaz.uk.xensource.com> <4C1F49B1.3060403@goop.org> <1277995730.28432.24.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1277995730.28432.24.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Campbell Cc: xen-devel@lists.xensource.com, Fantu , "Xu, Dongxiao" , Paul Durrant , djmagee@mageenet.net List-Id: xen-devel@lists.xenproject.org On 07/01/2010 04:48 PM, Ian Campbell wrote: > On Mon, 2010-06-21 at 12:14 +0100, Jeremy Fitzhardinge wrote: > >> Subject: [PATCH] xen/netback: make sure all the group structures are initialized before starting async code >> >> Split the netbk group array initialization into two phases: one to do >> simple "can't fail" initialization of lists, timers, locks, etc; and a >> second pass to allocate memory and start the async code. >> >> This makes sure that all the basic stuff is in place by the time the async code >> is running. >> > Paul noticed a crash in netback init because... > > >> @@ -1764,16 +1766,6 @@ static int __init netback_init(void) >> netbk->netbk_tx_pending_timer.function = >> netbk_tx_pending_timeout; >> >> - netbk->mmap_pages = >> - alloc_empty_pages_and_pagevec(MAX_PENDING_REQS); >> - if (!netbk->mmap_pages) { >> - printk(KERN_ALERT "%s: out of memory\n", __func__); >> - del_timer(&netbk->netbk_tx_pending_timer); >> - del_timer(&netbk->net_timer); >> - rc = -ENOMEM; >> - goto failed_init; >> - } >> - >> for (i = 0; i < MAX_PENDING_REQS; i++) { >> page = netbk->mmap_pages[i]; >> SetPageForeign(page, netif_page_release); >> > ...this dereference of netbk->mmap_pages[i]... > > >> @@ -1786,6 +1778,26 @@ static int __init netback_init(void) >> > [...] > >> + netbk->mmap_pages = >> + alloc_empty_pages_and_pagevec(MAX_PENDING_REQS); >> > ... happens before this initialisation of the array. > Hm, I hadn't meant to commit that properly. I had it locally and accidentally pushed it out. I only did that patch as an RFC in response to an issue alluded to by Dongxiao (or was it you?) about things not being fully initialized by the time the async code starts. Is this a real issue, and if so, what's the correct fix? Thanks, J