* [PATCH] ext4: Fix a BUG in mb_mark_used during trim.
@ 2011-03-03 9:33 Tao Ma
2011-03-03 10:01 ` Lukas Czerner
0 siblings, 1 reply; 10+ messages in thread
From: Tao Ma @ 2011-03-03 9:33 UTC (permalink / raw)
To: linux-ext4; +Cc: Theodore Ts'o
From: Tao Ma <boyu.mt@taobao.com>
In a bs=4096 volume, if we call FITRIM with the following parameter as
fstrim_range(start = 102400, len = 134144000, minlen = 10240),
we will trigger a BUG_ON.
BUG_ON(start + len > (e4b->bd_sb->s_blocksize << 3));
Mar 4 00:55:52 boyu-tm kernel: ------------[ cut here ]------------
Mar 4 00:55:52 boyu-tm kernel: kernel BUG at fs/ext4/mballoc.c:1506!
Mar 4 01:21:09 boyu-tm kernel: Code: d4 00 00 00 00 49 89 fe 8b 56 0c 44 8b 7e 04 89 55 c4 48 8b 4f 28 89 d6 44 01 fe 48 63 d6 48 8b 41 18 48 c1 e0 03 48 39 c2 76 04 <0f> 0b eb fe 48 8b 55 b0 8b 47 34 3b 42 08 74 04 0f 0b eb fe 48
Mar 4 01:21:09 boyu-tm kernel: RIP [<ffffffffa053eb42>] mb_mark_used+0x47/0x26c [ext4]
Mar 4 01:21:09 boyu-tm kernel: RSP <ffff880121e45c38>
Mar 4 01:21:09 boyu-tm kernel: ---[ end trace 9f461696f6a9dcf2 ]---
The reason is that in ext4_trim_fs, the last_block is checked wrongly.
if (len >= EXT4_BLOCKS_PER_GROUP(sb))
len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
else
last_block = first_block + len;
So if len < EXT4_BLOCKS_PER_GROUP while first_block + len > EXT4_BLOCKS_PER_GROUP,
last_block will be set to a overflow value which exceeds EXT4_BLOCKS_PER_GROUP.
This patch fixes it and adjusts len accordingly.
Cc: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Tao Ma <boyu.mt@taobao.com>
---
fs/ext4/mballoc.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 29d7d17..ed63c3e 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4889,10 +4889,11 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
break;
}
- if (len >= EXT4_BLOCKS_PER_GROUP(sb))
- len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
+ if (first_block + len >= EXT4_BLOCKS_PER_GROUP(sb))
+ last_block = EXT4_BLOCKS_PER_GROUP(sb);
else
last_block = first_block + len;
+ len -= last_block - first_block;
if (e4b.bd_info->bb_free >= minlen) {
cnt = ext4_trim_all_free(sb, &e4b, first_block,
--
1.6.3.GIT
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ext4: Fix a BUG in mb_mark_used during trim.
2011-03-03 9:33 [PATCH] ext4: Fix a BUG in mb_mark_used during trim Tao Ma
@ 2011-03-03 10:01 ` Lukas Czerner
2011-03-03 10:13 ` Tao Ma
0 siblings, 1 reply; 10+ messages in thread
From: Lukas Czerner @ 2011-03-03 10:01 UTC (permalink / raw)
To: Tao Ma; +Cc: linux-ext4, Theodore Ts'o, lczerner
On Thu, 3 Mar 2011, Tao Ma wrote:
> From: Tao Ma <boyu.mt@taobao.com>
>
> In a bs=4096 volume, if we call FITRIM with the following parameter as
> fstrim_range(start = 102400, len = 134144000, minlen = 10240),
> we will trigger a BUG_ON.
> BUG_ON(start + len > (e4b->bd_sb->s_blocksize << 3));
>
> Mar 4 00:55:52 boyu-tm kernel: ------------[ cut here ]------------
> Mar 4 00:55:52 boyu-tm kernel: kernel BUG at fs/ext4/mballoc.c:1506!
> Mar 4 01:21:09 boyu-tm kernel: Code: d4 00 00 00 00 49 89 fe 8b 56 0c 44 8b 7e 04 89 55 c4 48 8b 4f 28 89 d6 44 01 fe 48 63 d6 48 8b 41 18 48 c1 e0 03 48 39 c2 76 04 <0f> 0b eb fe 48 8b 55 b0 8b 47 34 3b 42 08 74 04 0f 0b eb fe 48
> Mar 4 01:21:09 boyu-tm kernel: RIP [<ffffffffa053eb42>] mb_mark_used+0x47/0x26c [ext4]
> Mar 4 01:21:09 boyu-tm kernel: RSP <ffff880121e45c38>
> Mar 4 01:21:09 boyu-tm kernel: ---[ end trace 9f461696f6a9dcf2 ]---
>
> The reason is that in ext4_trim_fs, the last_block is checked wrongly.
> if (len >= EXT4_BLOCKS_PER_GROUP(sb))
> len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> else
> last_block = first_block + len;
>
> So if len < EXT4_BLOCKS_PER_GROUP while first_block + len > EXT4_BLOCKS_PER_GROUP,
> last_block will be set to a overflow value which exceeds EXT4_BLOCKS_PER_GROUP.
>
> This patch fixes it and adjusts len accordingly.
Oh, thanks for spotting this. One comment though.
>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
> ---
> fs/ext4/mballoc.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 29d7d17..ed63c3e 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4889,10 +4889,11 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
> break;
> }
>
> - if (len >= EXT4_BLOCKS_PER_GROUP(sb))
> - len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> + if (first_block + len >= EXT4_BLOCKS_PER_GROUP(sb))
> + last_block = EXT4_BLOCKS_PER_GROUP(sb);
> else
> last_block = first_block + len;
Since last_block would not change until the last group, can we simplify
the condition ?
if (first_block + len < EXT4_BLOCKS_PER_GROUP(sb))
last_block = first_block + len;
> + len -= last_block - first_block;
>
> if (e4b.bd_info->bb_free >= minlen) {
> cnt = ext4_trim_all_free(sb, &e4b, first_block,
>
Thanks!
-Lukas
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ext4: Fix a BUG in mb_mark_used during trim.
2011-03-03 10:01 ` Lukas Czerner
@ 2011-03-03 10:13 ` Tao Ma
2011-03-03 10:17 ` Lukas Czerner
0 siblings, 1 reply; 10+ messages in thread
From: Tao Ma @ 2011-03-03 10:13 UTC (permalink / raw)
To: Lukas Czerner; +Cc: linux-ext4, Theodore Ts'o
On 03/03/2011 06:01 PM, Lukas Czerner wrote:
> On Thu, 3 Mar 2011, Tao Ma wrote:
>
>
>> From: Tao Ma<boyu.mt@taobao.com>
>>
>> In a bs=4096 volume, if we call FITRIM with the following parameter as
>> fstrim_range(start = 102400, len = 134144000, minlen = 10240),
>> we will trigger a BUG_ON.
>> BUG_ON(start + len> (e4b->bd_sb->s_blocksize<< 3));
>>
>> Mar 4 00:55:52 boyu-tm kernel: ------------[ cut here ]------------
>> Mar 4 00:55:52 boyu-tm kernel: kernel BUG at fs/ext4/mballoc.c:1506!
>> Mar 4 01:21:09 boyu-tm kernel: Code: d4 00 00 00 00 49 89 fe 8b 56 0c 44 8b 7e 04 89 55 c4 48 8b 4f 28 89 d6 44 01 fe 48 63 d6 48 8b 41 18 48 c1 e0 03 48 39 c2 76 04<0f> 0b eb fe 48 8b 55 b0 8b 47 34 3b 42 08 74 04 0f 0b eb fe 48
>> Mar 4 01:21:09 boyu-tm kernel: RIP [<ffffffffa053eb42>] mb_mark_used+0x47/0x26c [ext4]
>> Mar 4 01:21:09 boyu-tm kernel: RSP<ffff880121e45c38>
>> Mar 4 01:21:09 boyu-tm kernel: ---[ end trace 9f461696f6a9dcf2 ]---
>>
>> The reason is that in ext4_trim_fs, the last_block is checked wrongly.
>> if (len>= EXT4_BLOCKS_PER_GROUP(sb))
>> len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
>> else
>> last_block = first_block + len;
>>
>> So if len< EXT4_BLOCKS_PER_GROUP while first_block + len> EXT4_BLOCKS_PER_GROUP,
>> last_block will be set to a overflow value which exceeds EXT4_BLOCKS_PER_GROUP.
>>
>> This patch fixes it and adjusts len accordingly.
>>
> Oh, thanks for spotting this. One comment though.
>
>
>> Cc: "Theodore Ts'o"<tytso@mit.edu>
>> Signed-off-by: Tao Ma<boyu.mt@taobao.com>
>> ---
>> fs/ext4/mballoc.c | 5 +++--
>> 1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index 29d7d17..ed63c3e 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -4889,10 +4889,11 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
>> break;
>> }
>>
>> - if (len>= EXT4_BLOCKS_PER_GROUP(sb))
>> - len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
>> + if (first_block + len>= EXT4_BLOCKS_PER_GROUP(sb))
>> + last_block = EXT4_BLOCKS_PER_GROUP(sb);
>> else
>> last_block = first_block + len;
>>
> Since last_block would not change until the last group, can we simplify
> the condition ?
>
> if (first_block + len< EXT4_BLOCKS_PER_GROUP(sb))
> last_block = first_block + len;
>
yes, it should work. But it is a bit tricky. And I guess we need a
comment here
to describe the situation. Otherwise, the people who read the code would
consider
why last_block won't be reset in case first _block + len >=
EXT4_BLOCKS_PER_GROUP
every time. Agree?
>
>> + len -= last_block - first_block;
>>
>> if (e4b.bd_info->bb_free>= minlen) {
>> cnt = ext4_trim_all_free(sb,&e4b, first_block,
>>
>>
> Thanks!
> -Lukas
>
Regards,
Tao
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ext4: Fix a BUG in mb_mark_used during trim.
2011-03-03 10:13 ` Tao Ma
@ 2011-03-03 10:17 ` Lukas Czerner
2011-03-03 13:59 ` [PATCH v2] " Tao Ma
0 siblings, 1 reply; 10+ messages in thread
From: Lukas Czerner @ 2011-03-03 10:17 UTC (permalink / raw)
To: Tao Ma; +Cc: Lukas Czerner, linux-ext4, Theodore Ts'o
On Thu, 3 Mar 2011, Tao Ma wrote:
> On 03/03/2011 06:01 PM, Lukas Czerner wrote:
> > On Thu, 3 Mar 2011, Tao Ma wrote:
> >
> >
> > > From: Tao Ma<boyu.mt@taobao.com>
> > >
> > > In a bs=4096 volume, if we call FITRIM with the following parameter as
> > > fstrim_range(start = 102400, len = 134144000, minlen = 10240),
> > > we will trigger a BUG_ON.
> > > BUG_ON(start + len> (e4b->bd_sb->s_blocksize<< 3));
> > >
> > > Mar 4 00:55:52 boyu-tm kernel: ------------[ cut here ]------------
> > > Mar 4 00:55:52 boyu-tm kernel: kernel BUG at fs/ext4/mballoc.c:1506!
> > > Mar 4 01:21:09 boyu-tm kernel: Code: d4 00 00 00 00 49 89 fe 8b 56 0c 44
> > > 8b 7e 04 89 55 c4 48 8b 4f 28 89 d6 44 01 fe 48 63 d6 48 8b 41 18 48 c1 e0
> > > 03 48 39 c2 76 04<0f> 0b eb fe 48 8b 55 b0 8b 47 34 3b 42 08 74 04 0f 0b
> > > eb fe 48
> > > Mar 4 01:21:09 boyu-tm kernel: RIP [<ffffffffa053eb42>]
> > > mb_mark_used+0x47/0x26c [ext4]
> > > Mar 4 01:21:09 boyu-tm kernel: RSP<ffff880121e45c38>
> > > Mar 4 01:21:09 boyu-tm kernel: ---[ end trace 9f461696f6a9dcf2 ]---
> > >
> > > The reason is that in ext4_trim_fs, the last_block is checked wrongly.
> > > if (len>= EXT4_BLOCKS_PER_GROUP(sb))
> > > len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> > > else
> > > last_block = first_block + len;
> > >
> > > So if len< EXT4_BLOCKS_PER_GROUP while first_block + len>
> > > EXT4_BLOCKS_PER_GROUP,
> > > last_block will be set to a overflow value which exceeds
> > > EXT4_BLOCKS_PER_GROUP.
> > >
> > > This patch fixes it and adjusts len accordingly.
> > >
> > Oh, thanks for spotting this. One comment though.
> >
> >
> > > Cc: "Theodore Ts'o"<tytso@mit.edu>
> > > Signed-off-by: Tao Ma<boyu.mt@taobao.com>
> > > ---
> > > fs/ext4/mballoc.c | 5 +++--
> > > 1 files changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> > > index 29d7d17..ed63c3e 100644
> > > --- a/fs/ext4/mballoc.c
> > > +++ b/fs/ext4/mballoc.c
> > > @@ -4889,10 +4889,11 @@ int ext4_trim_fs(struct super_block *sb, struct
> > > fstrim_range *range)
> > > break;
> > > }
> > >
> > > - if (len>= EXT4_BLOCKS_PER_GROUP(sb))
> > > - len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> > > + if (first_block + len>= EXT4_BLOCKS_PER_GROUP(sb))
> > > + last_block = EXT4_BLOCKS_PER_GROUP(sb);
> > > else
> > > last_block = first_block + len;
> > >
> > Since last_block would not change until the last group, can we simplify
> > the condition ?
> >
> > if (first_block + len< EXT4_BLOCKS_PER_GROUP(sb))
> > last_block = first_block + len;
> >
> yes, it should work. But it is a bit tricky. And I guess we need a comment
> here
> to describe the situation. Otherwise, the people who read the code would
> consider
> why last_block won't be reset in case first _block + len >=
> EXT4_BLOCKS_PER_GROUP
> every time. Agree?
Agree, comment would be useful.
Thanks!
-Lukas
> >
> > > + len -= last_block - first_block;
> > >
> > > if (e4b.bd_info->bb_free>= minlen) {
> > > cnt = ext4_trim_all_free(sb,&e4b, first_block,
> > >
> > >
> > Thanks!
> > -Lukas
> >
> Regards,
> Tao
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] ext4: Fix a BUG in mb_mark_used during trim.
2011-03-03 10:17 ` Lukas Czerner
@ 2011-03-03 13:59 ` Tao Ma
2011-03-03 14:20 ` Lukas Czerner
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Tao Ma @ 2011-03-03 13:59 UTC (permalink / raw)
To: linux-ext4; +Cc: Lukas Czerner, Theodore Ts'o
From: Tao Ma <boyu.mt@taobao.com>
In a bs=4096 volume, if we call FITRIM with the following parameter as
fstrim_range(start = 102400, len = 134144000, minlen = 10240),
we will trigger a BUG_ON.
BUG_ON(start + len > (e4b->bd_sb->s_blocksize << 3));
Mar 4 00:55:52 boyu-tm kernel: ------------[ cut here ]------------
Mar 4 00:55:52 boyu-tm kernel: kernel BUG at fs/ext4/mballoc.c:1506!
Mar 4 01:21:09 boyu-tm kernel: Code: d4 00 00 00 00 49 89 fe 8b 56 0c 44 8b 7e 04 89 55 c4 48 8b 4f 28 89 d6 44 01 fe 48 63 d6 48 8b 41 18 48 c1 e0 03 48 39 c2 76 04 <0f> 0b eb fe 48 8b 55 b0 8b 47 34 3b 42 08 74 04 0f 0b eb fe 48
Mar 4 01:21:09 boyu-tm kernel: RIP [<ffffffffa053eb42>] mb_mark_used+0x47/0x26c [ext4]
Mar 4 01:21:09 boyu-tm kernel: RSP <ffff880121e45c38>
Mar 4 01:21:09 boyu-tm kernel: ---[ end trace 9f461696f6a9dcf2 ]---
The reason is that in ext4_trim_fs, the last_block is checked wrongly.
if (len >= EXT4_BLOCKS_PER_GROUP(sb))
len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
else
last_block = first_block + len;
So if len < EXT4_BLOCKS_PER_GROUP while first_block + len > EXT4_BLOCKS_PER_GROUP,
last_block will be set to a overflow value which exceeds EXT4_BLOCKS_PER_GROUP.
This patch fixes it and adjusts len accordingly.
Cc: Lukas Czerner <lczerner@redhat.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Tao Ma <boyu.mt@taobao.com>
---
fs/ext4/mballoc.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 29d7d17..290b36c 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4889,10 +4889,15 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
break;
}
- if (len >= EXT4_BLOCKS_PER_GROUP(sb))
- len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
- else
+ /*
+ * For all the groups except the last one, last block will
+ * always be EXT4_BLOCKS_PER_GROUP(sb), so we only need to
+ * change it for the last group in which case start +
+ * len < EXT4_BLOCKS_PER_GROUP(sb).
+ */
+ if (first_block + len < EXT4_BLOCKS_PER_GROUP(sb))
last_block = first_block + len;
+ len -= last_block - first_block;
if (e4b.bd_info->bb_free >= minlen) {
cnt = ext4_trim_all_free(sb, &e4b, first_block,
--
1.6.3.GIT
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ext4: Fix a BUG in mb_mark_used during trim.
2011-03-03 13:59 ` [PATCH v2] " Tao Ma
@ 2011-03-03 14:20 ` Lukas Czerner
[not found] ` <20110303141109.GB16191@infradead.org>
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Lukas Czerner @ 2011-03-03 14:20 UTC (permalink / raw)
To: Tao Ma; +Cc: linux-ext4, Lukas Czerner, Theodore Ts'o
On Thu, 3 Mar 2011, Tao Ma wrote:
> From: Tao Ma <boyu.mt@taobao.com>
>
> In a bs=4096 volume, if we call FITRIM with the following parameter as
> fstrim_range(start = 102400, len = 134144000, minlen = 10240),
> we will trigger a BUG_ON.
> BUG_ON(start + len > (e4b->bd_sb->s_blocksize << 3));
>
> Mar 4 00:55:52 boyu-tm kernel: ------------[ cut here ]------------
> Mar 4 00:55:52 boyu-tm kernel: kernel BUG at fs/ext4/mballoc.c:1506!
> Mar 4 01:21:09 boyu-tm kernel: Code: d4 00 00 00 00 49 89 fe 8b 56 0c 44 8b 7e 04 89 55 c4 48 8b 4f 28 89 d6 44 01 fe 48 63 d6 48 8b 41 18 48 c1 e0 03 48 39 c2 76 04 <0f> 0b eb fe 48 8b 55 b0 8b 47 34 3b 42 08 74 04 0f 0b eb fe 48
> Mar 4 01:21:09 boyu-tm kernel: RIP [<ffffffffa053eb42>] mb_mark_used+0x47/0x26c [ext4]
> Mar 4 01:21:09 boyu-tm kernel: RSP <ffff880121e45c38>
> Mar 4 01:21:09 boyu-tm kernel: ---[ end trace 9f461696f6a9dcf2 ]---
>
> The reason is that in ext4_trim_fs, the last_block is checked wrongly.
> if (len >= EXT4_BLOCKS_PER_GROUP(sb))
> len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> else
> last_block = first_block + len;
>
> So if len < EXT4_BLOCKS_PER_GROUP while first_block + len > EXT4_BLOCKS_PER_GROUP,
> last_block will be set to a overflow value which exceeds EXT4_BLOCKS_PER_GROUP.
>
> This patch fixes it and adjusts len accordingly.
>
> Cc: Lukas Czerner <lczerner@redhat.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
> ---
> fs/ext4/mballoc.c | 11 ++++++++---
> 1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 29d7d17..290b36c 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4889,10 +4889,15 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
> break;
> }
>
> - if (len >= EXT4_BLOCKS_PER_GROUP(sb))
> - len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> - else
> + /*
> + * For all the groups except the last one, last block will
> + * always be EXT4_BLOCKS_PER_GROUP(sb), so we only need to
> + * change it for the last group in which case start +
> + * len < EXT4_BLOCKS_PER_GROUP(sb).
> + */
> + if (first_block + len < EXT4_BLOCKS_PER_GROUP(sb))
> last_block = first_block + len;
> + len -= last_block - first_block;
>
> if (e4b.bd_info->bb_free >= minlen) {
> cnt = ext4_trim_all_free(sb, &e4b, first_block,
>
Looks good.
Thanks!
-Lukas
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ext4: Fix a BUG in mb_mark_used during trim.
[not found] ` <20110303141109.GB16191@infradead.org>
@ 2011-03-03 15:33 ` Tao Ma
2011-03-03 15:38 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Tao Ma @ 2011-03-03 15:33 UTC (permalink / raw)
To: Christoph Hellwig, linux-ext4
Hi Christoph,
On 03/03/2011 10:11 PM, Christoph Hellwig wrote:
> On Thu, Mar 03, 2011 at 09:59:34PM +0800, Tao Ma wrote:
>> From: Tao Ma<boyu.mt@taobao.com>
>>
>> In a bs=4096 volume, if we call FITRIM with the following parameter as
>> fstrim_range(start = 102400, len = 134144000, minlen = 10240),
> Can you add a xfstests regression test for this particular invocation?
> We already have Lukas' fstrim.c tester in the tree, so it could be a
> simple wrapper around it.
Actually these numbers are related to the block size of a ext4 volume.
To be more specific, with the above number, we have
first_block + block_len > EXT4_BLOCKS_PER_GROUP while
block_len < EXT4_BLOCKS_PER_GROUP.
So do you think it is generic enough for a test case in xfstests since
now this
tool kit is also used to test xfs and btrfs?
Regards,
Tao
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ext4: Fix a BUG in mb_mark_used during trim.
2011-03-03 15:33 ` Tao Ma
@ 2011-03-03 15:38 ` Christoph Hellwig
0 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2011-03-03 15:38 UTC (permalink / raw)
To: Tao Ma; +Cc: linux-ext4
On Thu, Mar 03, 2011 at 11:33:35PM +0800, Tao Ma wrote:
> Actually these numbers are related to the block size of a ext4 volume.
> To be more specific, with the above number, we have
> first_block + block_len > EXT4_BLOCKS_PER_GROUP while
> block_len < EXT4_BLOCKS_PER_GROUP.
> So do you think it is generic enough for a test case in xfstests
> since now this
> tool kit is also used to test xfs and btrfs?
xfstests also allows filesystem-specific tests. Most of the specific
tests are for xfs, but there are few for udf right now as well.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ext4: Fix a kernel panic in mb_mark_used during trim.
2011-03-03 13:59 ` [PATCH v2] " Tao Ma
2011-03-03 14:20 ` Lukas Czerner
[not found] ` <20110303141109.GB16191@infradead.org>
@ 2011-03-16 15:20 ` Tao Ma
2011-03-23 19:47 ` [PATCH v2] ext4: Fix a BUG " Ted Ts'o
3 siblings, 0 replies; 10+ messages in thread
From: Tao Ma @ 2011-03-16 15:20 UTC (permalink / raw)
To: linux-ext4; +Cc: Lukas Czerner, Theodore Ts'o
Hi Ted,
This is a fix for kernel panic in ext4. So any comment for it?
Regards,
Tao
On 03/03/2011 09:59 PM, Tao Ma wrote:
> From: Tao Ma <boyu.mt@taobao.com>
>
> In a bs=4096 volume, if we call FITRIM with the following parameter as
> fstrim_range(start = 102400, len = 134144000, minlen = 10240),
> we will trigger a BUG_ON.
> BUG_ON(start + len > (e4b->bd_sb->s_blocksize << 3));
>
> Mar 4 00:55:52 boyu-tm kernel: ------------[ cut here ]------------
> Mar 4 00:55:52 boyu-tm kernel: kernel BUG at fs/ext4/mballoc.c:1506!
> Mar 4 01:21:09 boyu-tm kernel: Code: d4 00 00 00 00 49 89 fe 8b 56 0c 44 8b 7e 04 89 55 c4 48 8b 4f 28 89 d6 44 01 fe 48 63 d6 48 8b 41 18 48 c1 e0 03 48 39 c2 76 04 <0f> 0b eb fe 48 8b 55 b0 8b 47 34 3b 42 08 74 04 0f 0b eb fe 48
> Mar 4 01:21:09 boyu-tm kernel: RIP [<ffffffffa053eb42>] mb_mark_used+0x47/0x26c [ext4]
> Mar 4 01:21:09 boyu-tm kernel: RSP <ffff880121e45c38>
> Mar 4 01:21:09 boyu-tm kernel: ---[ end trace 9f461696f6a9dcf2 ]---
>
> The reason is that in ext4_trim_fs, the last_block is checked wrongly.
> if (len >= EXT4_BLOCKS_PER_GROUP(sb))
> len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> else
> last_block = first_block + len;
>
> So if len < EXT4_BLOCKS_PER_GROUP while first_block + len > EXT4_BLOCKS_PER_GROUP,
> last_block will be set to a overflow value which exceeds EXT4_BLOCKS_PER_GROUP.
>
> This patch fixes it and adjusts len accordingly.
>
> Cc: Lukas Czerner <lczerner@redhat.com>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
> ---
> fs/ext4/mballoc.c | 11 ++++++++---
> 1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 29d7d17..290b36c 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4889,10 +4889,15 @@ int ext4_trim_fs(struct super_block *sb, struct fstrim_range *range)
> break;
> }
>
> - if (len >= EXT4_BLOCKS_PER_GROUP(sb))
> - len -= (EXT4_BLOCKS_PER_GROUP(sb) - first_block);
> - else
> + /*
> + * For all the groups except the last one, last block will
> + * always be EXT4_BLOCKS_PER_GROUP(sb), so we only need to
> + * change it for the last group in which case start +
> + * len < EXT4_BLOCKS_PER_GROUP(sb).
> + */
> + if (first_block + len < EXT4_BLOCKS_PER_GROUP(sb))
> last_block = first_block + len;
> + len -= last_block - first_block;
>
> if (e4b.bd_info->bb_free >= minlen) {
> cnt = ext4_trim_all_free(sb, &e4b, first_block,
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ext4: Fix a BUG in mb_mark_used during trim.
2011-03-03 13:59 ` [PATCH v2] " Tao Ma
` (2 preceding siblings ...)
2011-03-16 15:20 ` [PATCH v2] ext4: Fix a kernel panic " Tao Ma
@ 2011-03-23 19:47 ` Ted Ts'o
3 siblings, 0 replies; 10+ messages in thread
From: Ted Ts'o @ 2011-03-23 19:47 UTC (permalink / raw)
To: Tao Ma; +Cc: linux-ext4, Lukas Czerner
Thanks, added to the ext4 patch queue. My apologies for the delay.
- Ted
On Thu, Mar 03, 2011 at 09:59:34PM +0800, Tao Ma wrote:
> From: Tao Ma <boyu.mt@taobao.com>
>
> In a bs=4096 volume, if we call FITRIM with the following parameter as
> fstrim_range(start = 102400, len = 134144000, minlen = 10240),
> we will trigger a BUG_ON.
> BUG_ON(start + len > (e4b->bd_sb->s_blocksize << 3));
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-03-23 19:47 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-03 9:33 [PATCH] ext4: Fix a BUG in mb_mark_used during trim Tao Ma
2011-03-03 10:01 ` Lukas Czerner
2011-03-03 10:13 ` Tao Ma
2011-03-03 10:17 ` Lukas Czerner
2011-03-03 13:59 ` [PATCH v2] " Tao Ma
2011-03-03 14:20 ` Lukas Czerner
[not found] ` <20110303141109.GB16191@infradead.org>
2011-03-03 15:33 ` Tao Ma
2011-03-03 15:38 ` Christoph Hellwig
2011-03-16 15:20 ` [PATCH v2] ext4: Fix a kernel panic " Tao Ma
2011-03-23 19:47 ` [PATCH v2] ext4: Fix a BUG " Ted Ts'o
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).