Re: HPET regression in 2.6.26 versus 2.6.25 -- retried 2.6.27-rc3 patch (and patch method)

From: David Witbrodt
Date: Thu Aug 14 2008 - 23:41:08 EST




> > I used 'git apply --check ' first, and got no errors, so
> > I applied it, built, installed, and rebooted.
>
> could apply with
> cat revert_*.;patch | patch -p1

Yinghai,

When I got home from work tonight, I decided to make sure that I applied
your patch correctly. I considered trying it once with cat/patch and
once with 'git apply', and then comparing the results with 'diff'. I
ended up becoming convinced that 'git apply' worked just fine, and
never tried the cat/patch method:

=================================
$ git show |head
commit 30a2f3c60a84092c8084dfe788b710f8d0768cd4
Author: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Tue Aug 12 18:55:39 2008 -0700

Linux 2.6.27-rc3

diff --git a/Makefile b/Makefile
index fd3ca6e..53bf6ec 100644
--- a/Makefile
+++ b/Makefile

$ git apply --check --verbose ../yh-patch1-2.6.27-rc3.diff
Checking patch arch/x86/kernel/e820.c...
Checking patch arch/x86/kernel/setup.c...
Checking patch include/asm-x86/e820.h...

$ git apply --verbose ../yh-patch1-2.6.27-rc3.diff
Checking patch arch/x86/kernel/e820.c...
Checking patch arch/x86/kernel/setup.c...
Checking patch include/asm-x86/e820.h...
Applied patch arch/x86/kernel/e820.c cleanly.
Applied patch arch/x86/kernel/setup.c cleanly.
Applied patch include/asm-x86/e820.h cleanly.

dawitbro@fileserver:~/sandbox/git-kernel/linux-2.6$ git status
# Not currently on any branch.
# Changed but not updated:
# (use "git add <file>..." to update what will be committed)
#
# modified: arch/x86/kernel/e820.c
# modified: arch/x86/kernel/setup.c
# modified: include/asm-x86/e820.h
#
no changes added to commit (use "git add" and/or "git commit -a")

$ git diff > ../yh-patch1-2.6.27-rc3.git-apply.diff
=================================

As you can see, I renamed your patch file to 'yh-patch1-2.6.27-rc3.diff', and
after applying your patch I created a diff against 2.6.27-rc3 called
'yh-patch1-2.6.27-rc3.git-apply.diff'.

I have attached the output of the following command so that you can see that
your patch applied correctly:

diff -y -W 200 yh-patch1-2.6.27-rc3.diff yh-patch1-2.6.27-rc3.git-apply.diff

At this point, I retried a kernel with your patch...


> can you try enable kexec and kdump in you .config.
>
> it should works. my .config have config_kexec

I ran 'make oldconfig' to get my .config up to date, then 'make menuconfig'
to make sure I had enabled CONFIG_KEXEC and CONFIG_CRASH_DUMP:

$ egrep 'KEXEC|DUMP' .config
CONFIG_KEXEC=y
CONFIG_CRASH_DUMP=y


I took these steps, and posted this info, for the sake of our individual and
collective sanity! ;-) I want you all to be sure that I applied the patch
correctly, and adjusted the .config as requested, before building the kernel.

The kernel built without error, so I installed and rebooted. It locked up in
the usual way. I rebooted, using the "hpet=disable" parameter, and it booted
just fine, just like all the others since 3def3d6d...


>> I do not know how to bisect with your patch if I have a "bad" but no "good"
>> to start with. Can you explain how I should proceed when I _do_ get home?
>> (I can just enabled those config options and try the patch again, but I am
>> confused about the bisect you are asking me to perform.)
>
> just like the old way doing git-bisect, but before compiling, apply
> the batch, and before git-bisect good or bad, revert the patch.

Since I am still confused about how I should perform the bisect you are
asking for, I will wait until you can clarify. I responded earlier from
work, where I had no access to a Linux machine, so I could not quote the
git documentation I have read... to explain better why I am confused. Here
are the sections of 'git help bisect' that I had in mind:

===== BEGIN QUOTED SECTIONS =================
[...]
This command uses git-rev-list --bisect option to help drive the binary
search process to find which change introduced a bug, given an old
"good" commit object name and a later "bad" commit object name.

[...]
Basic bisect commands: start, bad, good

The way you use it is:


$ git bisect start
$ git bisect bad # Current version is bad
$ git bisect good v2.6.13-rc2 # v2.6.13-rc2 was the last version
# tested that was good
When you give at least one bad and one good versions, it will bisect
the revision tree and say something like:

Bisecting: 675 revisions left to test after this

