On 06/27/2017 03:28 PM, Jan Beulich wrote:
Boris Ostrovsky <boris.ostrov...@oracle.com> 06/22/17 8:56 PM >>>
Changes in v5:
* Fixed off-by-one error in setting first_dirty
* Changed struct page_info.u.free to a union to permit use of ACCESS_ONCE in
check_and_stop_scrub()
I don't see the need for this:
+static void check_and_stop_scrub(struct page_info *head)
+{
+ if ( head->u.free.scrub_state == BUDDY_SCRUBBING )
+ {
+ struct page_info pg;
+
+ head->u.free.scrub_state = BUDDY_SCRUB_ABORT;
+ spin_lock_kick();
+ for ( ; ; )
+ {
+ /* Can't ACCESS_ONCE() a bitfield. */
+ pg.u.free.val = ACCESS_ONCE(head->u.free.val);
Something like ACCESS_ONCE(head->u.free).val ought to work (or read_atomic(),
due to the questionable scalar type check in ACCESS_ONCE()).
Hmm... I couldn't get this to work with either suggestion.
page_alloc.c:751:13: error: conversion to non-scalar type requested
pg.u.free = read_atomic(&head->u.free);
page_alloc.c:753:6: error: conversion to non-scalar type requested
if ( ACCESS_ONCE(head->u.free).scrub_state != BUDDY_SCRUB_ABORT )
@@ -1106,25 +1155,53 @@ bool scrub_free_pages(void)
do {
while ( !page_list_empty(&heap(node, zone, order)) )
{
- unsigned int i;
+ unsigned int i, dirty_cnt;
+ struct scrub_wait_state st;
/* Unscrubbed pages are always at the end of the list. */
pg = page_list_last(&heap(node, zone, order));
if ( pg->u.free.first_dirty == INVALID_DIRTY_IDX )
break;
+ ASSERT(!pg->u.free.scrub_state);
Please use BUDDY_NOT_SCRUBBING here.
@@ -1138,6 +1215,17 @@ bool scrub_free_pages(void)
}
}
+ st.pg = pg;
+ st.first_dirty = (i >= (1UL << order) - 1) ?
+ INVALID_DIRTY_IDX : i + 1;
Would you mind explaining to me (again?) why you can't set pg's first_dirty
directly here? In case I'm not mistaken and this has been asked before, maybe
this is a hint that a comment might be warranted.
In get_free_buddy() (formerly part of alloc_heap_pages()) I have
/* Find smallest order which can satisfy the request. */
for ( j = order; j <= MAX_ORDER; j++ )
{
if ( (pg = page_list_remove_head(&heap(node, zone, j))) )
{
if ( pg->u.free.first_dirty == INVALID_DIRTY_IDX )
return pg;
/*
* We grab single pages (order=0) even if they are
* unscrubbed. Given that scrubbing one page is
fairly quick
* it is not worth breaking higher orders.
*/
if ( (order == 0) || use_unscrubbed )
{
check_and_stop_scrub(pg);
return pg;
}
If first_dirty gets assigned INVALID_DIRTY_IDX then get_free_buddy()
will return pg right away without telling the scrubber that the buddy
has been taken for use. The scrubber will then put the buddy back on the
heap.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
