[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v5 6/8] mm: Keep heap accessible to others while scrubbing
On 06/27/2017 03:28 PM, Jan Beulich wrote: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> 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@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |