| Message ID | 1504026557-11365-5-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 29, 2017 at 01:09:16PM -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()). > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > --- > Changes in v2: > * Removed '#ifdef CONFIG_SCRUB_DEBUG' > > xen/common/page_alloc.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c > index 3db77c5..6c08983 100644 > --- a/xen/common/page_alloc.c > +++ b/xen/common/page_alloc.c > @@ -2247,16 +2247,12 @@ void free_domheap_pages(struct page_info *pg, unsigned int order) > > spin_unlock_recursive(&d->page_alloc_lock); > > -#ifndef CONFIG_SCRUB_DEBUG > /* > * Normally we expect a domain to clear pages before freeing them, > * if it cares about the secrecy of their contents. However, after > * a domain has died we assume responsibility for erasure. > */ > - scrub = !!d->is_dying; > -#else > - scrub = true; > -#endif > + scrub = !!d->is_dying | scrub_debug; Use logical or here please. And the !! for is_dying is not necessary. Also please expand the comment to say why scrub_debug is also checked.
On 08/30/2017 04:42 AM, Wei Liu wrote: > On Tue, Aug 29, 2017 at 01:09:16PM -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()). >> >> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> --- >> Changes in v2: >> * Removed '#ifdef CONFIG_SCRUB_DEBUG' >> >> xen/common/page_alloc.c | 6 +----- >> 1 file changed, 1 insertion(+), 5 deletions(-) >> >> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c >> index 3db77c5..6c08983 100644 >> --- a/xen/common/page_alloc.c >> +++ b/xen/common/page_alloc.c >> @@ -2247,16 +2247,12 @@ void free_domheap_pages(struct page_info *pg, unsigned int order) >> >> spin_unlock_recursive(&d->page_alloc_lock); >> >> -#ifndef CONFIG_SCRUB_DEBUG >> /* >> * Normally we expect a domain to clear pages before freeing them, >> * if it cares about the secrecy of their contents. However, after >> * a domain has died we assume responsibility for erasure. >> */ >> - scrub = !!d->is_dying; >> -#else >> - scrub = true; >> -#endif >> + scrub = !!d->is_dying | scrub_debug; > Use logical or here please. And the !! for is_dying is not necessary. Yes, I'll change this. > > Also please expand the comment to say why scrub_debug is also checked. TBH, I think variable name (scrub_debug) should be sufficient. I could say something like "when scrub debugging is on we always scrub" but I am not sure how useful that would be. (Previous name, boot_scrub_done, would have indeed be helped by a comment). -boris
On Wed, Aug 30, 2017 at 10:19:49AM -0400, Boris Ostrovsky wrote: > On 08/30/2017 04:42 AM, Wei Liu wrote: > > On Tue, Aug 29, 2017 at 01:09:16PM -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()). > >> > >> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > >> --- > >> Changes in v2: > >> * Removed '#ifdef CONFIG_SCRUB_DEBUG' > >> > >> xen/common/page_alloc.c | 6 +----- > >> 1 file changed, 1 insertion(+), 5 deletions(-) > >> > >> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c > >> index 3db77c5..6c08983 100644 > >> --- a/xen/common/page_alloc.c > >> +++ b/xen/common/page_alloc.c > >> @@ -2247,16 +2247,12 @@ void free_domheap_pages(struct page_info *pg, unsigned int order) > >> > >> spin_unlock_recursive(&d->page_alloc_lock); > >> > >> -#ifndef CONFIG_SCRUB_DEBUG > >> /* > >> * Normally we expect a domain to clear pages before freeing them, > >> * if it cares about the secrecy of their contents. However, after > >> * a domain has died we assume responsibility for erasure. > >> */ > >> - scrub = !!d->is_dying; > >> -#else > >> - scrub = true; > >> -#endif > >> + scrub = !!d->is_dying | scrub_debug; > > Use logical or here please. And the !! for is_dying is not necessary. > > Yes, I'll change this. > > > > > Also please expand the comment to say why scrub_debug is also checked. > > TBH, I think variable name (scrub_debug) should be sufficient. I could > say something like "when scrub debugging is on we always scrub" but I am > not sure how useful that would be. (Previous name, boot_scrub_done, > would have indeed be helped by a comment). > Hmm... OK, I won't insist on this.
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 3db77c5..6c08983 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -2247,16 +2247,12 @@ void free_domheap_pages(struct page_info *pg, unsigned int order) spin_unlock_recursive(&d->page_alloc_lock); -#ifndef CONFIG_SCRUB_DEBUG /* * Normally we expect a domain to clear pages before freeing them, * if it cares about the secrecy of their contents. However, after * a domain has died we assume responsibility for erasure. */ - scrub = !!d->is_dying; -#else - scrub = true; -#endif + scrub = !!d->is_dying | scrub_debug; } else {
There is no need to scrub pages freed during dom0 construction since heap will be scrubbed once dom0 is ready (by scrub_heap_pages()). Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- Changes in v2: * Removed '#ifdef CONFIG_SCRUB_DEBUG' xen/common/page_alloc.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)