From mboxrd@z Thu Jan  1 00:00:00 1970
From: Olaf Hering <olaf@aepfle.de>
Subject: Re: [PATCH] tools: fix build after recent xenpaging
	changes
Date: Fri, 24 Jun 2011 17:24:45 +0200
Message-ID: <20110624152445.GA30547@aepfle.de>
References: <20110624121613.GA17634@whitby.uk.xensource.com>
	<1308918826.32717.85.camel@zakaz.uk.xensource.com>
	<20110624133201.GB27793@aepfle.de>
	<1308923299.32717.109.camel@zakaz.uk.xensource.com>
	<20110624135708.GA28839@aepfle.de>
	<1308925806.32717.119.camel@zakaz.uk.xensource.com>
	<20110624143511.GA30080@aepfle.de>
	<1308926473.32717.129.camel@zakaz.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Return-path: <xen-devel-bounces@lists.xensource.com>
Content-Disposition: inline
In-Reply-To: <1308926473.32717.129.camel@zakaz.uk.xensource.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Ian Campbell <Ian.Campbell@eu.citrix.com>
Cc: Tim Deegan <Tim.Deegan@eu.citrix.com>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
List-Id: xen-devel@lists.xenproject.org

On Fri, Jun 24, Ian Campbell wrote:

> On Fri, 2011-06-24 at 15:35 +0100, Olaf Hering wrote:
> > On Fri, Jun 24, Ian Campbell wrote:
> > 
> > > > The break exits the for() loop, not the while(1). In the next iteration
> > > > page 1 may still be in paging->bitmap and tried again.
> > > 
> > > I'd missed the interrupted -> 1 in the while loop. I presume there is
> > > some other exit condition which triggers once everything has been paged
> > > back in and actually causes the daemon to exit?
> > 
> > If the for() loop found nothing, then this triggers:
> > 
> > 775             /* If no more pages to process, exit loop */
> > 776             if ( i == paging->domain_info->max_pages )
> > 777                 break;
> 
> Oh yes.
> 
> It's a bit counter intuitive to have a for loop which only processes the
> first thing it finds and then relying on going round another outer loop
> to pickup the second etc. It's at least O(N^2), isn't it?
> 
> Why not 
> 	count = 0;
> 	for ( i = 0; i < paging->domain_info->max_pages; i++ )
>             {
>                 if ( test_bit(i, paging->bitmap) )
>                 {
>                     page_in_trigger(i);
>                     count++
>                 }
>             }
>             /* If no more pages to process, exit loop */
>             if ( !count )
>                 break;
> 
> That will at least process as many pages as it can on each iteration
> through the outer loop. Although it will most likely exacerbate the
> locking issue I pointed to earlier.

I think an early version of my change had something like that, but it
did not fillup the ringbuffer for some reason.
I will look at this change again and see what can be improved.

Olaf