Re: [Xen-devel] [PATCHES v8 1/8] mm: Place unscrubbed pages at the end of pagelist

2017-08-21 Thread Julien Grall 

Hi Jan,

On 21/08/17 14:49, Jan Beulich wrote:

On 17.08.17 at 12:30,  wrote:

On 16/08/17 19:33, Boris Ostrovsky wrote:

.. so that it's easy to find pages that need to be scrubbed (those pages are
now marked with _PGC_need_scrub bit).

We keep track of the first unscrubbed page in a page buddy using first_dirty
field. For now it can have two values, 0 (whole buddy needs scrubbing) or
INVALID_DIRTY_IDX (the buddy does not need to be scrubbed). Subsequent

patches

will allow scrubbing to be interrupted, resulting in first_dirty taking any
value.

Signed-off-by: Boris Ostrovsky 


For the ARM bits:

Acked-by: Julien Grall 


I've started committing the series when I noticed patches 4, 5, and 6
are still lacking ARM side acks.


Whoops, thank you for the remainder. You can ack my ack on the 4, 5, and 6:

Acked-by: Julien Grall 

Cheers,


--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCHES v8 1/8] mm: Place unscrubbed pages at the end of pagelist

2017-08-21 Thread Jan Beulich 
>>> On 17.08.17 at 12:30,  wrote:
> On 16/08/17 19:33, Boris Ostrovsky wrote:
>> .. so that it's easy to find pages that need to be scrubbed (those pages are
>> now marked with _PGC_need_scrub bit).
>>
>> We keep track of the first unscrubbed page in a page buddy using first_dirty
>> field. For now it can have two values, 0 (whole buddy needs scrubbing) or
>> INVALID_DIRTY_IDX (the buddy does not need to be scrubbed). Subsequent 
> patches
>> will allow scrubbing to be interrupted, resulting in first_dirty taking any
>> value.
>>
>> Signed-off-by: Boris Ostrovsky 
> 
> For the ARM bits:
> 
> Acked-by: Julien Grall 

I've started committing the series when I noticed patches 4, 5, and 6
are still lacking ARM side acks.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCHES v8 1/8] mm: Place unscrubbed pages at the end of pagelist

2017-08-18 Thread Boris Ostrovsky 
On 08/18/2017 05:11 AM, Jan Beulich wrote:
 On 16.08.17 at 20:33,  wrote:
>> .. so that it's easy to find pages that need to be scrubbed (those pages are
>> now marked with _PGC_need_scrub bit).
>>
>> We keep track of the first unscrubbed page in a page buddy using first_dirty
>> field. For now it can have two values, 0 (whole buddy needs scrubbing) or
>> INVALID_DIRTY_IDX (the buddy does not need to be scrubbed). Subsequent 
>> patches
>> will allow scrubbing to be interrupted, resulting in first_dirty taking any
>> value.
>>
>> Signed-off-by: Boris Ostrovsky 
> Reviewed-by: Jan Beulich 
> with one remark:
>
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -261,7 +261,11 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
>>  #ifdef CONFIG_X86
>>  const unsigned long *badpage = NULL;
>>  unsigned int i, array_size;
>> +
>> +BUILD_BUG_ON(8 * sizeof(((struct page_info *)0)->u.free.first_dirty) <
>> + MAX_ORDER + 1);
>>  #endif
>> +BUILD_BUG_ON(sizeof(((struct page_info *)0)->u) != sizeof(unsigned 
>> long));
> As I'm generally opposed to casts whenever one can get away
> without, I dislike these as well. In the case here, short of a local
> variable of suitable type, I'd suggest using frame_table instead
> of the open-coded cast. If you're fine with that, this can easily
> be done while committing.

Sure.


-boris

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCHES v8 1/8] mm: Place unscrubbed pages at the end of pagelist

