Message ID | 20210526161129.28572-1-julien@xen.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] xen/page_alloc: Remove dead code in alloc_domheap_pages() | expand |
On 26.05.2021 18:11, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > Since commit 1aac966e24e9 "xen: support RAM at addresses 0 and 4096", > bits_to_zone() will never return 0 and it is expected that we have > minimum 2 zones. > > Therefore the check in alloc_domheap_pages() is unnecessary and can > be removed. However, for sanity, it is replaced with an ASSERT(). > > Also take the opportunity to switch from min_t() to min() as > bits_to_zone() cannot return a negative value. The macro is tweaked > to make it clearer. > > This bug was discovered and resolved using Coverity Static Analysis > Security Testing (SAST) by Synopsys, Inc. > > Signed-off-by: Julien Grall <jgrall@amazon.com> > > --- > Changes in v2: > - Remove BUILD_BUG_ON() > - Switch from min_t() to min() Since this fulfills the dependencies put in place at the time, the Reviewed-by: Jan Beulich <jbeulich@suse.com> continues to apply here. Thanks for making the adjustments. Jan
Hi Jan, On 27/05/2021 08:15, Jan Beulich wrote: > On 26.05.2021 18:11, Julien Grall wrote: >> From: Julien Grall <jgrall@amazon.com> >> >> Since commit 1aac966e24e9 "xen: support RAM at addresses 0 and 4096", >> bits_to_zone() will never return 0 and it is expected that we have >> minimum 2 zones. >> >> Therefore the check in alloc_domheap_pages() is unnecessary and can >> be removed. However, for sanity, it is replaced with an ASSERT(). >> >> Also take the opportunity to switch from min_t() to min() as >> bits_to_zone() cannot return a negative value. The macro is tweaked >> to make it clearer. >> >> This bug was discovered and resolved using Coverity Static Analysis >> Security Testing (SAST) by Synopsys, Inc. >> >> Signed-off-by: Julien Grall <jgrall@amazon.com> >> >> --- >> Changes in v2: >> - Remove BUILD_BUG_ON() >> - Switch from min_t() to min() > > Since this fulfills the dependencies put in place at the time, the > Reviewed-by: Jan Beulich <jbeulich@suse.com> > continues to apply here. Thanks for making the adjustments. Thanks for the review. It is now committed. Cheers,
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index ace6333c18ea..958ba0cd9256 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -441,7 +441,7 @@ mfn_t __init alloc_boot_pages(unsigned long nr_pfns, unsigned long pfn_align) #define MEMZONE_XEN 0 #define NR_ZONES (PADDR_BITS - PAGE_SHIFT + 1) -#define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 1 : ((b) - PAGE_SHIFT)) +#define bits_to_zone(b) (((b) < (PAGE_SHIFT + 1)) ? 1U : ((b) - PAGE_SHIFT)) #define page_to_zone(pg) (is_xen_heap_page(pg) ? MEMZONE_XEN : \ (flsl(mfn_x(page_to_mfn(pg))) ? : 1)) @@ -2336,8 +2336,9 @@ struct page_info *alloc_domheap_pages( bits = domain_clamp_alloc_bitsize(memflags & MEMF_no_owner ? NULL : d, bits ? : (BITS_PER_LONG+PAGE_SHIFT)); - if ( (zone_hi = min_t(unsigned int, bits_to_zone(bits), zone_hi)) == 0 ) - return NULL; + + zone_hi = min(bits_to_zone(bits), zone_hi); + ASSERT(zone_hi != 0); if ( memflags & MEMF_no_owner ) memflags |= MEMF_no_refcount;