| Message ID | 1556916614-21512-2-git-send-email-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] xen/arm: fix nr_pdxs calculation | expand |
>>> On 03.05.19 at 22:50, <sstabellini@kernel.org> wrote: > Fix the issue by passing j+1 and i+1 to find_next_zero_bit and > find_next_bit. Also add a check for i >= BITS_PER_LONG because > find_{,next_}zero_bit() on x86 assume their last argument to be less > than their middle one. I had pointed out x86 since I knew it has this assumption. Now that you mention it here, I would have expected you've checked that Arm doesn't make similar assumptions. 32-bit Arm looks to do, though (while 64-bit has a dedicated if() to deal with the situation). Jan
On Mon, 6 May 2019, Jan Beulich wrote: > >>> On 03.05.19 at 22:50, <sstabellini@kernel.org> wrote: > > Fix the issue by passing j+1 and i+1 to find_next_zero_bit and > > find_next_bit. Also add a check for i >= BITS_PER_LONG because > > find_{,next_}zero_bit() on x86 assume their last argument to be less > > than their middle one. > > I had pointed out x86 since I knew it has this assumption. Now > that you mention it here, I would have expected you've checked > that Arm doesn't make similar assumptions. 32-bit Arm looks to > do, though (while 64-bit has a dedicated if() to deal with the > situation). I think that either way we want to say that those functions are not supposed to be called that way. I'll update the commit message.
diff --git a/xen/common/pdx.c b/xen/common/pdx.c index 50c21b6..bb7e437 100644 --- a/xen/common/pdx.c +++ b/xen/common/pdx.c @@ -83,8 +83,10 @@ void __init pfn_pdx_hole_setup(unsigned long mask) */ for ( j = MAX_ORDER-1; ; ) { - i = find_next_zero_bit(&mask, BITS_PER_LONG, j); - j = find_next_bit(&mask, BITS_PER_LONG, i); + i = find_next_zero_bit(&mask, BITS_PER_LONG, j + 1); + if ( i >= BITS_PER_LONG ) + break; + j = find_next_bit(&mask, BITS_PER_LONG, i + 1); if ( j >= BITS_PER_LONG ) break; if ( j - i > hole_shift )
pfn_pdx_hole_setup is meant to skip the first MAX_ORDER bits, but actually it only skips the first MAX_ORDER-1 bits. The issue was probably introduced by bdb5439c3f, when changing to loop to start from MAX_ORDER-1 an adjustment by 1 was needed in the call to find_next_bit() but not done. Fix the issue by passing j+1 and i+1 to find_next_zero_bit and find_next_bit. Also add a check for i >= BITS_PER_LONG because find_{,next_}zero_bit() on x86 assume their last argument to be less than their middle one. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> Signed-off-by: Jan Beulich <JBeulich@suse.com> CC: andrew.cooper3@citrix.com CC: JBeulich@suse.com CC: George.Dunlap@eu.citrix.com CC: ian.jackson@eu.citrix.com CC: konrad.wilk@oracle.com CC: tim@xen.org CC: wei.liu2@citrix.com --- xen/common/pdx.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)