[PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Yinghai Lu]

Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
during add range) broke mtrr cleanup on his setup in 3.9.5.
corresponding commit in upstream is fbe06b7bae7c.

The reason is add_range_with_merge could generate blank spot.

We could avoid that by searching new expanded start/end, that
new range should include all connected ranges in range array.
At last add the new expanded start/end to the range array.
Also move up left array so do not add new blank slot in the
range array.

-v2: move left array to avoid enhance add_range()

Reported-by: Joshua Covington <joshuacov@googlemail.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: <stable@vger.kernel.org> v3.9

---
 arch/x86/kernel/cpu/mtrr/cleanup.c |    2 +-
 kernel/range.c                     |   21 +++++++++++----------
 2 files changed, 12 insertions(+), 11 deletions(-)

Index: linux-2.6/kernel/range.c
===================================================================
--- linux-2.6.orig/kernel/range.c
+++ linux-2.6/kernel/range.c
@@ -32,9 +32,8 @@ int add_range_with_merge(struct range *r
 	if (start >= end)
 		return nr_range;
 
-	/* Try to merge it with old one: */
+	/* get new start/end: */
 	for (i = 0; i < nr_range; i++) {
-		u64 final_start, final_end;
 		u64 common_start, common_end;
 
 		if (!range[i].end)
@@ -45,14 +44,16 @@ int add_range_with_merge(struct range *r
 		if (common_start > common_end)
 			continue;
 
-		final_start = min(range[i].start, start);
-		final_end = max(range[i].end, end);
-
-		/* clear it and add it back for further merge */
-		range[i].start = 0;
-		range[i].end =  0;
-		return add_range_with_merge(range, az, nr_range,
-			final_start, final_end);
+		/* new start/end, will add it back at last */
+		start = min(range[i].start, start);
+		end = max(range[i].end, end);
+
+		memmove(&range[i], &range[i + 1],
+			(nr_range - (i + 1)) * sizeof(range[i]));
+		range[nr_range - 1].start = 0;
+		range[nr_range - 1].end   = 0;
+		nr_range--;
+		i--;
 	}
 
 	/* Need to add it: */

Re: [PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Sergey Meirovich]

Hi,

On 10 June 2013 09:29, Yinghai Lu <yinghai@kernel.org> wrote:
> Joshua reported: Commit cd7b304dfaf1 (x86, range: fix missing merge
> during add range) broke mtrr cleanup on his setup in 3.9.5.
> corresponding commit in upstream is fbe06b7bae7c.
>
> The reason is add_range_with_merge could generate blank spot.
>
> We could avoid that by searching new expanded start/end, that
> new range should include all connected ranges in range array.
> At last add the new expanded start/end to the range array.
> Also move up left array so do not add new blank slot in the
> range array.
>
> -v2: move left array to avoid enhance add_range()
>
> Reported-by: Joshua Covington <joshuacov@googlemail.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: <stable@vger.kernel.org> v3.9
>
> ---
>  arch/x86/kernel/cpu/mtrr/cleanup.c |    2 +-
>  kernel/range.c                     |   21 +++++++++++----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
>
> Index: linux-2.6/kernel/range.c
> ===================================================================
> --- linux-2.6.orig/kernel/range.c
> +++ linux-2.6/kernel/range.c
> @@ -32,9 +32,8 @@ int add_range_with_merge(struct range *r
>         if (start >= end)
>                 return nr_range;
>
> -       /* Try to merge it with old one: */
> +       /* get new start/end: */
>         for (i = 0; i < nr_range; i++) {
> -               u64 final_start, final_end;
>                 u64 common_start, common_end;
>
>                 if (!range[i].end)
> @@ -45,14 +44,16 @@ int add_range_with_merge(struct range *r
>                 if (common_start > common_end)
>                         continue;
>
> -               final_start = min(range[i].start, start);
> -               final_end = max(range[i].end, end);
> -
> -               /* clear it and add it back for further merge */
> -               range[i].start = 0;
> -               range[i].end =  0;
> -               return add_range_with_merge(range, az, nr_range,
> -                       final_start, final_end);
> +               /* new start/end, will add it back at last */
> +               start = min(range[i].start, start);
> +               end = max(range[i].end, end);
> +
> +               memmove(&range[i], &range[i + 1],
> +                       (nr_range - (i + 1)) * sizeof(range[i]));
> +               range[nr_range - 1].start = 0;
> +               range[nr_range - 1].end   = 0;
> +               nr_range--;
> +               i--;
>         }
>
>         /* Need to add it: */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

patches 1,2(latest one) have not helped me on 3.9.5:

rathamahata@piledriver /usr/local/src/linux-3.9.5 $ cat /proc/cmdline
BOOT_IMAGE=/boot/linux-3.9.5 root=/dev/sda1 rootflags=commit=9
elevator=cfq enable_mtrr_cleanup mtrr_spare_reg_nr=1
rathamahata@piledriver /usr/local/src/linux-3.9.5 $ cat /proc/mtrr
reg00: base=0x000000000 (    0MB), size= 2048MB, count=1: write-back
reg01: base=0x080000000 ( 2048MB), size=  256MB, count=1: write-back
reg02: base=0x08f800000 ( 2296MB), size=    8MB, count=1: uncachable
reg03: base=0x0b0000000 ( 2816MB), size=  256MB, count=1: write-combining
reg04: base=0x0c0000000 ( 3072MB), size=  256MB, count=1: write-combining
rathamahata@piledriver /usr/local/src/linux-3.9.5 $ free -m
             total       used       free     shared    buffers     cached
Mem:          7370        714       6656          0         35        243
-/+ buffers/cache:        434       6935
Swap:         3659          0       3659
rathamahata@piledriver /usr/local/src/linux-3.9.5 $

Re: [PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Yinghai Lu]

On Mon, Jun 10, 2013 at 3:55 AM, Sergey Meirovich <rathamahata@gmail.com> wrote:

> patches 1,2(latest one) have not helped me on 3.9.5:

So v1:
https://patchwork.kernel.org/patch/2694981/
https://patchwork.kernel.org/patch/2694971/

and v2:
https://patchwork.kernel.org/patch/2695891/
https://patchwork.kernel.org/patch/2695881/

Neither of two versions fix the problem on your setup?

Can you post boot log with mtrr_cleanup_debug on 3.9.4?

Thanks

Yinghai

Re: [PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Joshua C.]

2013/6/10 Yinghai Lu <yinghai@kernel.org>:
> On Mon, Jun 10, 2013 at 3:55 AM, Sergey Meirovich <rathamahata@gmail.com> wrote:
>
>> patches 1,2(latest one) have not helped me on 3.9.5:
>
> So v1:
> https://patchwork.kernel.org/patch/2694981/
> https://patchwork.kernel.org/patch/2694971/
>
> and v2:
> https://patchwork.kernel.org/patch/2695891/
> https://patchwork.kernel.org/patch/2695881/
>
> Neither of two versions fix the problem on your setup?
>
> Can you post boot log with mtrr_cleanup_debug on 3.9.4?
>
> Thanks
>
> Yinghai

I got some warnings when trying to build v2 of the patches:

# make -s ARCH=x86_64 V=1 -j4 bzImage
kernel/range.c: In function 'add_range_with_merge':
kernel/range.c:51:3: error: implicit declaration of function 'memmove'
[-Werror=implicit-function-declaration]
kernel/range.c:51:3: warning: incompatible implicit declaration of
built-in function 'memmove' [enabled by default]
cc1: some warnings being treated as errors
scripts/Makefile.build:307: recipe for target 'kernel/range.o' failed
make[1]: *** [kernel/range.o] Error 1
Makefile:793: recipe for target 'kernel' failed
make: *** [kernel] Error 2
make: *** Waiting for unfinished jobs....


--
--joshua

Re: [PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Joshua C.]

2013/6/11 Joshua C. <joshuacov@gmail.com>:
> 2013/6/10 Yinghai Lu <yinghai@kernel.org>:
>> On Mon, Jun 10, 2013 at 3:55 AM, Sergey Meirovich <rathamahata@gmail.com> wrote:
>>
>>> patches 1,2(latest one) have not helped me on 3.9.5:
>>
>> So v1:
>> https://patchwork.kernel.org/patch/2694981/
>> https://patchwork.kernel.org/patch/2694971/
>>
>> and v2:
>> https://patchwork.kernel.org/patch/2695891/
>> https://patchwork.kernel.org/patch/2695881/
>>
>> Neither of two versions fix the problem on your setup?
>>
>> Can you post boot log with mtrr_cleanup_debug on 3.9.4?
>>
>> Thanks
>>
>> Yinghai
>
> I got some warnings when trying to build v2 of the patches:
>
> # make -s ARCH=x86_64 V=1 -j4 bzImage
> kernel/range.c: In function 'add_range_with_merge':
> kernel/range.c:51:3: error: implicit declaration of function 'memmove'
> [-Werror=implicit-function-declaration]
> kernel/range.c:51:3: warning: incompatible implicit declaration of
> built-in function 'memmove' [enabled by default]
> cc1: some warnings being treated as errors
> scripts/Makefile.build:307: recipe for target 'kernel/range.o' failed
> make[1]: *** [kernel/range.o] Error 1
> Makefile:793: recipe for target 'kernel' failed
> make: *** [kernel] Error 2
> make: *** Waiting for unfinished jobs....
>
>
> --
> --joshua

A trivial fix for the above warnings:

--- linux-2.6.orig/kernel/range.c
+++ linux-2.6/kernel/range.c
@@ -4,6 +4,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/sort.h>
+#include <linux/string.h>

 #include <linux/range.h>



--
--joshua

Re: [PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Yinghai Lu]

On Tue, Jun 11, 2013 at 3:04 AM, Joshua C. <joshuacov@gmail.com> wrote:
> 2013/6/11 Joshua C. <joshuacov@gmail.com>:
>> 2013/6/10 Yinghai Lu <yinghai@kernel.org>:
>>> On Mon, Jun 10, 2013 at 3:55 AM, Sergey Meirovich <rathamahata@gmail.com> wrote:
>>>
>>>> patches 1,2(latest one) have not helped me on 3.9.5:
>>>
>>> So v1:
>>> https://patchwork.kernel.org/patch/2694981/
>>> https://patchwork.kernel.org/patch/2694971/
>>>
>>> and v2:
>>> https://patchwork.kernel.org/patch/2695891/
>>> https://patchwork.kernel.org/patch/2695881/
>>>
>>> Neither of two versions fix the problem on your setup?
>>>
>>> Can you post boot log with mtrr_cleanup_debug on 3.9.4?
>>>
>>> Thanks
>>>
>>> Yinghai
>>
>> I got some warnings when trying to build v2 of the patches:
>>
>> # make -s ARCH=x86_64 V=1 -j4 bzImage
>> kernel/range.c: In function 'add_range_with_merge':
>> kernel/range.c:51:3: error: implicit declaration of function 'memmove'
>> [-Werror=implicit-function-declaration]
>> kernel/range.c:51:3: warning: incompatible implicit declaration of
>> built-in function 'memmove' [enabled by default]
>> cc1: some warnings being treated as errors
>> scripts/Makefile.build:307: recipe for target 'kernel/range.o' failed
>> make[1]: *** [kernel/range.o] Error 1
>> Makefile:793: recipe for target 'kernel' failed
>> make: *** [kernel] Error 2
>> make: *** Waiting for unfinished jobs....
>>
>>
>> --
>> --joshua

it does not report warning when I compile it with

gcc (SUSE Linux) 4.7.2 20130108 [gcc-4_7-branch revision 195012

>
> A trivial fix for the above warnings:
>
> --- linux-2.6.orig/kernel/range.c
> +++ linux-2.6/kernel/range.c
> @@ -4,6 +4,7 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/sort.h>
> +#include <linux/string.h>
>
>  #include <linux/range.h>
>

Re: [PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Joshua C.]

2013/6/11 Yinghai Lu <yinghai@kernel.org>:
> On Tue, Jun 11, 2013 at 3:04 AM, Joshua C. <joshuacov@gmail.com> wrote:
>> 2013/6/11 Joshua C. <joshuacov@gmail.com>:
>>> 2013/6/10 Yinghai Lu <yinghai@kernel.org>:
>>>> On Mon, Jun 10, 2013 at 3:55 AM, Sergey Meirovich <rathamahata@gmail.com> wrote:
>>>>
>>>>> patches 1,2(latest one) have not helped me on 3.9.5:
>>>>
>>>> So v1:
>>>> https://patchwork.kernel.org/patch/2694981/
>>>> https://patchwork.kernel.org/patch/2694971/
>>>>
>>>> and v2:
>>>> https://patchwork.kernel.org/patch/2695891/
>>>> https://patchwork.kernel.org/patch/2695881/
>>>>
>>>> Neither of two versions fix the problem on your setup?
>>>>
>>>> Can you post boot log with mtrr_cleanup_debug on 3.9.4?
>>>>
>>>> Thanks
>>>>
>>>> Yinghai
>>>
>>> I got some warnings when trying to build v2 of the patches:
>>>
>>> # make -s ARCH=x86_64 V=1 -j4 bzImage
>>> kernel/range.c: In function 'add_range_with_merge':
>>> kernel/range.c:51:3: error: implicit declaration of function 'memmove'
>>> [-Werror=implicit-function-declaration]
>>> kernel/range.c:51:3: warning: incompatible implicit declaration of
>>> built-in function 'memmove' [enabled by default]
>>> cc1: some warnings being treated as errors
>>> scripts/Makefile.build:307: recipe for target 'kernel/range.o' failed
>>> make[1]: *** [kernel/range.o] Error 1
>>> Makefile:793: recipe for target 'kernel' failed
>>> make: *** [kernel] Error 2
>>> make: *** Waiting for unfinished jobs....
>>>
>>>
>>> --
>>> --joshua
>
> it does not report warning when I compile it with
>
> gcc (SUSE Linux) 4.7.2 20130108 [gcc-4_7-branch revision 195012
>
>>
>> A trivial fix for the above warnings:
>>
>> --- linux-2.6.orig/kernel/range.c
>> +++ linux-2.6/kernel/range.c
>> @@ -4,6 +4,7 @@
>>  #include <linux/kernel.h>
>>  #include <linux/init.h>
>>  #include <linux/sort.h>
>> +#include <linux/string.h>
>>
>>  #include <linux/range.h>
>>

I use fedora17 with gcc version 4.7.3 (git checkout on 20130411). I
have no idea where this came from...


--
--joshua

Re: [PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Yinghai Lu]

On Tue, Jun 11, 2013 at 10:08 AM, Joshua C. <joshuacov@gmail.com> wrote:
> 2013/6/11 Yinghai Lu <yinghai@kernel.org>:
>> On Tue, Jun 11, 2013 at 3:04 AM, Joshua C. <joshuacov@gmail.com> wrote:
>>> 2013/6/11 Joshua C. <joshuacov@gmail.com>:
>>>> 2013/6/10 Yinghai Lu <yinghai@kernel.org>:
>>>>> On Mon, Jun 10, 2013 at 3:55 AM, Sergey Meirovich <rathamahata@gmail.com> wrote:
>>>>>
>>>>>> patches 1,2(latest one) have not helped me on 3.9.5:
>>>>>
>>>>> So v1:
>>>>> https://patchwork.kernel.org/patch/2694981/
>>>>> https://patchwork.kernel.org/patch/2694971/
>>>>>
>>>>> and v2:
>>>>> https://patchwork.kernel.org/patch/2695891/
>>>>> https://patchwork.kernel.org/patch/2695881/
>>>>>
>>>>> Neither of two versions fix the problem on your setup?
>>>>>
>>>>> Can you post boot log with mtrr_cleanup_debug on 3.9.4?
>>>>>
>>>>> Thanks
>>>>>
>>>>> Yinghai
>>>>
>>>> I got some warnings when trying to build v2 of the patches:
>>>>
>>>> # make -s ARCH=x86_64 V=1 -j4 bzImage
>>>> kernel/range.c: In function 'add_range_with_merge':
>>>> kernel/range.c:51:3: error: implicit declaration of function 'memmove'
>>>> [-Werror=implicit-function-declaration]
>>>> kernel/range.c:51:3: warning: incompatible implicit declaration of
>>>> built-in function 'memmove' [enabled by default]
>>>> cc1: some warnings being treated as errors
>>>> scripts/Makefile.build:307: recipe for target 'kernel/range.o' failed
>>>> make[1]: *** [kernel/range.o] Error 1
>>>> Makefile:793: recipe for target 'kernel' failed
>>>> make: *** [kernel] Error 2
>>>> make: *** Waiting for unfinished jobs....
>>>>
>>>>
>>>> --
>>>> --joshua
>>
>> it does not report warning when I compile it with
>>
>> gcc (SUSE Linux) 4.7.2 20130108 [gcc-4_7-branch revision 195012
>>
>>>
>>> A trivial fix for the above warnings:
>>>
>>> --- linux-2.6.orig/kernel/range.c
>>> +++ linux-2.6/kernel/range.c
>>> @@ -4,6 +4,7 @@
>>>  #include <linux/kernel.h>
>>>  #include <linux/init.h>
>>>  #include <linux/sort.h>
>>> +#include <linux/string.h>
>>>
>>>  #include <linux/range.h>
>>>
>
> I use fedora17 with gcc version 4.7.3 (git checkout on 20130411). I
> have no idea where this came from...

Ok. Your config should have CONFIG_DYNAMIC_DEBUG defined,
and my config does not have that defined.

we have linux/kernel.h
                linux/printk.h
                linux/dynamic_debug.h

dynamic_debug.h will include linux/string.h when CONFIG_DYNAMIC_DEBUG
is not defined.

Hi, Andrew,

How to fix this inconsistency caused by dynamic_debug.h

1. let linux/kernel.h include string.h directly
2. or let dynamic_debug include string.h always
3. let printk.h include string.h.
...

Yinghai

Re: [PATCH v2 2/2] x86, range: Do not add new blank slot with add_range_with_merge

[Joshua C.]

2013/6/11 Yinghai Lu <yinghai@kernel.org>:
> On Tue, Jun 11, 2013 at 10:08 AM, Joshua C. <joshuacov@gmail.com> wrote:
>> 2013/6/11 Yinghai Lu <yinghai@kernel.org>:
>>> On Tue, Jun 11, 2013 at 3:04 AM, Joshua C. <joshuacov@gmail.com> wrote:
>>>> 2013/6/11 Joshua C. <joshuacov@gmail.com>:
>>>>> 2013/6/10 Yinghai Lu <yinghai@kernel.org>:
>>>>>> On Mon, Jun 10, 2013 at 3:55 AM, Sergey Meirovich <rathamahata@gmail.com> wrote:
>>>>>>
>>>>>>> patches 1,2(latest one) have not helped me on 3.9.5:
>>>>>>
>>>>>> So v1:
>>>>>> https://patchwork.kernel.org/patch/2694981/
>>>>>> https://patchwork.kernel.org/patch/2694971/
>>>>>>
>>>>>> and v2:
>>>>>> https://patchwork.kernel.org/patch/2695891/
>>>>>> https://patchwork.kernel.org/patch/2695881/
>>>>>>
>>>>>> Neither of two versions fix the problem on your setup?
>>>>>>
>>>>>> Can you post boot log with mtrr_cleanup_debug on 3.9.4?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Yinghai
>>>>>
>>>>> I got some warnings when trying to build v2 of the patches:
>>>>>
>>>>> # make -s ARCH=x86_64 V=1 -j4 bzImage
>>>>> kernel/range.c: In function 'add_range_with_merge':
>>>>> kernel/range.c:51:3: error: implicit declaration of function 'memmove'
>>>>> [-Werror=implicit-function-declaration]
>>>>> kernel/range.c:51:3: warning: incompatible implicit declaration of
>>>>> built-in function 'memmove' [enabled by default]
>>>>> cc1: some warnings being treated as errors
>>>>> scripts/Makefile.build:307: recipe for target 'kernel/range.o' failed
>>>>> make[1]: *** [kernel/range.o] Error 1
>>>>> Makefile:793: recipe for target 'kernel' failed
>>>>> make: *** [kernel] Error 2
>>>>> make: *** Waiting for unfinished jobs....
>>>>>
>>>>>
>>>>> --
>>>>> --joshua
>>>
>>> it does not report warning when I compile it with
>>>
>>> gcc (SUSE Linux) 4.7.2 20130108 [gcc-4_7-branch revision 195012
>>>
>>>>
>>>> A trivial fix for the above warnings:
>>>>
>>>> --- linux-2.6.orig/kernel/range.c
>>>> +++ linux-2.6/kernel/range.c
>>>> @@ -4,6 +4,7 @@
>>>>  #include <linux/kernel.h>
>>>>  #include <linux/init.h>
>>>>  #include <linux/sort.h>
>>>> +#include <linux/string.h>
>>>>
>>>>  #include <linux/range.h>
>>>>
>>
>> I use fedora17 with gcc version 4.7.3 (git checkout on 20130411). I
>> have no idea where this came from...
>
> Ok. Your config should have CONFIG_DYNAMIC_DEBUG defined,
> and my config does not have that defined.
>
> we have linux/kernel.h
>                 linux/printk.h
>                 linux/dynamic_debug.h
>
> dynamic_debug.h will include linux/string.h when CONFIG_DYNAMIC_DEBUG
> is not defined.
>
> Hi, Andrew,
>
> How to fix this inconsistency caused by dynamic_debug.h
>
> 1. let linux/kernel.h include string.h directly
> 2. or let dynamic_debug include string.h always
> 3. let printk.h include string.h.
> ...
>
> Yinghai

You guys are better at kernel coding than I am, but can't we just
directly add 'string.h' just like the small patch I posted? In the
worst case the file will be included twice... Maybe this is not a good
coding style but it solves the problem and doesn't cause any side
effects. So why not do it the way I did?

--
--joshua