2007-06-26 09:47:42

by Valerie Clement

[permalink] [raw]
Subject: Re: performance testing

Valerie Clement wrote:
> Alex Tomas wrote:
>> Jean noel Cordenner wrote:
>>> The last patch queue concerns the 2.6.22-rc4 kernel, so we took the
>>> previous ext4 patch queue including the modifications suggested by
>>> dmitriy:
>>> http://article.gmane.org/gmane.comp.file-systems.ext4/2291
>>> This solve the oops problem but after a while, the system hangs. We are
>>> still trying to find where the bug is.
>>> When we remove all the patches until booked-page-flag.patch in the
>>> series, the system still hangs. When using another filesystem, or
>>> whithout any patches it works.
>>
>> any details? backtraces? dmesg?
>
> We are trying to get some traces, but with 2.6.22-rc5 and 2.6.22-rc6
> kernels, the serial console isn't working on our systems (x86_64), the
> magic SysRq keys either. Strange...
>
> When the system hangs, no messages are logged.
> It seems that the hangs only occur with ext4 FS when applying the
> patches of the current git tree.
> We are trying now to find which patch is faulty.
>
> Val?rie
>

It seems that the faulty patch is "ext4-journal_chksum-2.6.20.patch".
Looking at the patch, I think the following change is not correct:

@@ -116,21 +120,36 @@ static int journal_write_commit_record(j

bh = jh2bh(descriptor);

- /* AKPM: buglet - add `i' to tmp! */
for (i = 0; i < bh->b_size; i += 512) {
- journal_header_t *tmp = (journal_header_t*)bh->b_data;
+ struct commit_header *tmp = (struct commit_header*)bh->b_data +
+ i;

Shouldn't it be :

struct commit_header *tmp = (struct commit_header*)(bh->b_data + i);

Val?rie


2007-06-26 10:36:35

by Girish Shilamkar

[permalink] [raw]
Subject: Re: performance testing

On Tue, 2007-06-26 at 11:48 +0200, Valerie Clement wrote:
> Shouldn't it be :
>
> struct commit_header *tmp = (struct commit_header*)(bh->b_data + i);
>
Ohhh, yes you are right. This is the correct thing to do.
This patch which is been used has some endian-ness bugs. I had sent an
updated patch for 2.6.22-rc5 on
19 June. I think we should be using the updated patch else it may break
on big-endian machines.
Even the updated patch has this bug i.e "struct commit_header *tmp
=...."

Thanks,
Girish.

2007-06-26 11:02:08

by Valerie Clement

[permalink] [raw]
Subject: Re: performance testing

Girish Shilamkar wrote:
> On Tue, 2007-06-26 at 11:48 +0200, Valerie Clement wrote:
>> Shouldn't it be :
>>
>> struct commit_header *tmp = (struct commit_header*)(bh->b_data + i);
>>
> Ohhh, yes you are right. This is the correct thing to do.
> This patch which is been used has some endian-ness bugs. I had sent an
> updated patch for 2.6.22-rc5 on
> 19 June. I think we should be using the updated patch else it may break
> on big-endian machines.
> Even the updated patch has this bug i.e "struct commit_header *tmp
> =...."
>
> Thanks,
> Girish.
>
Hi Girish,
I made the change on my system and tested it. Good news, everything
seems to work well now. Could you please make and post the patch?
Thanks,
Val?rie