Mailing List Archive

[PATCH 6 of 9] xenpaging: add evict_pages function
# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1316067230 -7200
# Node ID 3a3a5979b799d948802183d10d65894ee84a872f
# Parent 6beca8cbc2c92900859712f8738db17084bcebdb
xenpaging: add evict_pages function

Add new function to evict a couple of pages.
Allocate all possible slots in a paging file to allow growing and
shrinking of the number of paged-out pages. Adjust other places to
iterate over all slots.

This change is required by subsequent patches.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r 6beca8cbc2c9 -r 3a3a5979b799 tools/xenpaging/xenpaging.c
--- a/tools/xenpaging/xenpaging.c
+++ b/tools/xenpaging/xenpaging.c
@@ -597,6 +597,30 @@ static int evict_victim(xenpaging_t *pag
return ret;
}

+/* Evict a couple of pages and write them to a free slot in the paging file */
+static int evict_pages(xenpaging_t *paging, int fd, xenpaging_victim_t *victims, int num_pages)
+{
+ xc_interface *xch = paging->xc_handle;
+ int rc, slot, num = 0;
+
+ for ( slot = 0; slot < paging->max_pages && num < num_pages; slot++ )
+ {
+ /* Slot is allocated */
+ if ( victims[slot].gfn != INVALID_MFN )
+ continue;
+
+ rc = evict_victim(paging, &victims[slot], fd, slot);
+ if ( rc == -ENOSPC )
+ break;
+ if ( rc == -EINTR )
+ break;
+ if ( num && num % 100 == 0 )
+ DPRINTF("%d pages evicted\n", num);
+ num++;
+ }
+ return num;
+}
+
int main(int argc, char *argv[])
{
struct sigaction act;
@@ -639,7 +663,12 @@ int main(int argc, char *argv[])
return 2;
}

- victims = calloc(paging->num_pages, sizeof(xenpaging_victim_t));
+ /* Allocate upper limit of pages to allow growing and shrinking */
+ victims = calloc(paging->max_pages, sizeof(xenpaging_victim_t));
+
+ /* Mark all slots as unallocated */
+ for ( i = 0; i < paging->max_pages; i++ )
+ victims[i].gfn = INVALID_MFN;

/* ensure that if we get a signal, we'll do cleanup, then exit */
act.sa_handler = close_handler;
@@ -653,18 +682,7 @@ int main(int argc, char *argv[])
/* listen for page-in events to stop pager */
create_page_in_thread(paging);

- /* Evict pages */
- for ( i = 0; i < paging->num_pages; i++ )
- {
- rc = evict_victim(paging, &victims[i], fd, i);
- if ( rc == -ENOSPC )
- break;
- if ( rc == -EINTR )
- break;
- if ( i % 100 == 0 )
- DPRINTF("%d pages evicted\n", i);
- }
-
+ i = evict_pages(paging, fd, victims, paging->num_pages);
DPRINTF("%d pages evicted. Done.\n", i);