[...]
and you continue along, compiling that one, testing it, and depending
on whether it is good or bad, you say "git bisect good" or "git bisect
bad", and ask for the next bisection.
===== END QUOTED SECTIONS =================

If there is a way to use 'git bisect' beginning with a "bad" version
but no "good" version, then it is an advanced usage that I have not
read about and do not understand how to use. As soon as you tell me
how to carry out the process, I will do so and report the results.


In the meantime, can you comment on the bisection I did last night?
I found something very interesting about the commit that first causes
the lockup (3def3d6d...), and the very next commit (1e934dda...) -- if
I checkout 1e94... and try to revert the changes made in 3def..., the
kernel freezes in spite of the revert.

Because of this, I would conclude that your patch for 2.6.27-rc3 was
doomed before you began, and we should look more carefully at the
commits from February instead of trying to revert at the 2.6.27 HEAD.

I am not a kernel developer, so my opinions are probably safe to ignore,
but I think we should be trying to extract information from my (faulty)
machines about what is happening differently between the "bad" commit
3def3d6d... and the "good" commit before it.


I am too tired to continue my experiments tonight -- it is summer
semester final exam time at the college where I tutor, and answer
questions about calculus 2, linear algebra, nursing math, etc., has
worn me right out -- but while I wait to find out the next step I
should do with the 2.6.27-rc3 patch, I am going to try to get some
useful info printed to the console before the kernel locks up back
in those Feb. revisions.


Thanks,
Dave W.[PATCH] x86: revert back to use request_resource to reserve kernel res | diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
| index 9af8907..feff5b7 100644
Signed-off-by: Yinghai Lu <yhlu.kernel@xxxxxxxxx> | --- a/arch/x86/kernel/e820.c
| +++ b/arch/x86/kernel/e820.c
--- | @@ -1267,7 +1267,7 @@ static inline const char *e820_type_to_string(int e820_type)
arch/x86/kernel/e820.c | 17 +++++++++++++++-- <
arch/x86/kernel/setup.c | 22 +++++++++++++++------- <
include/asm-x86/e820.h | 3 ++- <
3 files changed, 32 insertions(+), 10 deletions(-) <
<
Index: linux-2.6/arch/x86/kernel/e820.c <
=================================================================== <
--- linux-2.6.orig/arch/x86/kernel/e820.c <
+++ linux-2.6/arch/x86/kernel/e820.c <
@@ -1267,7 +1267,7 @@ static inline const char *e820_type_to_s <
/* /*
* Mark e820 reserved areas as busy for the resource manager. * Mark e820 reserved areas as busy for the resource manager.
*/ */
-void __init e820_reserve_resources(void) -void __init e820_reserve_resources(void)
+void __init e820_reserve_resources(struct resource **res_kernel, int nr_res_k) +void __init e820_reserve_resources(struct resource **res_kernel, int nr_res_k)
{ {
int i; int i;
struct resource *res; struct resource *res;
@@ -1287,7 +1287,20 @@ void __init e820_reserve_resources(void) @@ -1287,7 +1287,20 @@ void __init e820_reserve_resources(void)
res->end = end; res->end = end;

res->flags = IORESOURCE_MEM | IORESOURCE_BUSY; res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
- insert_resource(&iomem_resource, res); - insert_resource(&iomem_resource, res);
+ request_resource(&iomem_resource, res); + request_resource(&iomem_resource, res);
+ if (e820.map[i].type == E820_RAM) { + if (e820.map[i].type == E820_RAM) {
+ int j; + int j;
+ /* + /*
+ * We don't know which RAM region contains kernel data, + * We don't know which RAM region contains kernel data,
+ * so we try it repeatedly and let the resource manager + * so we try it repeatedly and let the resource manager
+ * test it. + * test it.
+ */ + */
+ for (j = 0; j < nr_res_k; j++) { + for (j = 0; j < nr_res_k; j++) {
+ if (!res_kernel[j]) + if (!res_kernel[j])
+ continue; + continue;
+ request_resource(res, res_kernel[j]); + request_resource(res, res_kernel[j]);
+ } + }
+ } + }
res++; res++;
} }

Index: linux-2.6/arch/x86/kernel/setup.c | diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
=================================================================== | index 68b48e3..f82c50b 100644
--- linux-2.6.orig/arch/x86/kernel/setup.c | --- a/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c | +++ b/arch/x86/kernel/setup.c
@@ -437,6 +437,7 @@ static void __init reserve_early_setup_d | @@ -437,6 +437,7 @@ static void __init reserve_early_setup_data(void)
* --------- Crashkernel reservation ------------------------------ * --------- Crashkernel reservation ------------------------------
*/ */

