* [PATCH] fix off-by-one mistakes in vm_alloc()
@ 2013-07-18 9:59 Jan Beulich
2013-08-05 13:08 ` Ping: " Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2013-07-18 9:59 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 2629 bytes --]
Also add another pair of assertions to catch eventual further cases of
incorrect accounting, and remove the temporary debuggin messages again
which commit 68caac7f ("x86: don't use destroy_xen_mappings() for
vunmap()") added.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -57,8 +57,8 @@ void *vm_alloc(unsigned int nr, unsigned
{
struct page_info *pg;
- ASSERT(!test_bit(vm_low, vm_bitmap));
- for ( start = vm_low; ; )
+ ASSERT(vm_low == vm_top || !test_bit(vm_low, vm_bitmap));
+ for ( start = vm_low; start < vm_top; )
{
bit = find_next_bit(vm_bitmap, vm_top, start + 1);
if ( bit > vm_top )
@@ -68,12 +68,18 @@ void *vm_alloc(unsigned int nr, unsigned
* corresponding page a guard one.
*/
start = (start + align) & ~(align - 1);
- if ( start + nr <= bit )
- break;
- start = bit < vm_top ?
- find_next_zero_bit(vm_bitmap, vm_top, bit + 1) : bit;
- if ( start >= vm_top )
- break;
+ if ( bit < vm_top )
+ {
+ if ( start + nr < bit )
+ break;
+ start = find_next_zero_bit(vm_bitmap, vm_top, bit + 1);
+ }
+ else
+ {
+ if ( start + nr <= bit )
+ break;
+ start = bit;
+ }
}
if ( start < vm_top )
@@ -115,6 +121,10 @@ void *vm_alloc(unsigned int nr, unsigned
for ( bit = start; bit < start + nr; ++bit )
__set_bit(bit, vm_bitmap);
+ if ( bit < vm_top )
+ ASSERT(!test_bit(bit, vm_bitmap));
+ else
+ ASSERT(bit == vm_top);
if ( start <= vm_low + 2 )
vm_low = bit;
spin_unlock(&vm_lock);
@@ -177,7 +187,6 @@ void *__vmap(const unsigned long *mfn, u
void *va = vm_alloc(nr * granularity, align);
unsigned long cur = (unsigned long)va;
-printk("vmap(%p:%#x)\n", va, nr * granularity);//temp
for ( ; va && nr--; ++mfn, cur += PAGE_SIZE * granularity )
{
if ( map_pages_to_xen(cur, *mfn, granularity, flags) )
@@ -202,7 +211,6 @@ void vunmap(const void *va)
destroy_xen_mappings(addr, addr + PAGE_SIZE * vm_size(va));
#else /* Avoid tearing down intermediate page tables. */
-printk("vunmap(%p:%#x)\n", va, vm_size(va));//temp
map_pages_to_xen((unsigned long)va, 0, vm_size(va), _PAGE_NONE);
#endif
vm_free(va);
[-- Attachment #2: vmap-off-by-one.patch --]
[-- Type: text/plain, Size: 2664 bytes --]
fix off-by-one mistakes in vm_alloc()
Also add another pair of assertions to catch eventual further cases of
incorrect accounting, and remove the temporary debuggin messages again
which commit 68caac7f ("x86: don't use destroy_xen_mappings() for
vunmap()") added.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -57,8 +57,8 @@ void *vm_alloc(unsigned int nr, unsigned
{
struct page_info *pg;
- ASSERT(!test_bit(vm_low, vm_bitmap));
- for ( start = vm_low; ; )
+ ASSERT(vm_low == vm_top || !test_bit(vm_low, vm_bitmap));
+ for ( start = vm_low; start < vm_top; )
{
bit = find_next_bit(vm_bitmap, vm_top, start + 1);
if ( bit > vm_top )
@@ -68,12 +68,18 @@ void *vm_alloc(unsigned int nr, unsigned
* corresponding page a guard one.
*/
start = (start + align) & ~(align - 1);
- if ( start + nr <= bit )
- break;
- start = bit < vm_top ?
- find_next_zero_bit(vm_bitmap, vm_top, bit + 1) : bit;
- if ( start >= vm_top )
- break;
+ if ( bit < vm_top )
+ {
+ if ( start + nr < bit )
+ break;
+ start = find_next_zero_bit(vm_bitmap, vm_top, bit + 1);
+ }
+ else
+ {
+ if ( start + nr <= bit )
+ break;
+ start = bit;
+ }
}
if ( start < vm_top )
@@ -115,6 +121,10 @@ void *vm_alloc(unsigned int nr, unsigned
for ( bit = start; bit < start + nr; ++bit )
__set_bit(bit, vm_bitmap);
+ if ( bit < vm_top )
+ ASSERT(!test_bit(bit, vm_bitmap));
+ else
+ ASSERT(bit == vm_top);
if ( start <= vm_low + 2 )
vm_low = bit;
spin_unlock(&vm_lock);
@@ -177,7 +187,6 @@ void *__vmap(const unsigned long *mfn, u
void *va = vm_alloc(nr * granularity, align);
unsigned long cur = (unsigned long)va;
-printk("vmap(%p:%#x)\n", va, nr * granularity);//temp
for ( ; va && nr--; ++mfn, cur += PAGE_SIZE * granularity )
{
if ( map_pages_to_xen(cur, *mfn, granularity, flags) )
@@ -202,7 +211,6 @@ void vunmap(const void *va)
destroy_xen_mappings(addr, addr + PAGE_SIZE * vm_size(va));
#else /* Avoid tearing down intermediate page tables. */
-printk("vunmap(%p:%#x)\n", va, vm_size(va));//temp
map_pages_to_xen((unsigned long)va, 0, vm_size(va), _PAGE_NONE);
#endif
vm_free(va);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Ping: [PATCH] fix off-by-one mistakes in vm_alloc()
2013-07-18 9:59 [PATCH] fix off-by-one mistakes in vm_alloc() Jan Beulich
@ 2013-08-05 13:08 ` Jan Beulich
2013-08-05 14:54 ` Andrew Cooper
2013-08-05 16:03 ` Keir Fraser
0 siblings, 2 replies; 4+ messages in thread
From: Jan Beulich @ 2013-08-05 13:08 UTC (permalink / raw)
To: Keir Fraser; +Cc: xen-devel
Ping? (This is expected to fix recurring testsuite failures.)
>>> On 18.07.13 at 11:59, "Jan Beulich" <JBeulich@suse.com> wrote:
> Also add another pair of assertions to catch eventual further cases of
> incorrect accounting, and remove the temporary debuggin messages again
> which commit 68caac7f ("x86: don't use destroy_xen_mappings() for
> vunmap()") added.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/common/vmap.c
> +++ b/xen/common/vmap.c
> @@ -57,8 +57,8 @@ void *vm_alloc(unsigned int nr, unsigned
> {
> struct page_info *pg;
>
> - ASSERT(!test_bit(vm_low, vm_bitmap));
> - for ( start = vm_low; ; )
> + ASSERT(vm_low == vm_top || !test_bit(vm_low, vm_bitmap));
> + for ( start = vm_low; start < vm_top; )
> {
> bit = find_next_bit(vm_bitmap, vm_top, start + 1);
> if ( bit > vm_top )
> @@ -68,12 +68,18 @@ void *vm_alloc(unsigned int nr, unsigned
> * corresponding page a guard one.
> */
> start = (start + align) & ~(align - 1);
> - if ( start + nr <= bit )
> - break;
> - start = bit < vm_top ?
> - find_next_zero_bit(vm_bitmap, vm_top, bit + 1) : bit;
> - if ( start >= vm_top )
> - break;
> + if ( bit < vm_top )
> + {
> + if ( start + nr < bit )
> + break;
> + start = find_next_zero_bit(vm_bitmap, vm_top, bit + 1);
> + }
> + else
> + {
> + if ( start + nr <= bit )
> + break;
> + start = bit;
> + }
> }
>
> if ( start < vm_top )
> @@ -115,6 +121,10 @@ void *vm_alloc(unsigned int nr, unsigned
>
> for ( bit = start; bit < start + nr; ++bit )
> __set_bit(bit, vm_bitmap);
> + if ( bit < vm_top )
> + ASSERT(!test_bit(bit, vm_bitmap));
> + else
> + ASSERT(bit == vm_top);
> if ( start <= vm_low + 2 )
> vm_low = bit;
> spin_unlock(&vm_lock);
> @@ -177,7 +187,6 @@ void *__vmap(const unsigned long *mfn, u
> void *va = vm_alloc(nr * granularity, align);
> unsigned long cur = (unsigned long)va;
>
> -printk("vmap(%p:%#x)\n", va, nr * granularity);//temp
> for ( ; va && nr--; ++mfn, cur += PAGE_SIZE * granularity )
> {
> if ( map_pages_to_xen(cur, *mfn, granularity, flags) )
> @@ -202,7 +211,6 @@ void vunmap(const void *va)
>
> destroy_xen_mappings(addr, addr + PAGE_SIZE * vm_size(va));
> #else /* Avoid tearing down intermediate page tables. */
> -printk("vunmap(%p:%#x)\n", va, vm_size(va));//temp
> map_pages_to_xen((unsigned long)va, 0, vm_size(va), _PAGE_NONE);
> #endif
> vm_free(va);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Ping: [PATCH] fix off-by-one mistakes in vm_alloc()
2013-08-05 13:08 ` Ping: " Jan Beulich
@ 2013-08-05 14:54 ` Andrew Cooper
2013-08-05 16:03 ` Keir Fraser
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2013-08-05 14:54 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser
On 05/08/13 14:08, Jan Beulich wrote:
> Ping? (This is expected to fix recurring testsuite failures.)
>
>>>> On 18.07.13 at 11:59, "Jan Beulich" <JBeulich@suse.com> wrote:
>> Also add another pair of assertions to catch eventual further cases of
>> incorrect accounting, and remove the temporary debuggin messages again
>> which commit 68caac7f ("x86: don't use destroy_xen_mappings() for
>> vunmap()") added.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
(Tentatively, if I have followed the logic correctly)
Reviewed-by Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> --- a/xen/common/vmap.c
>> +++ b/xen/common/vmap.c
>> @@ -57,8 +57,8 @@ void *vm_alloc(unsigned int nr, unsigned
>> {
>> struct page_info *pg;
>>
>> - ASSERT(!test_bit(vm_low, vm_bitmap));
>> - for ( start = vm_low; ; )
>> + ASSERT(vm_low == vm_top || !test_bit(vm_low, vm_bitmap));
>> + for ( start = vm_low; start < vm_top; )
>> {
>> bit = find_next_bit(vm_bitmap, vm_top, start + 1);
>> if ( bit > vm_top )
>> @@ -68,12 +68,18 @@ void *vm_alloc(unsigned int nr, unsigned
>> * corresponding page a guard one.
>> */
>> start = (start + align) & ~(align - 1);
>> - if ( start + nr <= bit )
>> - break;
>> - start = bit < vm_top ?
>> - find_next_zero_bit(vm_bitmap, vm_top, bit + 1) : bit;
>> - if ( start >= vm_top )
>> - break;
>> + if ( bit < vm_top )
>> + {
>> + if ( start + nr < bit )
>> + break;
>> + start = find_next_zero_bit(vm_bitmap, vm_top, bit + 1);
>> + }
>> + else
>> + {
>> + if ( start + nr <= bit )
>> + break;
>> + start = bit;
>> + }
>> }
>>
>> if ( start < vm_top )
>> @@ -115,6 +121,10 @@ void *vm_alloc(unsigned int nr, unsigned
>>
>> for ( bit = start; bit < start + nr; ++bit )
>> __set_bit(bit, vm_bitmap);
>> + if ( bit < vm_top )
>> + ASSERT(!test_bit(bit, vm_bitmap));
>> + else
>> + ASSERT(bit == vm_top);
>> if ( start <= vm_low + 2 )
>> vm_low = bit;
>> spin_unlock(&vm_lock);
>> @@ -177,7 +187,6 @@ void *__vmap(const unsigned long *mfn, u
>> void *va = vm_alloc(nr * granularity, align);
>> unsigned long cur = (unsigned long)va;
>>
>> -printk("vmap(%p:%#x)\n", va, nr * granularity);//temp
>> for ( ; va && nr--; ++mfn, cur += PAGE_SIZE * granularity )
>> {
>> if ( map_pages_to_xen(cur, *mfn, granularity, flags) )
>> @@ -202,7 +211,6 @@ void vunmap(const void *va)
>>
>> destroy_xen_mappings(addr, addr + PAGE_SIZE * vm_size(va));
>> #else /* Avoid tearing down intermediate page tables. */
>> -printk("vunmap(%p:%#x)\n", va, vm_size(va));//temp
>> map_pages_to_xen((unsigned long)va, 0, vm_size(va), _PAGE_NONE);
>> #endif
>> vm_free(va);
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Ping: [PATCH] fix off-by-one mistakes in vm_alloc()
2013-08-05 13:08 ` Ping: " Jan Beulich
2013-08-05 14:54 ` Andrew Cooper
@ 2013-08-05 16:03 ` Keir Fraser
1 sibling, 0 replies; 4+ messages in thread
From: Keir Fraser @ 2013-08-05 16:03 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On 05/08/2013 14:08, "Jan Beulich" <JBeulich@suse.com> wrote:
> Ping? (This is expected to fix recurring testsuite failures.)
>
>>>> On 18.07.13 at 11:59, "Jan Beulich" <JBeulich@suse.com> wrote:
>> Also add another pair of assertions to catch eventual further cases of
>> incorrect accounting, and remove the temporary debuggin messages again
>> which commit 68caac7f ("x86: don't use destroy_xen_mappings() for
>> vunmap()") added.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
>>
>> --- a/xen/common/vmap.c
>> +++ b/xen/common/vmap.c
>> @@ -57,8 +57,8 @@ void *vm_alloc(unsigned int nr, unsigned
>> {
>> struct page_info *pg;
>>
>> - ASSERT(!test_bit(vm_low, vm_bitmap));
>> - for ( start = vm_low; ; )
>> + ASSERT(vm_low == vm_top || !test_bit(vm_low, vm_bitmap));
>> + for ( start = vm_low; start < vm_top; )
>> {
>> bit = find_next_bit(vm_bitmap, vm_top, start + 1);
>> if ( bit > vm_top )
>> @@ -68,12 +68,18 @@ void *vm_alloc(unsigned int nr, unsigned
>> * corresponding page a guard one.
>> */
>> start = (start + align) & ~(align - 1);
>> - if ( start + nr <= bit )
>> - break;
>> - start = bit < vm_top ?
>> - find_next_zero_bit(vm_bitmap, vm_top, bit + 1) : bit;
>> - if ( start >= vm_top )
>> - break;
>> + if ( bit < vm_top )
>> + {
>> + if ( start + nr < bit )
>> + break;
>> + start = find_next_zero_bit(vm_bitmap, vm_top, bit + 1);
>> + }
>> + else
>> + {
>> + if ( start + nr <= bit )
>> + break;
>> + start = bit;
>> + }
>> }
>>
>> if ( start < vm_top )
>> @@ -115,6 +121,10 @@ void *vm_alloc(unsigned int nr, unsigned
>>
>> for ( bit = start; bit < start + nr; ++bit )
>> __set_bit(bit, vm_bitmap);
>> + if ( bit < vm_top )
>> + ASSERT(!test_bit(bit, vm_bitmap));
>> + else
>> + ASSERT(bit == vm_top);
>> if ( start <= vm_low + 2 )
>> vm_low = bit;
>> spin_unlock(&vm_lock);
>> @@ -177,7 +187,6 @@ void *__vmap(const unsigned long *mfn, u
>> void *va = vm_alloc(nr * granularity, align);
>> unsigned long cur = (unsigned long)va;
>>
>> -printk("vmap(%p:%#x)\n", va, nr * granularity);//temp
>> for ( ; va && nr--; ++mfn, cur += PAGE_SIZE * granularity )
>> {
>> if ( map_pages_to_xen(cur, *mfn, granularity, flags) )
>> @@ -202,7 +211,6 @@ void vunmap(const void *va)
>>
>> destroy_xen_mappings(addr, addr + PAGE_SIZE * vm_size(va));
>> #else /* Avoid tearing down intermediate page tables. */
>> -printk("vunmap(%p:%#x)\n", va, vm_size(va));//temp
>> map_pages_to_xen((unsigned long)va, 0, vm_size(va), _PAGE_NONE);
>> #endif
>> vm_free(va);
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-05 16:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-18 9:59 [PATCH] fix off-by-one mistakes in vm_alloc() Jan Beulich
2013-08-05 13:08 ` Ping: " Jan Beulich
2013-08-05 14:54 ` Andrew Cooper
2013-08-05 16:03 ` Keir Fraser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).