2017-08-18 Thread Jan Beulich 
>>> On 16.08.17 at 20:33,  wrote:
> .. so that it's easy to find pages that need to be scrubbed (those pages are
> now marked with _PGC_need_scrub bit).
> 
> We keep track of the first unscrubbed page in a page buddy using first_dirty
> field. For now it can have two values, 0 (whole buddy needs scrubbing) or
> INVALID_DIRTY_IDX (the buddy does not need to be scrubbed). Subsequent patches
> will allow scrubbing to be interrupted, resulting in first_dirty taking any
> value.
> 
> Signed-off-by: Boris Ostrovsky 

Reviewed-by: Jan Beulich 
with one remark:

> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -261,7 +261,11 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
>  #ifdef CONFIG_X86
>  const unsigned long *badpage = NULL;
>  unsigned int i, array_size;
> +
> +BUILD_BUG_ON(8 * sizeof(((struct page_info *)0)->u.free.first_dirty) <
> + MAX_ORDER + 1);
>  #endif
> +BUILD_BUG_ON(sizeof(((struct page_info *)0)->u) != sizeof(unsigned 
> long));

As I'm generally opposed to casts whenever one can get away
without, I dislike these as well. In the case here, short of a local
variable of suitable type, I'd suggest using frame_table instead
of the open-coded cast. If you're fine with that, this can easily
be done while committing.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCHES v8 1/8] mm: Place unscrubbed pages at the end of pagelist

2017-08-17 Thread Julien Grall 

Hi Boris,

On 16/08/17 19:33, Boris Ostrovsky wrote:

.. so that it's easy to find pages that need to be scrubbed (those pages are
now marked with _PGC_need_scrub bit).

We keep track of the first unscrubbed page in a page buddy using first_dirty
field. For now it can have two values, 0 (whole buddy needs scrubbing) or
INVALID_DIRTY_IDX (the buddy does not need to be scrubbed). Subsequent patches
will allow scrubbing to be interrupted, resulting in first_dirty taking any
value.

Signed-off-by: Boris Ostrovsky 


For the ARM bits:

Acked-by: Julien Grall 

Cheers,


---
Changes in v8:
* Changed x86's definition of page_info.u.free from using bitfields to natural
  datatypes
* Swapped order of bitfields in page_info.u.free for ARM
* Added BUILD_BUG_ON to check page_info.u.free.first_dirty size on x86, moved
  previously defined BUILD_BUG_ON from init_heap_pages() to init_boot_pages()
  (to avoid introducing extra '#ifdef x86' and to keep both together)

 xen/common/page_alloc.c  | 159 ---
 xen/include/asm-arm/mm.h |  17 -
 xen/include/asm-x86/mm.h |  15 +
 3 files changed, 167 insertions(+), 24 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 444ecf3..a39fd81 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -261,7 +261,11 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
 #ifdef CONFIG_X86
 const unsigned long *badpage = NULL;
 unsigned int i, array_size;
+
+BUILD_BUG_ON(8 * sizeof(((struct page_info *)0)->u.free.first_dirty) <
+ MAX_ORDER + 1);
 #endif
+BUILD_BUG_ON(sizeof(((struct page_info *)0)->u) != sizeof(unsigned long));

 ps = round_pgup(ps);
 pe = round_pgdown(pe);
@@ -375,6 +379,8 @@ typedef struct page_list_head 
heap_by_zone_and_order_t[NR_ZONES][MAX_ORDER+1];
 static heap_by_zone_and_order_t *_heap[MAX_NUMNODES];
 #define heap(node, zone, order) ((*_heap[node])[zone][order])

+static unsigned long node_need_scrub[MAX_NUMNODES];
+
 static unsigned long *avail[MAX_NUMNODES];
 static long total_avail_pages;

@@ -670,13 +676,30 @@ static void check_low_mem_virq(void)
 }
 }

