ext4: Make ext4_ext_find_extent fills ext_path completely
PATCH 2/2 ext4: Make ext4_ext_find_extent fills ext_path completely
When pos=0 or depth, the fields of ext4_ext_path is not completely filled.
This patch also removes some unnecessary code.
--- fs/ext4/extents.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
fs/ext4/extents.c +2 -2
@@ -524,6 +524,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, alloc =1 ; } path[0 ].p_hdr = eh; + path[0 ].p_bh =NULL ; i = depth;/* walk through the tree */
@@ -552,12 +553,11 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, } path[ppos].p_depth = i; - path[ppos].p_hdr = eh; - path[ppos].p_ext =NULL ; path[ppos].p_idx =NULL ;/* find extent */ ext4_ext_binsearch (inode, path + ppos, block); + path[ppos].p_block =ext4_ext_pblock (path[ppos].p_ext);ext4_ext_show_path (inode, path);
Signature · 1 line
1.5.4.5
Quoted 14 lines · Shen Feng
When pos=0 or depth, the fields of
ext4_ext_pathis not completely filled.
This patch also removes some unnecessary code.Signed-off-by Shen Feng <[email protected]>--- fs/ext4/extents.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)fs/ext4/extents.c +0
-0
fs/ext4/extents.c ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, · line 527
Quoted 4 lines
fs/ext4/extents.c +1
-0@@ -524,6 +524,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, alloc =1 ; } path[0 ].p_hdr = eh; + path[0 ].p_bh =NULL ;
We did a kzalloc before. So do we need this ?
fs/ext4/extents.c ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, · line 555
Quoted 8 lines
fs/ext4/extents.c +0
-1i = depth;/* walk through the tree */ @@ -552,12 +553,11 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, } path[ppos].p_depth = i; - path[ppos].p_hdr = eh;
This is needed. Because we look at extent_header in ext_binsearch.
fs/ext4/extents.c ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, · line 563
Quoted 9 lines
fs/ext4/extents.c +1
-1- path[ppos].p_ext =NULL ; path[ppos].p_idx =NULL ;/* find extent */ ext4_ext_binsearch (inode, path + ppos, block); + path[ppos].p_block =ext4_ext_pblock (path[ppos].p_ext);ext4_ext_show_path (inode, path);
-aneesh
Aneesh Kumar K.V Wrote:
Quoted 10 lines · Shen Feng
Quoted 9 lines
When pos=0 or depth, the fields of
ext4_ext_pathis not completely filled.
This patch also removes some unnecessary code.Signed-off-by Shen Feng <[email protected]>--- fs/ext4/extents.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
Quoted 12 lines
fs/ext4/extents.c ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, · line 527
Quoted 9 lines
fs/ext4/extents.c +1
-0@@ -524,6 +524,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, alloc =1 ; } path[0 ].p_hdr = eh; + path[0 ].p_bh =NULL ;We did a kzalloc before. So do we need this ?
If the path is not allocated by kzalloc, we need this.
Quoted 8 lines
Quoted 8 lines
fs/ext4/extents.c +0
-0i = depth;/* walk through the tree */ @@ -552,12 +553,11 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, }
Quoted 5 lines
fs/ext4/extents.c ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, · line 555
Quoted 2 lines
fs/ext4/extents.c +0
-1path[ppos].p_depth = i; - path[ppos].p_hdr = eh;This is needed. Because we look at
extent_headerinext_binsearch.
This has been done in the last while loop.
Quoted 16 lines
fs/ext4/extents.c ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, · line 563
Quoted 12 lines
fs/ext4/extents.c +1
-1- path[ppos].p_ext =NULL ; path[ppos].p_idx =NULL ;/* find extent */ ext4_ext_binsearch (inode, path + ppos, block); + path[ppos].p_block =ext4_ext_pblock (path[ppos].p_ext);ext4_ext_show_path (inode, path);-aneesh
Quoted 4 lines · Shen Feng
When pos=0 or depth, the fields of
ext4_ext_pathis not completely filled.
This patch also removes some unnecessary code.
I added this patch to the patch queue, unless somebody disagree.
Mingming
Quoted 31 lines
Signed-off-by Shen Feng <[email protected]>--- fs/ext4/extents.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)fs/ext4/extents.c +2
-2@@ -524,6 +524,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, alloc =1 ; } path[0 ].p_hdr = eh; + path[0 ].p_bh =NULL ; i = depth;/* walk through the tree */ @@ -552,12 +553,11 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, } path[ppos].p_depth = i; - path[ppos].p_hdr = eh; - path[ppos].p_ext =NULL ; path[ppos].p_idx =NULL ;/* find extent */ ext4_ext_binsearch (inode, path + ppos, block); + path[ppos].p_block =ext4_ext_pblock (path[ppos].p_ext);ext4_ext_show_path (inode, path);
Mingming Cao Wrote:
Quoted 29 lines · Shen Feng
Quoted 4 lines
When pos=0 or depth, the fields of
ext4_ext_pathis not completely filled.
This patch also removes some unnecessary code.I added this patch to the patch queue, unless somebody disagree.
Mingming
Quoted 20 lines
Signed-off-by Shen Feng <[email protected]>--- fs/ext4/extents.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)fs/ext4/extents.c +1
-0@@ -524,6 +524,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, alloc =1 ; } path[0 ].p_hdr = eh; + path[0 ].p_bh =NULL ; i = depth;/* walk through the tree */ @@ -552,12 +553,11 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, }
Quoted 8 lines
fs/ext4/extents.c ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, · line 560
Quoted 8 lines
fs/ext4/extents.c +1
-2path[ppos].p_depth = i; - path[ppos].p_hdr = eh; - path[ppos].p_ext =NULL ; path[ppos].p_idx =NULL ;/* find extent */ ext4_ext_binsearch (inode, path + ppos, block); + path[ppos].p_block =ext4_ext_pblock (path[ppos].p_ext);
Today I tested the patch with bonnie++ and found that sometimes path[ppos].p_ext may be NULL. This may cause kernel oops.
This is because ext4_ext_binsearch return directly when the leaf is empty.
Quoted 6 lines
Quoted 3 lines
fs/ext4/extents.c +0
-0ext4_ext_show_path (inode, path);
Shen Feng Wrote:
Quoted 35 lines
Mingming Cao Wrote:
Quoted 33 lines · Shen Feng
Quoted 4 lines
When pos=0 or depth, the fields of
ext4_ext_pathis not completely filled.
This patch also removes some unnecessary code.I added this patch to the patch queue, unless somebody disagree.
Mingming
Quoted 25 lines
Signed-off-by Shen Feng <[email protected]>--- fs/ext4/extents.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)fs/ext4/extents.c +1
-2@@ -524,6 +524,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, alloc =1 ; } path[0 ].p_hdr = eh; + path[0 ].p_bh =NULL ; i = depth;/* walk through the tree */ @@ -552,12 +553,11 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, } path[ppos].p_depth = i; - path[ppos].p_hdr = eh; - path[ppos].p_ext =NULL ; path[ppos].p_idx =NULL ;
Quoted 9 lines
Quoted 3 lines
fs/ext4/extents.c ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, · line 560
Quoted 3 lines
fs/ext4/extents.c +1
-0/* find extent */ ext4_ext_binsearch (inode, path + ppos, block); + path[ppos].p_block =ext4_ext_pblock (path[ppos].p_ext);Today I tested the patch with bonnie++ and found that sometimes path[ppos].p_ext may be NULL. This may cause kernel oops.
This is because
ext4_ext_binsearchreturn directly when the leaf is empty.
I updated the patch as following.
This patch is tested by bonnie++ and ltp fs test.
This patch is also rebased from the 2.6.26-RC5 and has no dependency to
ext4: add ext4_ prefix for ext_block and idx_pblock patch.
--- fs/ext4/extents.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
fs/ext4/extents.c +4 -1
@@ -524,6 +524,7 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, alloc =1 ; } path[0 ].p_hdr = eh; + path[0 ].p_bh =NULL ; i = depth;/* walk through the tree */
@@ -552,12 +553,14 @@ ext4_ext_find_extent(struct inode *inode, ext4_lblk_t block, } path[ppos].p_depth = i; - path[ppos].p_hdr = eh; path[ppos].p_ext =NULL ; path[ppos].p_idx =NULL ;/* find extent */ ext4_ext_binsearch (inode, path + ppos, block); +/* if not an empty leaf */ +if (path[ppos].p_ext) + path[ppos].p_block =ext_pblock (path[ppos].p_ext);ext4_ext_show_path (inode, path);
-- 1.5.5.1