* [PATCH 0/4] Scrubbing updates
@ 2017-08-28 20:40 Boris Ostrovsky
2017-08-28 20:40 ` [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled Boris Ostrovsky
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Boris Ostrovsky @ 2017-08-28 20:40 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, tim, jbeulich, Boris Ostrovsky
First patch fixes a long-standing bug where a low memory monitor is
not initializes if boottime scrubbing is turned off.
The other threee patches are performace optimizations.
Boris Ostrovsky (4):
mm: Initialize lowmem virq when boot-time scrubbing is disabled
mm: Don't poison a page if boot-time scrubbing is off
mm: Don't request scrubbing until dom0 is running
mm: Don't hold heap lock in alloc_heap_pages() longer than necessary
xen/common/page_alloc.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled
2017-08-28 20:40 [PATCH 0/4] Scrubbing updates Boris Ostrovsky
@ 2017-08-28 20:40 ` Boris Ostrovsky
2017-08-29 11:51 ` Wei Liu
2017-08-28 20:40 ` [PATCH 2/4] mm: Don't poison a page if boot-time scrubbing is off Boris Ostrovsky
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Boris Ostrovsky @ 2017-08-28 20:40 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, tim, jbeulich, Boris Ostrovsky
scrub_heap_pages() does early return if boot-time scrubbing is
disabled, neglecting to initialize lowmem VIRQ.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
xen/common/page_alloc.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 9fa62d2..7d56e92 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1849,6 +1849,14 @@ void __init scrub_heap_pages(void)
int last_distance, best_node;
int cpus;
+ /*
+ * Set bounds for the low mem virq algorithm.
+ * Not the most logical place for this but it needs to be done
+ * after dom0 has been constructed and this seems to be a
+ * convenient location.
+ */
+ setup_low_mem_virq();
+
if ( !opt_bootscrub )
return;
@@ -1970,10 +1978,6 @@ void __init scrub_heap_pages(void)
#ifdef CONFIG_SCRUB_DEBUG
boot_scrub_done = true;
#endif
-
- /* Now that the heap is initialized, run checks and set bounds
- * for the low mem virq algorithm. */
- setup_low_mem_virq();
}
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/4] mm: Don't poison a page if boot-time scrubbing is off
2017-08-28 20:40 [PATCH 0/4] Scrubbing updates Boris Ostrovsky
2017-08-28 20:40 ` [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled Boris Ostrovsky
@ 2017-08-28 20:40 ` Boris Ostrovsky
2017-08-29 11:52 ` Wei Liu
2017-08-28 20:40 ` [PATCH 3/4] mm: Don't request scrubbing until dom0 is running Boris Ostrovsky
2017-08-28 20:40 ` [PATCH 4/4] mm: Don't hold heap lock in alloc_heap_pages() longer than necessary Boris Ostrovsky
3 siblings, 1 reply; 20+ messages in thread
From: Boris Ostrovsky @ 2017-08-28 20:40 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, tim, jbeulich, Boris Ostrovsky
If boot-time scrubbing is turned off we don't check pages in
check_one_page(). Thus there is no reason to ever poison them.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
xen/common/page_alloc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 7d56e92..34a7992 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -712,6 +712,9 @@ static void poison_one_page(struct page_info *pg)
mfn_t mfn = _mfn(page_to_mfn(pg));
uint64_t *ptr;
+ if ( !boot_scrub_done )
+ return;
+
ptr = map_domain_page(mfn);
*ptr = ~SCRUB_PATTERN;
unmap_domain_page(ptr);
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-28 20:40 [PATCH 0/4] Scrubbing updates Boris Ostrovsky
2017-08-28 20:40 ` [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled Boris Ostrovsky
2017-08-28 20:40 ` [PATCH 2/4] mm: Don't poison a page if boot-time scrubbing is off Boris Ostrovsky
@ 2017-08-28 20:40 ` Boris Ostrovsky
2017-08-29 11:56 ` Wei Liu
2017-08-29 13:26 ` Jan Beulich
2017-08-28 20:40 ` [PATCH 4/4] mm: Don't hold heap lock in alloc_heap_pages() longer than necessary Boris Ostrovsky
3 siblings, 2 replies; 20+ messages in thread
From: Boris Ostrovsky @ 2017-08-28 20:40 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, tim, jbeulich, Boris Ostrovsky
There is no need to scrub pages freed during dom0 construction
since heap will be scrubbed once dom0 is ready (by scrub_heap_pages()).
Since boot_scrub_done will not be set if boot-time scrubbing is off we
also check for domain state.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
xen/common/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 34a7992..b93dae9 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2259,7 +2259,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
*/
scrub = !!d->is_dying;
#else
- scrub = true;
+ scrub = boot_scrub_done || !!d->is_dying;
#endif
}
else
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/4] mm: Don't hold heap lock in alloc_heap_pages() longer than necessary
2017-08-28 20:40 [PATCH 0/4] Scrubbing updates Boris Ostrovsky
` (2 preceding siblings ...)
2017-08-28 20:40 ` [PATCH 3/4] mm: Don't request scrubbing until dom0 is running Boris Ostrovsky
@ 2017-08-28 20:40 ` Boris Ostrovsky
2017-08-29 12:16 ` Wei Liu
2017-08-29 13:33 ` Jan Beulich
3 siblings, 2 replies; 20+ messages in thread
From: Boris Ostrovsky @ 2017-08-28 20:40 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, tim, jbeulich, Boris Ostrovsky
Once pages are removed from the heap we don't need to hold the heap
lock. It is especially useful to drop it for an unscrubbed buddy since
we will be scrubbing it.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
xen/common/page_alloc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index b93dae9..1ec788e 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -858,6 +858,7 @@ static struct page_info *alloc_heap_pages(
struct page_info *pg;
bool need_tlbflush = false;
uint32_t tlbflush_timestamp = 0;
+ unsigned int dirty_cnt = 0;
/* Make sure there are enough bits in memflags for nodeID. */
BUILD_BUG_ON((_MEMF_bits - _MEMF_node) < (8 * sizeof(nodeid_t)));
@@ -943,6 +944,8 @@ static struct page_info *alloc_heap_pages(
check_low_mem_virq();
+ spin_unlock(&heap_lock);
+
if ( d != NULL )
d->last_alloc_node = node;
@@ -955,7 +958,7 @@ static struct page_info *alloc_heap_pages(
{
if ( !(memflags & MEMF_no_scrub) )
scrub_one_page(&pg[i]);
- node_need_scrub[node]--;
+ dirty_cnt++;
}
pg[i].count_info = PGC_state_inuse;
@@ -977,6 +980,8 @@ static struct page_info *alloc_heap_pages(
check_one_page(&pg[i]);
}
+ spin_lock(&heap_lock);
+ node_need_scrub[node] -= dirty_cnt;
spin_unlock(&heap_lock);
if ( need_tlbflush )
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled
2017-08-28 20:40 ` [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled Boris Ostrovsky
@ 2017-08-29 11:51 ` Wei Liu
2017-08-29 13:12 ` Boris Ostrovsky
2017-08-29 13:19 ` Jan Beulich
0 siblings, 2 replies; 20+ messages in thread
From: Wei Liu @ 2017-08-29 11:51 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, xen-devel, jbeulich
On Mon, Aug 28, 2017 at 04:40:26PM -0400, Boris Ostrovsky wrote:
> scrub_heap_pages() does early return if boot-time scrubbing is
> disabled, neglecting to initialize lowmem VIRQ.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Seems more appropriate to lift the call to setup_low_mem_virq to the
caller of scrub_heap_pages.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/4] mm: Don't poison a page if boot-time scrubbing is off
2017-08-28 20:40 ` [PATCH 2/4] mm: Don't poison a page if boot-time scrubbing is off Boris Ostrovsky
@ 2017-08-29 11:52 ` Wei Liu
0 siblings, 0 replies; 20+ messages in thread
From: Wei Liu @ 2017-08-29 11:52 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, xen-devel, jbeulich
On Mon, Aug 28, 2017 at 04:40:27PM -0400, Boris Ostrovsky wrote:
> If boot-time scrubbing is turned off we don't check pages in
> check_one_page(). Thus there is no reason to ever poison them.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-28 20:40 ` [PATCH 3/4] mm: Don't request scrubbing until dom0 is running Boris Ostrovsky
@ 2017-08-29 11:56 ` Wei Liu
2017-08-29 13:22 ` Jan Beulich
2017-08-29 13:26 ` Jan Beulich
1 sibling, 1 reply; 20+ messages in thread
From: Wei Liu @ 2017-08-29 11:56 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, xen-devel, jbeulich
On Mon, Aug 28, 2017 at 04:40:28PM -0400, Boris Ostrovsky wrote:
> There is no need to scrub pages freed during dom0 construction
> since heap will be scrubbed once dom0 is ready (by scrub_heap_pages()).
>
> Since boot_scrub_done will not be set if boot-time scrubbing is off we
> also check for domain state.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> xen/common/page_alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 34a7992..b93dae9 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -2259,7 +2259,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
> */
> scrub = !!d->is_dying;
> #else
> - scrub = true;
> + scrub = boot_scrub_done || !!d->is_dying;
It seems that the debug and non-debug case should use the same
predicate.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] mm: Don't hold heap lock in alloc_heap_pages() longer than necessary
2017-08-28 20:40 ` [PATCH 4/4] mm: Don't hold heap lock in alloc_heap_pages() longer than necessary Boris Ostrovsky
@ 2017-08-29 12:16 ` Wei Liu
2017-08-29 13:33 ` Jan Beulich
1 sibling, 0 replies; 20+ messages in thread
From: Wei Liu @ 2017-08-29 12:16 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, xen-devel, jbeulich
On Mon, Aug 28, 2017 at 04:40:29PM -0400, Boris Ostrovsky wrote:
> Once pages are removed from the heap we don't need to hold the heap
> lock. It is especially useful to drop it for an unscrubbed buddy since
> we will be scrubbing it.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled
2017-08-29 11:51 ` Wei Liu
@ 2017-08-29 13:12 ` Boris Ostrovsky
2017-08-29 13:20 ` Jan Beulich
2017-08-29 13:19 ` Jan Beulich
1 sibling, 1 reply; 20+ messages in thread
From: Boris Ostrovsky @ 2017-08-29 13:12 UTC (permalink / raw)
To: Wei Liu
Cc: tim, sstabellini, George.Dunlap, andrew.cooper3, ian.jackson,
xen-devel, jbeulich
On 08/29/2017 07:51 AM, Wei Liu wrote:
> On Mon, Aug 28, 2017 at 04:40:26PM -0400, Boris Ostrovsky wrote:
>> scrub_heap_pages() does early return if boot-time scrubbing is
>> disabled, neglecting to initialize lowmem VIRQ.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Seems more appropriate to lift the call to setup_low_mem_virq to the
> caller of scrub_heap_pages.
setup_low_mem_virq() is only useful in page_alloc.c so I'd rather keep
it static there.
What I could do is create something like
void heap_init_late(void)
{
setup_low_mem_virq();
if ( opt_bootscrub )
scrub_heap_pages();
}
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled
2017-08-29 11:51 ` Wei Liu
2017-08-29 13:12 ` Boris Ostrovsky
@ 2017-08-29 13:19 ` Jan Beulich
1 sibling, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-08-29 13:19 UTC (permalink / raw)
To: wei.liu2, Boris Ostrovsky
Cc: tim, sstabellini, George.Dunlap, andrew.cooper3, ian.jackson, xen-devel
>>> On 29.08.17 at 13:51, <wei.liu2@citrix.com> wrote:
> On Mon, Aug 28, 2017 at 04:40:26PM -0400, Boris Ostrovsky wrote:
>> scrub_heap_pages() does early return if boot-time scrubbing is
>> disabled, neglecting to initialize lowmem VIRQ.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>
> Seems more appropriate to lift the call to setup_low_mem_virq to the
> caller of scrub_heap_pages.
That's not a good idea imo, as that would mean replicating the
code in ARM and x86. I'd rather see the early return to become
an ordinary if (skipping the rest of the function, preferably
without goto). Or, mainly to limit code churn, introduce a new
wrapper function (of perhaps a different name, as
setup_low_mem_virq() has nothing to do with scrubbing
anyway) and move the setup_low_mem_virq() call
there, making scrub_heap_pages() static.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled
2017-08-29 13:12 ` Boris Ostrovsky
@ 2017-08-29 13:20 ` Jan Beulich
0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-08-29 13:20 UTC (permalink / raw)
To: Wei Liu, Boris Ostrovsky
Cc: tim, sstabellini, George.Dunlap, andrew.cooper3, ian.jackson, xen-devel
>>> On 29.08.17 at 15:12, <boris.ostrovsky@oracle.com> wrote:
> On 08/29/2017 07:51 AM, Wei Liu wrote:
>> On Mon, Aug 28, 2017 at 04:40:26PM -0400, Boris Ostrovsky wrote:
>>> scrub_heap_pages() does early return if boot-time scrubbing is
>>> disabled, neglecting to initialize lowmem VIRQ.
>>>
>>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Seems more appropriate to lift the call to setup_low_mem_virq to the
>> caller of scrub_heap_pages.
>
> setup_low_mem_virq() is only useful in page_alloc.c so I'd rather keep
> it static there.
>
> What I could do is create something like
>
> void heap_init_late(void)
> {
> setup_low_mem_virq();
>
> if ( opt_bootscrub )
> scrub_heap_pages();
> }
Ah, yes, that's what I've outlined in the other reply just sent.
Don't forget the __init, though.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-29 11:56 ` Wei Liu
@ 2017-08-29 13:22 ` Jan Beulich
2017-08-29 13:27 ` Boris Ostrovsky
0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2017-08-29 13:22 UTC (permalink / raw)
To: wei.liu2
Cc: tim, sstabellini, George.Dunlap, andrew.cooper3, ian.jackson,
xen-devel, Boris Ostrovsky
>>> On 29.08.17 at 13:56, <wei.liu2@citrix.com> wrote:
> On Mon, Aug 28, 2017 at 04:40:28PM -0400, Boris Ostrovsky wrote:
>> There is no need to scrub pages freed during dom0 construction
>> since heap will be scrubbed once dom0 is ready (by scrub_heap_pages()).
>>
>> Since boot_scrub_done will not be set if boot-time scrubbing is off we
>> also check for domain state.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> ---
>> xen/common/page_alloc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
>> index 34a7992..b93dae9 100644
>> --- a/xen/common/page_alloc.c
>> +++ b/xen/common/page_alloc.c
>> @@ -2259,7 +2259,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
>> */
>> scrub = !!d->is_dying;
>> #else
>> - scrub = true;
>> + scrub = boot_scrub_done || !!d->is_dying;
>
> It seems that the debug and non-debug case should use the same
> predicate.
No - boot_scrub_done doesn't even exist in the other case.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-28 20:40 ` [PATCH 3/4] mm: Don't request scrubbing until dom0 is running Boris Ostrovsky
2017-08-29 11:56 ` Wei Liu
@ 2017-08-29 13:26 ` Jan Beulich
2017-08-29 13:31 ` Wei Liu
1 sibling, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2017-08-29 13:26 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, xen-devel
>>> On 28.08.17 at 22:40, <boris.ostrovsky@oracle.com> wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -2259,7 +2259,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
> */
> scrub = !!d->is_dying;
> #else
> - scrub = true;
> + scrub = boot_scrub_done || !!d->is_dying;
> #endif
Actually Wei has a point here - already when reviewing the original
series I had wondered whether a "#define boot_scrub_done false"
in the non-debug case wouldn't help eliminate some #ifdef-ary.
Here it would mean fulfilling Wei's request indirectly, by simply
removing the preprocessor directives altogether.
In any event the !! is pointless on an operand to || or && .
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-29 13:22 ` Jan Beulich
@ 2017-08-29 13:27 ` Boris Ostrovsky
2017-08-29 13:34 ` Jan Beulich
0 siblings, 1 reply; 20+ messages in thread
From: Boris Ostrovsky @ 2017-08-29 13:27 UTC (permalink / raw)
To: Jan Beulich, wei.liu2
Cc: tim, sstabellini, George.Dunlap, andrew.cooper3, ian.jackson, xen-devel
On 08/29/2017 09:22 AM, Jan Beulich wrote:
>>>> On 29.08.17 at 13:56, <wei.liu2@citrix.com> wrote:
>> On Mon, Aug 28, 2017 at 04:40:28PM -0400, Boris Ostrovsky wrote:
>>> There is no need to scrub pages freed during dom0 construction
>>> since heap will be scrubbed once dom0 is ready (by scrub_heap_pages()).
>>>
>>> Since boot_scrub_done will not be set if boot-time scrubbing is off we
>>> also check for domain state.
>>>
>>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> ---
>>> xen/common/page_alloc.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
>>> index 34a7992..b93dae9 100644
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -2259,7 +2259,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
>>> */
>>> scrub = !!d->is_dying;
>>> #else
>>> - scrub = true;
>>> + scrub = boot_scrub_done || !!d->is_dying;
>> It seems that the debug and non-debug case should use the same
>> predicate.
> No - boot_scrub_done doesn't even exist in the other case.
I read Wei's message as suggesting
scrub = !!d->is_dying;
#ifdef CONFIG_SCRUB_DEBUG
scrub |= boot_scrub_done;
#endif
which I could do.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-29 13:26 ` Jan Beulich
@ 2017-08-29 13:31 ` Wei Liu
0 siblings, 0 replies; 20+ messages in thread
From: Wei Liu @ 2017-08-29 13:31 UTC (permalink / raw)
To: Jan Beulich
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, xen-devel, Boris Ostrovsky
On Tue, Aug 29, 2017 at 07:26:28AM -0600, Jan Beulich wrote:
> >>> On 28.08.17 at 22:40, <boris.ostrovsky@oracle.com> wrote:
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -2259,7 +2259,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
> > */
> > scrub = !!d->is_dying;
> > #else
> > - scrub = true;
> > + scrub = boot_scrub_done || !!d->is_dying;
> > #endif
>
> Actually Wei has a point here - already when reviewing the original
> series I had wondered whether a "#define boot_scrub_done false"
> in the non-debug case wouldn't help eliminate some #ifdef-ary.
> Here it would mean fulfilling Wei's request indirectly, by simply
> removing the preprocessor directives altogether.
This SGTM
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] mm: Don't hold heap lock in alloc_heap_pages() longer than necessary
2017-08-28 20:40 ` [PATCH 4/4] mm: Don't hold heap lock in alloc_heap_pages() longer than necessary Boris Ostrovsky
2017-08-29 12:16 ` Wei Liu
@ 2017-08-29 13:33 ` Jan Beulich
1 sibling, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-08-29 13:33 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, xen-devel
>>> On 28.08.17 at 22:40, <boris.ostrovsky@oracle.com> wrote:
> @@ -943,6 +944,8 @@ static struct page_info *alloc_heap_pages(
>
> check_low_mem_virq();
>
> + spin_unlock(&heap_lock);
> +
> if ( d != NULL )
> d->last_alloc_node = node;
I'm not sure about the placement - as long as there's only a single
heap lock it certainly also protects the last_alloc_node updates
visible in context here. The consumer of this field also holds the
heap lock afaict, so at least for the moment it would feel more
safe if you moved the unlock past that update. With that feel
free to add
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-29 13:27 ` Boris Ostrovsky
@ 2017-08-29 13:34 ` Jan Beulich
2017-08-29 14:20 ` Boris Ostrovsky
0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2017-08-29 13:34 UTC (permalink / raw)
To: wei.liu2, Boris Ostrovsky
Cc: tim, sstabellini, George.Dunlap, andrew.cooper3, ian.jackson, xen-devel
>>> On 29.08.17 at 15:27, <boris.ostrovsky@oracle.com> wrote:
> On 08/29/2017 09:22 AM, Jan Beulich wrote:
>>>>> On 29.08.17 at 13:56, <wei.liu2@citrix.com> wrote:
>>> On Mon, Aug 28, 2017 at 04:40:28PM -0400, Boris Ostrovsky wrote:
>>>> There is no need to scrub pages freed during dom0 construction
>>>> since heap will be scrubbed once dom0 is ready (by scrub_heap_pages()).
>>>>
>>>> Since boot_scrub_done will not be set if boot-time scrubbing is off we
>>>> also check for domain state.
>>>>
>>>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>>> ---
>>>> xen/common/page_alloc.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
>>>> index 34a7992..b93dae9 100644
>>>> --- a/xen/common/page_alloc.c
>>>> +++ b/xen/common/page_alloc.c
>>>> @@ -2259,7 +2259,7 @@ void free_domheap_pages(struct page_info *pg, unsigned
> int order)
>>>> */
>>>> scrub = !!d->is_dying;
>>>> #else
>>>> - scrub = true;
>>>> + scrub = boot_scrub_done || !!d->is_dying;
>>> It seems that the debug and non-debug case should use the same
>>> predicate.
>> No - boot_scrub_done doesn't even exist in the other case.
>
>
> I read Wei's message as suggesting
>
> scrub = !!d->is_dying;
> #ifdef CONFIG_SCRUB_DEBUG
> scrub |= boot_scrub_done;
> #endif
>
>
> which I could do.
Yes, but please consider the "#define boot_scrub_done 0" approach
too.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-29 13:34 ` Jan Beulich
@ 2017-08-29 14:20 ` Boris Ostrovsky
2017-08-29 14:25 ` Jan Beulich
0 siblings, 1 reply; 20+ messages in thread
From: Boris Ostrovsky @ 2017-08-29 14:20 UTC (permalink / raw)
To: Jan Beulich, wei.liu2
Cc: tim, sstabellini, George.Dunlap, andrew.cooper3, ian.jackson, xen-devel
> Yes, but please consider the "#define boot_scrub_done 0" approach
> too.
Yes, I'll do that but I will rename boot_scrub_done to scrub_debug since
having boot_scrub_done=0 in non-debug case will not convey what it
actually is supposed to mean.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/4] mm: Don't request scrubbing until dom0 is running
2017-08-29 14:20 ` Boris Ostrovsky
@ 2017-08-29 14:25 ` Jan Beulich
0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-08-29 14:25 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: tim, sstabellini, wei.liu2, George.Dunlap, andrew.cooper3,
ian.jackson, xen-devel
>>> On 29.08.17 at 16:20, <boris.ostrovsky@oracle.com> wrote:
>> Yes, but please consider the "#define boot_scrub_done 0" approach
>> too.
>
> Yes, I'll do that but I will rename boot_scrub_done to scrub_debug since
> having boot_scrub_done=0 in non-debug case will not convey what it
> actually is supposed to mean.
Yeah, I too did realize that, but didn't have a good idea for a
replacement name. Yours sounds good.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-08-29 14:25 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-28 20:40 [PATCH 0/4] Scrubbing updates Boris Ostrovsky
2017-08-28 20:40 ` [PATCH 1/4] mm: Initialize lowmem virq when boot-time scrubbing is disabled Boris Ostrovsky
2017-08-29 11:51 ` Wei Liu
2017-08-29 13:12 ` Boris Ostrovsky
2017-08-29 13:20 ` Jan Beulich
2017-08-29 13:19 ` Jan Beulich
2017-08-28 20:40 ` [PATCH 2/4] mm: Don't poison a page if boot-time scrubbing is off Boris Ostrovsky
2017-08-29 11:52 ` Wei Liu
2017-08-28 20:40 ` [PATCH 3/4] mm: Don't request scrubbing until dom0 is running Boris Ostrovsky
2017-08-29 11:56 ` Wei Liu
2017-08-29 13:22 ` Jan Beulich
2017-08-29 13:27 ` Boris Ostrovsky
2017-08-29 13:34 ` Jan Beulich
2017-08-29 14:20 ` Boris Ostrovsky
2017-08-29 14:25 ` Jan Beulich
2017-08-29 13:26 ` Jan Beulich
2017-08-29 13:31 ` Wei Liu
2017-08-28 20:40 ` [PATCH 4/4] mm: Don't hold heap lock in alloc_heap_pages() longer than necessary Boris Ostrovsky
2017-08-29 12:16 ` Wei Liu
2017-08-29 13:33 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.