A simple dd oopses the kernel (2.6.24-rc7 with the latest patch queue):
dd if=/dev/zero of=/mnt/test/foo bs=1M count=8096
EXT4-fs: mballoc enabled
------------[ cut here ]------------
kernel BUG at fs/ext4/mballoc.c:3148!
The BUG_ON is:
BUG_ON(size <= 0 || size >= EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
where the value of "size" is 4293920768.
This is due to the overflow of the variable "start" in the
ext4_mb_normalize_request() function.
The patch below fixes it.
Signed-off-by: Valerie Clement <[email protected]>
---
mballoc.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
Index: linux-2.6.24-rc7/fs/ext4/mballoc.c
===================================================================
--- linux-2.6.24-rc7.orig/fs/ext4/mballoc.c 2008-01-16 19:22:45.000000000 +0100
+++ linux-2.6.24-rc7/fs/ext4/mballoc.c 2008-01-16 19:25:04.000000000 +0100
@@ -2990,6 +2990,7 @@ static void ext4_mb_normalize_request(st
struct list_head *cur;
loff_t size, orig_size;
ext4_lblk_t start, orig_start;
+ ext4_fsblk_t pstart;
struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
/* do normalize only data requests, metadata requests
@@ -3029,7 +3030,7 @@ static void ext4_mb_normalize_request(st
/* first, try to predict filesize */
/* XXX: should this table be tunable? */
- start = 0;
+ pstart = 0;
if (size <= 16 * 1024) {
size = 16 * 1024;
} else if (size <= 32 * 1024) {
@@ -3045,25 +3046,25 @@ static void ext4_mb_normalize_request(st
} else if (size <= 1024 * 1024) {
size = 1024 * 1024;
} else if (NRL_CHECK_SIZE(size, 4 * 1024 * 1024, max, bsbits)) {
- start = ac->ac_o_ex.fe_logical << bsbits;
- start = (start / (1024 * 1024)) * (1024 * 1024);
+ pstart = ac->ac_o_ex.fe_logical << bsbits;
+ pstart = (pstart / (1024 * 1024)) * (1024 * 1024);
size = 1024 * 1024;
} else if (NRL_CHECK_SIZE(size, 8 * 1024 * 1024, max, bsbits)) {
- start = ac->ac_o_ex.fe_logical << bsbits;
- start = (start / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
+ pstart = ac->ac_o_ex.fe_logical << bsbits;
+ pstart = (pstart / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
size = 4 * 1024 * 1024;
} else if(NRL_CHECK_SIZE(ac->ac_o_ex.fe_len,(8<<20)>>bsbits,max,bsbits)){
- start = ac->ac_o_ex.fe_logical;
- start = start << bsbits;
- start = (start / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
+ pstart = ac->ac_o_ex.fe_logical;
+ pstart = pstart << bsbits;
+ pstart = (pstart / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
size = 8 * 1024 * 1024;
} else {
- start = ac->ac_o_ex.fe_logical;
- start = start << bsbits;
+ pstart = ac->ac_o_ex.fe_logical;
+ pstart = pstart << bsbits;
size = ac->ac_o_ex.fe_len << bsbits;
}
orig_size = size = size >> bsbits;
- orig_start = start = start >> bsbits;
+ orig_start = start = pstart >> bsbits;
/* don't cover already allocated blocks in selected range */
if (ar->pleft && start <= ar->lleft) {
On Wed, 2008-01-16 at 20:11 +0100, Valerie Clement wrote:
> A simple dd oopses the kernel (2.6.24-rc7 with the latest patch queue):
> dd if=/dev/zero of=/mnt/test/foo bs=1M count=8096
>
> EXT4-fs: mballoc enabled
> ------------[ cut here ]------------
> kernel BUG at fs/ext4/mballoc.c:3148!
>
> The BUG_ON is:
> BUG_ON(size <= 0 || size >= EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
>
> where the value of "size" is 4293920768.
>
> This is due to the overflow of the variable "start" in the
> ext4_mb_normalize_request() function.
> The patch below fixes it.
>
Thanks!
> Signed-off-by: Valerie Clement <[email protected]>
> ---
>
> mballoc.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
>
> Index: linux-2.6.24-rc7/fs/ext4/mballoc.c
> ===================================================================
> --- linux-2.6.24-rc7.orig/fs/ext4/mballoc.c 2008-01-16 19:22:45.000000000 +0100
> +++ linux-2.6.24-rc7/fs/ext4/mballoc.c 2008-01-16 19:25:04.000000000 +0100
> @@ -2990,6 +2990,7 @@ static void ext4_mb_normalize_request(st
> struct list_head *cur;
> loff_t size, orig_size;
> ext4_lblk_t start, orig_start;
> + ext4_fsblk_t pstart;
ext4_fsblk_t is used for fs physical block number, here I think pstart
is pointing to some logical block location..
> struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
>
> /* do normalize only data requests, metadata requests
> @@ -3029,7 +3030,7 @@ static void ext4_mb_normalize_request(st
>
> /* first, try to predict filesize */
> /* XXX: should this table be tunable? */
> - start = 0;
> + pstart = 0;
> if (size <= 16 * 1024) {
> size = 16 * 1024;
> } else if (size <= 32 * 1024) {
> @@ -3045,25 +3046,25 @@ static void ext4_mb_normalize_request(st
> } else if (size <= 1024 * 1024) {
> size = 1024 * 1024;
> } else if (NRL_CHECK_SIZE(size, 4 * 1024 * 1024, max, bsbits)) {
> - start = ac->ac_o_ex.fe_logical << bsbits;
> - start = (start / (1024 * 1024)) * (1024 * 1024);
> + pstart = ac->ac_o_ex.fe_logical << bsbits;
> + pstart = (pstart / (1024 * 1024)) * (1024 * 1024);
How about using shift...
- start = ac->ac_o_ex.fe_logical << bsbits;
- start = (start / (1024 * 1024)) * (1024 * 1024);
+ start = (ac->ac_o_ex.fe_logical >> (20-bsbits)) << 20;
That would be more efficient and should fix the overflow issue
> size = 1024 * 1024;
> } else if (NRL_CHECK_SIZE(size, 8 * 1024 * 1024, max, bsbits)) {
> - start = ac->ac_o_ex.fe_logical << bsbits;
> - start = (start / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
> + pstart = ac->ac_o_ex.fe_logical << bsbits;
> + pstart = (pstart / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
+ start = (ac->ac_o_ex.fe_logical >> (22-bsbits)) << 22;
> size = 4 * 1024 * 1024;
> } else if(NRL_CHECK_SIZE(ac->ac_o_ex.fe_len,(8<<20)>>bsbits,max,bsbits)){
> - start = ac->ac_o_ex.fe_logical;
> - start = start << bsbits;
> - start = (start / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
> + pstart = ac->ac_o_ex.fe_logical;
> + pstart = pstart << bsbits;
> + pstart = (pstart / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
+ start = (ac->ac_o_ex.fe_logical >> (23-bsbits)) << 23;
> size = 8 * 1024 * 1024;
> } else {
> - start = ac->ac_o_ex.fe_logical;
> - start = start << bsbits;
> + pstart = ac->ac_o_ex.fe_logical;
> + pstart = pstart << bsbits;
> size = ac->ac_o_ex.fe_len << bsbits;
> }
> orig_size = size = size >> bsbits;
> - orig_start = start = start >> bsbits;
> + orig_start = start = pstart >> bsbits;
>
> /* don't cover already allocated blocks in selected range */
> if (ar->pleft && start <= ar->lleft) {
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 16, 2008 at 10:48:27AM -0800, Mingming Cao wrote:
> On Wed, 2008-01-16 at 20:11 +0100, Valerie Clement wrote:
> > A simple dd oopses the kernel (2.6.24-rc7 with the latest patch queue):
> > dd if=/dev/zero of=/mnt/test/foo bs=1M count=8096
> >
> > EXT4-fs: mballoc enabled
> > ------------[ cut here ]------------
> > kernel BUG at fs/ext4/mballoc.c:3148!
> >
> > The BUG_ON is:
> > BUG_ON(size <= 0 || size >= EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
> >
> > where the value of "size" is 4293920768.
> >
> > This is due to the overflow of the variable "start" in the
> > ext4_mb_normalize_request() function.
> > The patch below fixes it.
> >
> Thanks!
>
> > Signed-off-by: Valerie Clement <[email protected]>
> > ---
> >
> > mballoc.c | 23 ++++++++++++-----------
> > 1 file changed, 12 insertions(+), 11 deletions(-)
> >
> >
> > Index: linux-2.6.24-rc7/fs/ext4/mballoc.c
> > ===================================================================
> > --- linux-2.6.24-rc7.orig/fs/ext4/mballoc.c 2008-01-16 19:22:45.000000000 +0100
> > +++ linux-2.6.24-rc7/fs/ext4/mballoc.c 2008-01-16 19:25:04.000000000 +0100
> > @@ -2990,6 +2990,7 @@ static void ext4_mb_normalize_request(st
> > struct list_head *cur;
> > loff_t size, orig_size;
> > ext4_lblk_t start, orig_start;
> > + ext4_fsblk_t pstart;
>
> ext4_fsblk_t is used for fs physical block number, here I think pstart
> is pointing to some logical block location..
>
> > struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
> >
> > /* do normalize only data requests, metadata requests
> > @@ -3029,7 +3030,7 @@ static void ext4_mb_normalize_request(st
> >
> > /* first, try to predict filesize */
> > /* XXX: should this table be tunable? */
> > - start = 0;
> > + pstart = 0;
> > if (size <= 16 * 1024) {
> > size = 16 * 1024;
> > } else if (size <= 32 * 1024) {
> > @@ -3045,25 +3046,25 @@ static void ext4_mb_normalize_request(st
> > } else if (size <= 1024 * 1024) {
> > size = 1024 * 1024;
> > } else if (NRL_CHECK_SIZE(size, 4 * 1024 * 1024, max, bsbits)) {
> > - start = ac->ac_o_ex.fe_logical << bsbits;
> > - start = (start / (1024 * 1024)) * (1024 * 1024);
> > + pstart = ac->ac_o_ex.fe_logical << bsbits;
> > + pstart = (pstart / (1024 * 1024)) * (1024 * 1024);
>
> How about using shift...
>
> - start = ac->ac_o_ex.fe_logical << bsbits;
> - start = (start / (1024 * 1024)) * (1024 * 1024);
> + start = (ac->ac_o_ex.fe_logical >> (20-bsbits)) << 20;
>
> That would be more efficient and should fix the overflow issue
>
> > size = 1024 * 1024;
> > } else if (NRL_CHECK_SIZE(size, 8 * 1024 * 1024, max, bsbits)) {
> > - start = ac->ac_o_ex.fe_logical << bsbits;
> > - start = (start / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
> > + pstart = ac->ac_o_ex.fe_logical << bsbits;
> > + pstart = (pstart / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
>
> + start = (ac->ac_o_ex.fe_logical >> (22-bsbits)) << 22;
>
> > size = 4 * 1024 * 1024;
> > } else if(NRL_CHECK_SIZE(ac->ac_o_ex.fe_len,(8<<20)>>bsbits,max,bsbits)){
> > - start = ac->ac_o_ex.fe_logical;
> > - start = start << bsbits;
> > - start = (start / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
> > + pstart = ac->ac_o_ex.fe_logical;
> > + pstart = pstart << bsbits;
> > + pstart = (pstart / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
>
> + start = (ac->ac_o_ex.fe_logical >> (23-bsbits)) << 23;
>
> > size = 8 * 1024 * 1024;
> > } else {
> > - start = ac->ac_o_ex.fe_logical;
> > - start = start << bsbits;
> > + pstart = ac->ac_o_ex.fe_logical;
> > + pstart = pstart << bsbits;
> > size = ac->ac_o_ex.fe_len << bsbits;
What about this ? I guess we will overflow
start = start << bsbits;
I guess start should be of type loff_t. Patch below
-aneesh
ext4: Fix overflow in ext4_mb_normalize_request
From: Aneesh Kumar K.V <[email protected]>
kernel BUG at fs/ext4/mballoc.c:3148!
The BUG_ON is:
BUG_ON(size <= 0 || size >= EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
where the value of "size" is 4293920768.
This is due to the overflow of the variable "start" in the
ext4_mb_normalize_request() function.
Signed-off-by: Aneesh Kumar K.V <[email protected]>
---
fs/ext4/mballoc.c | 21 ++++++++-------------
1 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index d8cd81e..d8a2db8 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2998,7 +2998,7 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
int bsbits, max;
ext4_lblk_t end;
struct list_head *cur;
- loff_t size, orig_size;
+ loff_t size, orig_size, start_off;
ext4_lblk_t start, orig_start;
struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
@@ -3039,7 +3039,7 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
/* first, try to predict filesize */
/* XXX: should this table be tunable? */
- start = 0;
+ start_off = 0;
if (size <= 16 * 1024) {
size = 16 * 1024;
} else if (size <= 32 * 1024) {
@@ -3055,26 +3055,21 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
} else if (size <= 1024 * 1024) {
size = 1024 * 1024;
} else if (NRL_CHECK_SIZE(size, 4 * 1024 * 1024, max, bsbits)) {
- start = ac->ac_o_ex.fe_logical << bsbits;
- start = (start / (1024 * 1024)) * (1024 * 1024);
+ start_off = (ac->ac_o_ex.fe_logical >> (20 - bsbits)) << 20;
size = 1024 * 1024;
} else if (NRL_CHECK_SIZE(size, 8 * 1024 * 1024, max, bsbits)) {
- start = ac->ac_o_ex.fe_logical << bsbits;
- start = (start / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
+ start_off = (ac->ac_o_ex.fe_logical >> (22 - bsbits)) << 22;
size = 4 * 1024 * 1024;
} else if (NRL_CHECK_SIZE(ac->ac_o_ex.fe_len,
(8<<20)>>bsbits, max, bsbits)) {
- start = ac->ac_o_ex.fe_logical;
- start = start << bsbits;
- start = (start / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
+ start_off = (ac->ac_o_ex.fe_logical >> (23 - bsbits)) << 23;
size = 8 * 1024 * 1024;
} else {
- start = ac->ac_o_ex.fe_logical;
- start = start << bsbits;
- size = ac->ac_o_ex.fe_len << bsbits;
+ start_off = ac->ac_o_ex.fe_logical << bsbits;
+ size = ac->ac_o_ex.fe_len << bsbits;
}
orig_size = size = size >> bsbits;
- orig_start = start = start >> bsbits;
+ orig_start = start = start_off >> bsbits;
/* don't cover already allocated blocks in selected range */
if (ar->pleft && start <= ar->lleft) {
Aneesh Kumar K.V wrote:
> What about this ? I guess we will overflow
> start = start << bsbits;
>
Hi Aneesh,
your patch below doesn't fix the issue, because as start_off is also
loff_t, start_off = ac->ac_o_ex.fe_logical << bsbits also overflows.
> I guess start should be of type loff_t. Patch below
>
> -aneesh
>
> ext4: Fix overflow in ext4_mb_normalize_request
>
> From: Aneesh Kumar K.V <[email protected]>
>
> kernel BUG at fs/ext4/mballoc.c:3148!
>
> The BUG_ON is:
> BUG_ON(size <= 0 || size >= EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
>
> where the value of "size" is 4293920768.
>
> This is due to the overflow of the variable "start" in the
> ext4_mb_normalize_request() function.
>
> Signed-off-by: Aneesh Kumar K.V <[email protected]>
> ---
>
> fs/ext4/mballoc.c | 21 ++++++++-------------
> 1 files changed, 8 insertions(+), 13 deletions(-)
>
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index d8cd81e..d8a2db8 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2998,7 +2998,7 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
> int bsbits, max;
> ext4_lblk_t end;
> struct list_head *cur;
> - loff_t size, orig_size;
> + loff_t size, orig_size, start_off;
> ext4_lblk_t start, orig_start;
> struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
>
> @@ -3039,7 +3039,7 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>
> /* first, try to predict filesize */
> /* XXX: should this table be tunable? */
> - start = 0;
> + start_off = 0;
> if (size <= 16 * 1024) {
> size = 16 * 1024;
> } else if (size <= 32 * 1024) {
> @@ -3055,26 +3055,21 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
> } else if (size <= 1024 * 1024) {
> size = 1024 * 1024;
> } else if (NRL_CHECK_SIZE(size, 4 * 1024 * 1024, max, bsbits)) {
> - start = ac->ac_o_ex.fe_logical << bsbits;
> - start = (start / (1024 * 1024)) * (1024 * 1024);
> + start_off = (ac->ac_o_ex.fe_logical >> (20 - bsbits)) << 20;
> size = 1024 * 1024;
> } else if (NRL_CHECK_SIZE(size, 8 * 1024 * 1024, max, bsbits)) {
> - start = ac->ac_o_ex.fe_logical << bsbits;
> - start = (start / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
> + start_off = (ac->ac_o_ex.fe_logical >> (22 - bsbits)) << 22;
> size = 4 * 1024 * 1024;
> } else if (NRL_CHECK_SIZE(ac->ac_o_ex.fe_len,
> (8<<20)>>bsbits, max, bsbits)) {
> - start = ac->ac_o_ex.fe_logical;
> - start = start << bsbits;
> - start = (start / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
> + start_off = (ac->ac_o_ex.fe_logical >> (23 - bsbits)) << 23;
> size = 8 * 1024 * 1024;
> } else {
> - start = ac->ac_o_ex.fe_logical;
> - start = start << bsbits;
> - size = ac->ac_o_ex.fe_len << bsbits;
> + start_off = ac->ac_o_ex.fe_logical << bsbits;
> + size = ac->ac_o_ex.fe_len << bsbits;
> }
> orig_size = size = size >> bsbits;
> - orig_start = start = start >> bsbits;
> + orig_start = start = start_off >> bsbits;
>
> /* don't cover already allocated blocks in selected range */
> if (ar->pleft && start <= ar->lleft) {
>
On Thu, Jan 17, 2008 at 10:43:40AM +0100, Valerie Clement wrote:
> Aneesh Kumar K.V wrote:
>> What about this ? I guess we will overflow start = start << bsbits;
>>
>
> Hi Aneesh,
> your patch below doesn't fix the issue, because as start_off is also
> loff_t, start_off = ac->ac_o_ex.fe_logical << bsbits also overflows.
>
loff_t is 64 bits.
typedef long long __kernel_loff_t;
typedef __u32 ext4_lblk_t;
-aneesh
On Thu, Jan 17, 2008 at 10:43:40AM +0100, Valerie Clement wrote:
> Aneesh Kumar K.V wrote:
>> What about this ? I guess we will overflow start = start << bsbits;
>>
>
> Hi Aneesh,
> your patch below doesn't fix the issue, because as start_off is also
> loff_t, start_off = ac->ac_o_ex.fe_logical << bsbits also overflows.
>
loff_t is 64 bits.
typedef __kernel_loff_t loff_t;
typedef long long __kernel_loff_t;
typedef __u32 ext4_lblk_t;
typedef unsigned long long ext4_fsblk_t
start_off = ac->ac_o_ex.fe_logical << bsbits;
In the above line what we are storing in start_off is the offset in bytes.So it makes
sense to use the type loff_t. It is neither logical block nor physical block.
-aneesh
Aneesh Kumar K.V wrote:
> On Thu, Jan 17, 2008 at 10:43:40AM +0100, Valerie Clement wrote:
>> Aneesh Kumar K.V wrote:
>>> What about this ? I guess we will overflow start = start << bsbits;
>>>
>> Hi Aneesh,
>> your patch below doesn't fix the issue, because as start_off is also
>> loff_t, start_off = ac->ac_o_ex.fe_logical << bsbits also overflows.
>>
>
> loff_t is 64 bits.
>
> typedef __kernel_loff_t loff_t;
> typedef long long __kernel_loff_t;
> typedef __u32 ext4_lblk_t;
> typedef unsigned long long ext4_fsblk_t
>
> start_off = ac->ac_o_ex.fe_logical << bsbits;
>
> In the above line what we are storing in start_off is the offset in bytes.So it makes
> sense to use the type loff_t. It is neither logical block nor physical block.
Oh yes, sorry, you're right. I read too quickly.
In fact, it's missing a cast :
start_off = (loff_t) ac->ac_o_ex.fe_logical << bsbits;
With that change, the test is ok.
Val?rie
On Thu, Jan 17, 2008 at 02:09:41PM +0100, Valerie Clement wrote:
> Aneesh Kumar K.V wrote:
>> On Thu, Jan 17, 2008 at 10:43:40AM +0100, Valerie Clement wrote:
>>> Aneesh Kumar K.V wrote:
>>>> What about this ? I guess we will overflow start = start << bsbits;
>>>>
>>> Hi Aneesh,
>>> your patch below doesn't fix the issue, because as start_off is also
>>> loff_t, start_off = ac->ac_o_ex.fe_logical << bsbits also overflows.
>>>
>>
>> loff_t is 64 bits.
>>
>> typedef __kernel_loff_t loff_t;
>> typedef long long __kernel_loff_t;
>> typedef __u32 ext4_lblk_t;
>> typedef unsigned long long ext4_fsblk_t
>>
>> start_off = ac->ac_o_ex.fe_logical << bsbits;
>>
>> In the above line what we are storing in start_off is the offset in bytes.So it makes
>> sense to use the type loff_t. It is neither logical block nor physical block.
>
> Oh yes, sorry, you're right. I read too quickly.
>
> In fact, it's missing a cast :
> start_off = (loff_t) ac->ac_o_ex.fe_logical << bsbits;
>
> With that change, the test is ok.
Updated patch below.
-aneesh
On Thu, 2008-01-17 at 21:59 +0530, Aneesh Kumar K.V wrote:
> On Thu, Jan 17, 2008 at 02:09:41PM +0100, Valerie Clement wrote:
> > Aneesh Kumar K.V wrote:
> >> On Thu, Jan 17, 2008 at 10:43:40AM +0100, Valerie Clement wrote:
> >>> Aneesh Kumar K.V wrote:
> >>>> What about this ? I guess we will overflow start = start << bsbits;
> >>>>
> >>> Hi Aneesh,
> >>> your patch below doesn't fix the issue, because as start_off is also
> >>> loff_t, start_off = ac->ac_o_ex.fe_logical << bsbits also overflows.
> >>>
> >>
> >> loff_t is 64 bits.
> >>
> >> typedef __kernel_loff_t loff_t;
> >> typedef long long __kernel_loff_t;
> >> typedef __u32 ext4_lblk_t;
> >> typedef unsigned long long ext4_fsblk_t
> >>
> >> start_off = ac->ac_o_ex.fe_logical << bsbits;
> >>
> >> In the above line what we are storing in start_off is the offset in bytes.So it makes
> >> sense to use the type loff_t. It is neither logical block nor physical block.
> >
> > Oh yes, sorry, you're right. I read too quickly.
> >
> > In fact, it's missing a cast :
> > start_off = (loff_t) ac->ac_o_ex.fe_logical << bsbits;
> >
> > With that change, the test is ok.
>
> Updated patch below.
>
Thanks, folded to the mballoc-core patch
Mingming
> -aneesh