So I've finally has some time to look at the patch and reproduce the
problem. Thanks for noticing the problem, the patch seems good,
though I have one question. Is the p_block setting really necessary
? I do not think so, but I might be missing something. Here is
updated patch I've tested, bellow.
Note: there are some indent problems in your patch, like for example
this:
+ path[k].p_block =
+ le16_to_cpu(path[k].p_hdr->eh_entries)+1;
Anyway, what do you think about the modification ?
Thanks!
-Lukas
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index b3300eb..94bc1bd 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2570,7 +2570,7 @@ static int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start,
{
struct super_block *sb = inode->i_sb;
int depth = ext_depth(inode);
- struct ext4_ext_path *path;
+ struct ext4_ext_path *path = NULL;
ext4_fsblk_t partial_cluster = 0;
handle_t *handle;
int i, err;
@@ -2606,8 +2606,12 @@ again:
}
depth = ext_depth(inode);
ex = path[depth].p_ext;
- if (!ex)
+ if (!ex) {
+ ext4_ext_drop_refs(path);
+ kfree(path);
+ path = NULL;
goto cont;
+ }
ee_block = le32_to_cpu(ex->ee_block);
@@ -2637,8 +2641,6 @@ again:
if (err < 0)
goto out;
}
- ext4_ext_drop_refs(path);
- kfree(path);
}
cont:
@@ -2646,20 +2648,26 @@ cont:
* We start scanning from right side, freeing all the blocks
* after i_size and walking into the tree depth-wise.
*/
- depth = ext_depth(inode);
- path = kzalloc(sizeof(struct ext4_ext_path) * (depth + 1), GFP_NOFS);
- if (path == NULL) {
- ext4_journal_stop(handle);
- return -ENOMEM;
- }
- path[0].p_depth = depth;
- path[0].p_hdr = ext_inode_hdr(inode);
+ if (path)
+ i = depth;
+ else {
+ depth = ext_depth(inode);
+ path = kzalloc(sizeof(struct ext4_ext_path) * (depth + 1),
+ GFP_NOFS);
+ if (path == NULL) {
+ ext4_journal_stop(handle);
+ return -ENOMEM;
+ }
+ path[0].p_depth = depth;
+ path[0].p_hdr = ext_inode_hdr(inode);
- if (ext4_ext_check(inode, path[0].p_hdr, depth)) {
- err = -EIO;
- goto out;
+ if (ext4_ext_check(inode, path[0].p_hdr, depth)) {
+ err = -EIO;
+ goto out;
+ }
+ i = 0;
}
- i = err = 0;
+ err = 0;
while (i >= 0 && err == 0) {
if (i == depth) {
On Mon, 2 Jul 2012, Ashish Sangwan wrote:
> Date: Mon, 2 Jul 2012 16:51:43 +0530
> From: Ashish Sangwan <ashishsangwan2@xxxxxxxxx>
> To: Lukáš Czerner <lczerner@xxxxxxxxxx>
> Cc: sandeen@xxxxxxxxxx, Ted Tso <tytso@xxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx,
> linux-ext4@xxxxxxxxxxxxxxx, Namjae Jeon <linkinjeon@xxxxxxxxx>
> Subject: Re: [PATCH v2] ext4: fix hole punch failure when depth is greater
> than 0
>
> I will try to elaborate more about the problem and steps to reproduce:
> Linux version 3.4.0 on X86 box.
>
> Step1:
> First create a small partition as it would be easy to fragment quickly.
> The main intent for fragmenting the partition is to create a file with
> at least 2 extent indexes.
> You can also choose some other method to create such a file.
> I created 200Mb partition and while formating used blocksize as 4KB
> and used attached script to fragment.
> This script fragments the partition such that each extent is exactly
> 6FS blocks OR 24KB in length.
> Linux#> ./fragment.sh /mnt/
> 6+0 records in
> 6+0 records out
> 24576 bytes (24.0KB) copied, 0.087997 seconds, 272.7KB/s
> cp: write error: No space left on device
> Partition filled
> rm: can't remove '/mnt//file1.7564': No such file or directory
> fragmented partition /mnt/ with 24KB files
>
> Step2:
> Create a file with 342 extents i.e 2 extent indexes
> [root@sputnik ashish]# dd if=linux-3.4.tar.bz2 of=check_1 bs=24576 count=342
> 342+0 records in
> 342+0 records out
> 8404992 bytes (8.4 MB) copied, 0.0913289 s, 92.0 MB/s
> [root@sputnik ashish]# cp check_1 /mnt/check_2
> [root@sputnik ashish]# debugfs /dev/sdb1
> debugfs 1.41.14 (22-Dec-2010)
> debugfs: dump_extents check_2
> Level Entries Logical Physical Length Flags
> 0/ 1 1/ 2 0 - 2039 32792 2040
> 1/ 1 1/340 0 - 5 5771 - 5776 6
> 1/ 1 2/340 6 - 11 5783 - 5788 6
> <------- continue like wise till 340 ------------->
> 1/ 1 340/340 2034 - 2039 9835 - 9840 6
> 0/ 1 2/ 2 2040 - 2051 5764 12
> 1/ 1 1/ 2 2040 - 2045 9847 - 9852 6
> 1/ 1 2/ 2 2046 - 2051 9859 - 9864 6
>
> Step3: Punch hole at offset 4KB and length of hole is also 4KB
> There is attached fallocate.c
> [root@sputnik ashish]# ./fallacote.x86 /mnt/check_2
> ret = 0
> [root@sputnik ashish]#
>
> Check extent tree:
> [root@sputnik ashish]# debugfs /dev/sdb1
> debugfs 1.41.14 (22-Dec-2010)
> debugfs: dump_extents check_2
> Level Entries Logical Physical Length Flags
> 0/ 1 1/ 3 0 - 5 32792 6
> 1/ 1 1/ 2 0 - 1 5771 - 5772 2
> 1/ 1 2/ 2 2 - 5 5773 - 5776 4
> 0/ 1 2/ 3 6 - 2039 9871 2034
> 1/ 1 1/339 6 - 11 5783 - 5788 6
> <------- continue like wise till 339 ------------->
> 1/ 1 339/339 2034 - 2039 9835 - 9840 6
> 0/ 1 3/ 3 2040 - 2051 5764 12
> 1/ 1 1/ 2 2040 - 2045 9847 - 9852 6
> 1/ 1 2/ 2 2046 - 2051 9859 - 9864 6
>
> It is clearly visible that punching hole just split the first extent into 2
> but failed to remove the required blocks.
>
> Step4: To cross check, take diff of 2 files.
> [root@sputnik ashish]# diff check_1 /mnt/check_2
> [root@sputnik ashish]#
>
> The 2 files are exactly same.
>
> After applying this patch, correct results will be observed.
>
> On Mon, Jul 2, 2012 at 2:42 PM, Lukáš Czerner <lczerner@xxxxxxxxxx> wrote:
> > On Mon, 2 Jul 2012, Ashish Sangwan wrote:
> >
> >> Date: Mon, 2 Jul 2012 11:03:26 +0530
> >> From: Ashish Sangwan <ashishsangwan2@xxxxxxxxx>
> >> To: sandeen@xxxxxxxxxx, Lukáš Czerner <lczerner@xxxxxxxxxx>,
> >> Ted Tso <tytso@xxxxxxx>
> >> Cc: linux-kernel@xxxxxxxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx,
> >> Namjae Jeon <linkinjeon@xxxxxxxxx>
> >> Subject: Re: [PATCH v2] ext4: fix hole punch failure when depth is greater
> >> than 0
> >>
> >> Hi Ted, Lukas, Eric,
> >>
> >> Did any of you get a chance to look at it?
> >
> > I am sorry for the delay. I have been trying to reproduce this
> > problem without any success so far. But is was on ppc64 machine, so
> > I'll try that on x86_64 and then review the patch.
> >
> > Thanks!
> > -Lukas
> >
> >>
> >> On Fri, Jun 22, 2012 at 6:19 AM, Namjae Jeon <linkinjeon@xxxxxxxxx> wrote:
> >> > Hi. Eric.
> >> >
> >> > This is the patch for punch hole issue.
> >> >
> >> > Thanks.
> >> >
> >>
>
