| Message ID | 20220223183831.5951-1-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/mm: Remove always true ASSERT() in free_heap_pages() | expand |
On 23/02/2022 18:38, Julien Grall wrote: From: Julien Grall <jgrall@amazon.com><mailto:jgrall@amazon.com> free_heap_pages() has an ASSERT() checking that node is >= 0. However node is defined as an unsigned int. So it cannot be negative. Therefore remove the check as it will always be true. Coverity-ID: 1055631 Signed-off-by: Julien Grall <jgrall@amazon.com><mailto:jgrall@amazon.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com><mailto:andrew.cooper3@citrix.com>
On 23.02.2022 19:38, Julien Grall wrote: > From: Julien Grall <jgrall@amazon.com> > > free_heap_pages() has an ASSERT() checking that node is >= 0. However > node is defined as an unsigned int. So it cannot be negative. > > Therefore remove the check as it will always be true. > > Signed-off-by: Julien Grall <jgrall@amazon.com> > > --- > > I have looked at the history. AFAICT, node has always be defined > as unsigned int. So the ASSERT() may have never been useful (?). Commit f0738d2d3f81 introduced "node" as a local variable of type "int". Along with this commit f1c6ac275100 introduced ia64's paddr_to_nid() (backing phys_to_nid()), which was able to return -1. Hence at the time the assertion fulfilled a purpose. I should have dropped it in bd3e1195d694. Acked-by: Jan Beulich <jbeulich@suse.com> Jan
Hi Jan, On 24/02/2022 08:27, Jan Beulich wrote: > On 23.02.2022 19:38, Julien Grall wrote: >> From: Julien Grall <jgrall@amazon.com> >> >> free_heap_pages() has an ASSERT() checking that node is >= 0. However >> node is defined as an unsigned int. So it cannot be negative. >> >> Therefore remove the check as it will always be true. >> >> Signed-off-by: Julien Grall <jgrall@amazon.com> >> >> --- >> >> I have looked at the history. AFAICT, node has always be defined >> as unsigned int. So the ASSERT() may have never been useful (?). > > Commit f0738d2d3f81 introduced "node" as a local variable of type > "int". Along with this commit f1c6ac275100 introduced ia64's > paddr_to_nid() (backing phys_to_nid()), which was able to return -1. > Hence at the time the assertion fulfilled a purpose. I should have > dropped it in bd3e1195d694. Thanks for the information. It looks like I need to brush my git-blame skill :). > > Acked-by: Jan Beulich <jbeulich@suse.com> Thanks! Cheers,
Hi Andrew, On 23/02/2022 19:30, Andrew Cooper wrote: > On 23/02/2022 18:38, Julien Grall wrote: >> From: Julien Grall<jgrall@amazon.com> >> >> free_heap_pages() has an ASSERT() checking that node is >= 0. However >> node is defined as an unsigned int. So it cannot be negative. >> >> Therefore remove the check as it will always be true. > > Coverity-ID: 1055631 I will add it while committing. > >> Signed-off-by: Julien Grall<jgrall@amazon.com> > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks! Cheers,
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index 46357182375a..e971bf91e0be 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -1441,7 +1441,6 @@ static void free_heap_pages( unsigned int zone = page_to_zone(pg); ASSERT(order <= MAX_ORDER); - ASSERT(node >= 0); spin_lock(&heap_lock);