/* Swap pages in and out */
@@ -690,13 +708,13 @@ int main(int argc, char *argv[])
if ( test_and_clear_bit(req.gfn, paging->bitmap) )
{
/* Find where in the paging file to read from */
- for ( i = 0; i < paging->num_pages; i++ )
+ for ( i = 0; i < paging->max_pages; i++ )
{
if ( victims[i].gfn == req.gfn )
break;
}

- if ( i >= paging->num_pages )
+ if ( i >= paging->max_pages )
{
DPRINTF("Couldn't find page %"PRIx64"\n", req.gfn);
goto out;

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 6 of 9] xenpaging: add evict_pages function [ In reply to ]
On Thu, 2011-09-15 at 02:16 -0400, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1316067230 -7200
> # Node ID 3a3a5979b799d948802183d10d65894ee84a872f
> # Parent 6beca8cbc2c92900859712f8738db17084bcebdb
> xenpaging: add evict_pages function
>
> Add new function to evict a couple of pages.

Do you really mean "a couple" here? (that generally means exactly two).
>From the implementation I think you mean evict a batch of pages?

> Allocate all possible slots in a paging file to allow growing and
> shrinking of the number of paged-out pages. Adjust other places to
> iterate over all slots.
>
> This change is required by subsequent patches.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>
> diff -r 6beca8cbc2c9 -r 3a3a5979b799 tools/xenpaging/xenpaging.c
> --- a/tools/xenpaging/xenpaging.c
> +++ b/tools/xenpaging/xenpaging.c
> @@ -597,6 +597,30 @@ static int evict_victim(xenpaging_t *pag
> return ret;
> }
>
> +/* Evict a couple of pages and write them to a free slot in the paging file */
> +static int evict_pages(xenpaging_t *paging, int fd, xenpaging_victim_t *victims, int num_pages)
> +{
> + xc_interface *xch = paging->xc_handle;
> + int rc, slot, num = 0;
> +
> + for ( slot = 0; slot < paging->max_pages && num < num_pages; slot++ )
> + {
> + /* Slot is allocated */
> + if ( victims[slot].gfn != INVALID_MFN )
> + continue;
> +
> + rc = evict_victim(paging, &victims[slot], fd, slot);
> + if ( rc == -ENOSPC )
> + break;
> + if ( rc == -EINTR )
> + break;
> + if ( num && num % 100 == 0 )
> + DPRINTF("%d pages evicted\n", num);
> + num++;
> + }
> + return num;
> +}
> +
> int main(int argc, char *argv[])
> {
> struct sigaction act;
> @@ -639,7 +663,12 @@ int main(int argc, char *argv[])
> return 2;
> }
>
> - victims = calloc(paging->num_pages, sizeof(xenpaging_victim_t));
> + /* Allocate upper limit of pages to allow growing and shrinking */
> + victims = calloc(paging->max_pages, sizeof(xenpaging_victim_t));
> +
> + /* Mark all slots as unallocated */
> + for ( i = 0; i < paging->max_pages; i++ )
> + victims[i].gfn = INVALID_MFN;
>
> /* ensure that if we get a signal, we'll do cleanup, then exit */
> act.sa_handler = close_handler;
> @@ -653,18 +682,7 @@ int main(int argc, char *argv[])
> /* listen for page-in events to stop pager */
> create_page_in_thread(paging);
>
> - /* Evict pages */
> - for ( i = 0; i < paging->num_pages; i++ )
> - {
> - rc = evict_victim(paging, &victims[i], fd, i);
> - if ( rc == -ENOSPC )
> - break;
> - if ( rc == -EINTR )
> - break;
> - if ( i % 100 == 0 )
> - DPRINTF("%d pages evicted\n", i);
> - }
> -
> + i = evict_pages(paging, fd, victims, paging->num_pages);
> DPRINTF("%d pages evicted. Done.\n", i);
>
> /* Swap pages in and out */
> @@ -690,13 +708,13 @@ int main(int argc, char *argv[])
> if ( test_and_clear_bit(req.gfn, paging->bitmap) )
> {
> /* Find where in the paging file to read from */
> - for ( i = 0; i < paging->num_pages; i++ )
> + for ( i = 0; i < paging->max_pages; i++ )
> {
> if ( victims[i].gfn == req.gfn )
> break;
> }
>
> - if ( i >= paging->num_pages )
> + if ( i >= paging->max_pages )
> {
> DPRINTF("Couldn't find page %"PRIx64"\n", req.gfn);
> goto out;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 6 of 9] xenpaging: add evict_pages function [ In reply to ]
On Thu, Sep 15, 2011 at 9:16 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Thu, 2011-09-15 at 02:16 -0400, Olaf Hering wrote:
>> # HG changeset patch
>> # User Olaf Hering <olaf@aepfle.de>
>> # Date 1316067230 -7200
>> # Node ID 3a3a5979b799d948802183d10d65894ee84a872f
>> # Parent  6beca8cbc2c92900859712f8738db17084bcebdb
>> xenpaging: add evict_pages function
>>
>> Add new function to evict a couple of pages.
>
> Do you really mean "a couple" here? (that generally means exactly two).

LIterally "couple" means two, but at least in US idiom, "a couple of
[foo]" means a small indeterminate number, usually 2-4.

In any case, a more precise description seems like a better idea -- it
looks like it takes an argument for the number of pages to evict; and
it's not adding a new function, it's pulling existing code into a
function. So, "Pull eviction loop into a function" would probably be
a better description.

-George

> >From the implementation I think you mean evict a batch of pages?
>
>> Allocate all possible slots in a paging file to allow growing and
>> shrinking of the number of paged-out pages. Adjust other places to
>> iterate over all slots.
>>
>> This change is required by subsequent patches.
>>
>> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>>
>> diff -r 6beca8cbc2c9 -r 3a3a5979b799 tools/xenpaging/xenpaging.c
>> --- a/tools/xenpaging/xenpaging.c
>> +++ b/tools/xenpaging/xenpaging.c
>> @@ -597,6 +597,30 @@ static int evict_victim(xenpaging_t *pag
>>      return ret;
>>  }
>>
>> +/* Evict a couple of pages and write them to a free slot in the paging file */
>> +static int evict_pages(xenpaging_t *paging, int fd, xenpaging_victim_t *victims, int num_pages)
>> +{
>> +    xc_interface *xch = paging->xc_handle;
>> +    int rc, slot, num = 0;
>> +
>> +    for ( slot = 0; slot < paging->max_pages && num < num_pages; slot++ )
>> +    {
>> +        /* Slot is allocated */
>> +        if ( victims[slot].gfn != INVALID_MFN )
>> +            continue;
>> +
>> +        rc = evict_victim(paging, &victims[slot], fd, slot);
>> +        if ( rc == -ENOSPC )
>> +            break;
>> +        if ( rc == -EINTR )
>> +            break;
>> +        if ( num && num % 100 == 0 )
>> +            DPRINTF("%d pages evicted\n", num);
>> +        num++;
>> +    }
>> +    return num;
>> +}
>> +
>>  int main(int argc, char *argv[])
>>  {
>>      struct sigaction act;
>> @@ -639,7 +663,12 @@ int main(int argc, char *argv[])
>>          return 2;
>>      }
>>
>> -    victims = calloc(paging->num_pages, sizeof(xenpaging_victim_t));
>> +    /* Allocate upper limit of pages to allow growing and shrinking */
>> +    victims = calloc(paging->max_pages, sizeof(xenpaging_victim_t));
>> +
>> +    /* Mark all slots as unallocated */
>> +    for ( i = 0; i < paging->max_pages; i++ )
>> +        victims[i].gfn = INVALID_MFN;
>>
>>      /* ensure that if we get a signal, we'll do cleanup, then exit */
>>      act.sa_handler = close_handler;
>> @@ -653,18 +682,7 @@ int main(int argc, char *argv[])
>>      /* listen for page-in events to stop pager */
>>      create_page_in_thread(paging);
>>
>> -    /* Evict pages */
>> -    for ( i = 0; i < paging->num_pages; i++ )
>> -    {
>> -        rc = evict_victim(paging, &victims[i], fd, i);
>> -        if ( rc == -ENOSPC )
>> -            break;
>> -        if ( rc == -EINTR )
>> -            break;
>> -        if ( i % 100 == 0 )
>> -            DPRINTF("%d pages evicted\n", i);
>> -    }
>> -
>> +    i = evict_pages(paging, fd, victims, paging->num_pages);
>>      DPRINTF("%d pages evicted. Done.\n", i);
>>
>>      /* Swap pages in and out */
>> @@ -690,13 +708,13 @@ int main(int argc, char *argv[])
>>              if ( test_and_clear_bit(req.gfn, paging->bitmap) )
>>              {
>>                  /* Find where in the paging file to read from */
>> -                for ( i = 0; i < paging->num_pages; i++ )
>> +                for ( i = 0; i < paging->max_pages; i++ )
>>                  {
>>                      if ( victims[i].gfn == req.gfn )
>>                          break;
>>                  }
>>
>> -                if ( i >= paging->num_pages )
>> +                if ( i >= paging->max_pages )
>>                  {
>>                      DPRINTF("Couldn't find page %"PRIx64"\n", req.gfn);
>>                      goto out;
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 6 of 9] xenpaging: add evict_pages function [ In reply to ]
On Thu, Sep 15, George Dunlap wrote:

> On Thu, Sep 15, 2011 at 9:16 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2011-09-15 at 02:16 -0400, Olaf Hering wrote:
> >> # HG changeset patch
> >> # User Olaf Hering <olaf@aepfle.de>
> >> # Date 1316067230 -7200
> >> # Node ID 3a3a5979b799d948802183d10d65894ee84a872f
> >> # Parent  6beca8cbc2c92900859712f8738db17084bcebdb
> >> xenpaging: add evict_pages function
> >>
> >> Add new function to evict a couple of pages.
> >
> > Do you really mean "a couple" here? (that generally means exactly two).
>
> LIterally "couple" means two, but at least in US idiom, "a couple of
> [foo]" means a small indeterminate number, usually 2-4.
>
> In any case, a more precise description seems like a better idea -- it
> looks like it takes an argument for the number of pages to evict; and
> it's not adding a new function, it's pulling existing code into a
> function. So, "Pull eviction loop into a function" would probably be
> a better description.


Thanks to both of you, I will improve the description.

Olaf

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 6 of 9] xenpaging: add evict_pages function [ In reply to ]
Olaf Hering writes ("Re: [Xen-devel] [PATCH 6 of 9] xenpaging: add evict_pages function"):
> On Thu, Sep 15, George Dunlap wrote:
> > In any case, a more precise description seems like a better idea -- it
> > looks like it takes an argument for the number of pages to evict; and
> > it's not adding a new function, it's pulling existing code into a
> > function. So, "Pull eviction loop into a function" would probably be
> > a better description.
>
> Thanks to both of you, I will improve the description.

Thanks. Just to be clear, I think I'm expecting a repost either of
this patch, or perhaps of the whole series ? Ian C made a comment
about combining two of your later patches, too.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH 6 of 9] xenpaging: add evict_pages function [ In reply to ]
On Wed, Sep 28, Ian Jackson wrote:

> Olaf Hering writes ("Re: [Xen-devel] [PATCH 6 of 9] xenpaging: add evict_pages function"):
> > On Thu, Sep 15, George Dunlap wrote:
> > > In any case, a more precise description seems like a better idea -- it
> > > looks like it takes an argument for the number of pages to evict; and
> > > it's not adding a new function, it's pulling existing code into a
> > > function. So, "Pull eviction loop into a function" would probably be
> > > a better description.
> >
> > Thanks to both of you, I will improve the description.
>
> Thanks. Just to be clear, I think I'm expecting a repost either of
> this patch, or perhaps of the whole series ? Ian C made a comment
> about combining two of your later patches, too.

Yes, I will post a new series, replacing this one.

Olaf

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