| Message ID | 1503952829-11065-2-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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
>>> 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
>>> 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
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(); }
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(-)