+/* Pages that need a scrub are added to tail, otherwise to head. */
+static void page_list_add_scrub(struct page_info *pg, unsigned int node,
+unsigned int zone, unsigned int order,
+unsigned int first_dirty)
+{
+PFN_ORDER(pg) = order;
+pg->u.free.first_dirty = first_dirty;
+
+if ( first_dirty != INVALID_DIRTY_IDX )
+{
+ASSERT(first_dirty < (1U << order));
+page_list_add_tail(pg, (node, zone, order));
+}
+else
+page_list_add(pg, (node, zone, order));
+}
+
 /* Allocate 2^@order contiguous pages. */
 static struct page_info *alloc_heap_pages(
 unsigned int zone_lo, unsigned int zone_hi,
 unsigned int order, unsigned int memflags,
 struct domain *d)
 {
-unsigned int i, j, zone = 0, nodemask_retry = 0;
+unsigned int i, j, zone = 0, nodemask_retry = 0, first_dirty;
 nodeid_t first_node, node = MEMF_get_node(memflags), req_node = node;
 unsigned long request = 1UL << order;
 struct page_info *pg;
@@ -790,12 +813,26 @@ static struct page_info *alloc_heap_pages(
 return NULL;

  found:
+
+first_dirty = pg->u.free.first_dirty;
+
 /* We may have to halve the chunk a number of times. */
 while ( j != order )
 {
-PFN_ORDER(pg) = --j;
-page_list_add_tail(pg, (node, zone, j));
-pg += 1 << j;
+j--;
+page_list_add_scrub(pg, node, zone, j,
+(1U << j) > first_dirty ?
+first_dirty : INVALID_DIRTY_IDX);
+pg += 1U << j;
+
+if ( first_dirty != INVALID_DIRTY_IDX )
+{
+/* Adjust first_dirty */
+if ( first_dirty >= 1U << j )
+first_dirty -= 1U << j;
+else
+first_dirty = 0; /* We've moved past original first_dirty */
+}
 }

 ASSERT(avail[node][zone] >= request);
@@ -842,12 +879,20 @@ static int reserve_offlined_page(struct page_info *head)
 unsigned int node = phys_to_nid(page_to_maddr(head));
 int zone = page_to_zone(head), i, head_order = PFN_ORDER(head), count = 0;
 struct page_info *cur_head;
-int cur_order;
+unsigned int cur_order, first_dirty;

 ASSERT(spin_is_locked(_lock));

 cur_head = head;

+/*
+ * We may break the buddy so let's mark the head as clean. Then, when
+ * merging chunks back into the heap, we will see whether the chunk has
+ * unscrubbed pages and set its first_dirty properly.
+ */
+first_dirty = head->u.free.first_dirty;
+head->u.free.first_dirty = INVALID_DIRTY_IDX;
+
 page_list_del(head, (node, zone, head_order));

 while ( cur_head < (head + (1 << head_order)) )
@@ -858,6 +903,8 @@ static int

[Xen-devel] [PATCHES v8 1/8] mm: Place unscrubbed pages at the end of pagelist

2017-08-16 Thread Boris Ostrovsky 
.. so that it's easy to find pages that need to be scrubbed (those pages are
now marked with _PGC_need_scrub bit).

We keep track of the first unscrubbed page in a page buddy using first_dirty
field. For now it can have two values, 0 (whole buddy needs scrubbing) or
INVALID_DIRTY_IDX (the buddy does not need to be scrubbed). Subsequent patches
will allow scrubbing to be interrupted, resulting in first_dirty taking any
value.

Signed-off-by: Boris Ostrovsky 
---
Changes in v8:
* Changed x86's definition of page_info.u.free from using bitfields to natural
  datatypes
* Swapped order of bitfields in page_info.u.free for ARM
* Added BUILD_BUG_ON to check page_info.u.free.first_dirty size on x86, moved
  previously defined BUILD_BUG_ON from init_heap_pages() to init_boot_pages()
  (to avoid introducing extra '#ifdef x86' and to keep both together)

 xen/common/page_alloc.c  | 159 ---
 xen/include/asm-arm/mm.h |  17 -
 xen/include/asm-x86/mm.h |  15 +
 3 files changed, 167 insertions(+), 24 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 444ecf3..a39fd81 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -261,7 +261,11 @@ void __init init_boot_pages(paddr_t ps, paddr_t pe)
 #ifdef CONFIG_X86
 const unsigned long *badpage = NULL;
 unsigned int i, array_size;
+
+BUILD_BUG_ON(8 * sizeof(((struct page_info *)0)->u.free.first_dirty) <
+ MAX_ORDER + 1);
 #endif
+BUILD_BUG_ON(sizeof(((struct page_info *)0)->u) != sizeof(unsigned long));
 
 ps = round_pgup(ps);
 pe = round_pgdown(pe);
@@ -375,6 +379,8 @@ typedef struct page_list_head 
heap_by_zone_and_order_t[NR_ZONES][MAX_ORDER+1];
 static heap_by_zone_and_order_t *_heap[MAX_NUMNODES];
 #define heap(node, zone, order) ((*_heap[node])[zone][order])
 
+static unsigned long node_need_scrub[MAX_NUMNODES];
+
 static unsigned long *avail[MAX_NUMNODES];
 static long total_avail_pages;
 
@@ -670,13 +676,30 @@ static void check_low_mem_virq(void)
 }
 }
 
+/* Pages that need a scrub are added to tail, otherwise to head. */
+static void page_list_add_scrub(struct page_info *pg, unsigned int node,
+unsigned int zone, unsigned int order,
+unsigned int first_dirty)
+{
+PFN_ORDER(pg) = order;
+pg->u.free.first_dirty = first_dirty;
+
+if ( first_dirty != INVALID_DIRTY_IDX )
+{
+ASSERT(first_dirty < (1U << order));
+page_list_add_tail(pg, (node, zone, order));
+}
+else
+page_list_add(pg, (node, zone, order));
+}
+
 /* Allocate 2^@order contiguous pages. */
 static struct page_info *alloc_heap_pages(
 unsigned int zone_lo, unsigned int zone_hi,
 unsigned int order, unsigned int memflags,
 struct domain *d)
 {
-unsigned int i, j, zone = 0, nodemask_retry = 0;
+unsigned int i, j, zone = 0, nodemask_retry = 0, first_dirty;
 nodeid_t first_node, node = MEMF_get_node(memflags), req_node = node;
 unsigned long request = 1UL << order;
 struct page_info *pg;
@@ -790,12 +813,26 @@ static struct page_info *alloc_heap_pages(
 return NULL;
 
  found: 
+
+first_dirty = pg->u.free.first_dirty;
+
 /* We may have to halve the chunk a number of times. */
 while ( j != order )
 {
-PFN_ORDER(pg) = --j;
-page_list_add_tail(pg, (node, zone, j));
-pg += 1 << j;
+j--;
+page_list_add_scrub(pg, node, zone, j,
+(1U << j) > first_dirty ?
+first_dirty : INVALID_DIRTY_IDX);
+pg += 1U << j;
+
+if ( first_dirty != INVALID_DIRTY_IDX )
+{
+/* Adjust first_dirty */
+if ( first_dirty >= 1U << j )
+first_dirty -= 1U << j;
+else
+first_dirty = 0; /* We've moved past original first_dirty */
+}
 }
 
 ASSERT(avail[node][zone] >= request);
@@ -842,12 +879,20 @@ static int reserve_offlined_page(struct page_info *head)
 unsigned int node = phys_to_nid(page_to_maddr(head));
 int zone = page_to_zone(head), i, head_order = PFN_ORDER(head), count = 0;
 struct page_info *cur_head;
-int cur_order;
+unsigned int cur_order, first_dirty;
 
 ASSERT(spin_is_locked(_lock));
 
 cur_head = head;
 
+/*
+ * We may break the buddy so let's mark the head as clean. Then, when
+ * merging chunks back into the heap, we will see whether the chunk has
+ * unscrubbed pages and set its first_dirty properly.
+ */
+first_dirty = head->u.free.first_dirty;
+head->u.free.first_dirty = INVALID_DIRTY_IDX;
+
 page_list_del(head, (node, zone, head_order));
 
 while ( cur_head < (head + (1 << head_order)) )
@@ -858,6 +903,8 @@ static int reserve_offlined_page(struct page_info *head)
 if ( page_state_is(cur_head, offlined) )
 {