+static struct resource *crashk_res_ptr; +static struct resource *crashk_res_ptr;
#ifdef CONFIG_KEXEC #ifdef CONFIG_KEXEC

/** /**
@@ -517,7 +518,7 @@ static void __init reserve_crashkernel(v | @@ -517,7 +518,7 @@ static void __init reserve_crashkernel(void)

crashk_res.start = crash_base; crashk_res.start = crash_base;
crashk_res.end = crash_base + crash_size - 1; crashk_res.end = crash_base + crash_size - 1;
- insert_resource(&iomem_resource, &crashk_res); - insert_resource(&iomem_resource, &crashk_res);
+ crashk_res_ptr = &crashk_res; + crashk_res_ptr = &crashk_res;
} }
#else #else
static void __init reserve_crashkernel(void) static void __init reserve_crashkernel(void)
@@ -593,6 +594,9 @@ struct x86_quirks *x86_quirks __initdata | @@ -593,6 +594,9 @@ struct x86_quirks *x86_quirks __initdata = &default_x86_quirks;

void __init setup_arch(char **cmdline_p) void __init setup_arch(char **cmdline_p)
{ {
+ struct resource *res_kernel[4]; + struct resource *res_kernel[4];
+ int num_res; + int num_res;
+ +
#ifdef CONFIG_X86_32 #ifdef CONFIG_X86_32
memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data)); memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
visws_early_detect(); visws_early_detect();
@@ -699,11 +703,6 @@ void __init setup_arch(char **cmdline_p) @@ -699,11 +703,6 @@ void __init setup_arch(char **cmdline_p)
probe_roms(); probe_roms();
#endif #endif

- /* after parse_early_param, so could debug it */ - /* after parse_early_param, so could debug it */
- insert_resource(&iomem_resource, &code_resource); - insert_resource(&iomem_resource, &code_resource);
- insert_resource(&iomem_resource, &data_resource); - insert_resource(&iomem_resource, &data_resource);
- insert_resource(&iomem_resource, &bss_resource); - insert_resource(&iomem_resource, &bss_resource);
- -
if (efi_enabled) if (efi_enabled)
efi_init(); efi_init();

@@ -865,7 +864,16 @@ void __init setup_arch(char **cmdline_p) | @@ -863,7 +862,16 @@ void __init setup_arch(char **cmdline_p)

kvm_guest_init(); kvm_guest_init();

- e820_reserve_resources(); - e820_reserve_resources();
+ res_kernel[0] = &code_resource; + res_kernel[0] = &code_resource;
+ res_kernel[1] = &data_resource; + res_kernel[1] = &data_resource;
+ res_kernel[2] = &bss_resource; + res_kernel[2] = &bss_resource;
+ num_res = 3; + num_res = 3;
+ if (crashk_res_ptr) { + if (crashk_res_ptr) {
+ res_kernel[num_res] = crashk_res_ptr; + res_kernel[num_res] = crashk_res_ptr;
+ num_res++; + num_res++;
+ } + }
+ e820_reserve_resources(res_kernel, num_res); + e820_reserve_resources(res_kernel, num_res);
+ +
e820_mark_nosave_regions(max_low_pfn); e820_mark_nosave_regions(max_low_pfn);

#ifdef CONFIG_X86_32 #ifdef CONFIG_X86_32
Index: linux-2.6/include/asm-x86/e820.h | diff --git a/include/asm-x86/e820.h b/include/asm-x86/e820.h
=================================================================== | index 16a31e2..a574ad9 100644
--- linux-2.6.orig/include/asm-x86/e820.h | --- a/include/asm-x86/e820.h
+++ linux-2.6/include/asm-x86/e820.h | +++ b/include/asm-x86/e820.h
@@ -120,7 +120,8 @@ extern void e820_register_active_regions | @@ -119,7 +119,8 @@ extern void e820_register_active_regions(int nid, unsigned long start_pfn,
unsigned long end_pfn); unsigned long end_pfn);
extern u64 e820_hole_size(u64 start, u64 end); extern u64 e820_hole_size(u64 start, u64 end);
extern void finish_e820_parsing(void); extern void finish_e820_parsing(void);
-extern void e820_reserve_resources(void); -extern void e820_reserve_resources(void);
+struct resource; +struct resource;
+extern void e820_reserve_resources(struct resource **res, int num); +extern void e820_reserve_resources(struct resource **res, int num);
extern void setup_memory_map(void); extern void setup_memory_map(void);
extern char *default_machine_specific_memory_setup(void); extern char *default_machine_specific_memory_setup(void);
extern char *machine_specific_memory_setup(void); extern char *machine_specific_memory_setup(void);