2012-07-15 13:03:54

by Wang Sheng-Hui

[permalink] [raw]
Subject: Question about extents.c/ext4_ext_punch_hole

Dear all,

I'm reading the code of extents.c, and confused by the function "ext4_ext_punch_hole".

In my understanding, if we want to punch hole for the range (loff_t offset, loff_t length),
then we finally will get one hole block-size aligned, right?

[code in ext4_ext_punch_hole]
first_block = (offset + sb->s_blocksize - 1) >>
EXT4_BLOCK_SIZE_BITS(sb);
stop_block = (offset + length) >> EXT4_BLOCK_SIZE_BITS(sb);
[code]

>From the code, we'll get one hole range contained in the range [offset, length+offset-1].

For example, we set the block size to 1K, and we specify the range [offset:1000, offset:2000].
Then we'll get first_block:1,stop_block:2, with some head/tail range ignored.
{-23------|===========|----952---}
1000 1024 2048 2999
offset offset+length-1
'==='means punched hole here.

Please correct me if I'm wrong.


Regards,


2012-07-16 06:36:18

by Lukas Czerner

[permalink] [raw]
Subject: Re: Question about extents.c/ext4_ext_punch_hole

On Sun, 15 Jul 2012, Wang Sheng-Hui wrote:

> Date: Sun, 15 Jul 2012 21:03:42 +0800
> From: Wang Sheng-Hui <[email protected]>
> To: Allison Henderson <[email protected]>,
> Lukas Czerner <[email protected]>, Theodore Ts'o <[email protected]>,
> [email protected]
> Subject: Question about extents.c/ext4_ext_punch_hole
>
> Dear all,
>
> I'm reading the code of extents.c, and confused by the function "ext4_ext_punch_hole".
>
> In my understanding, if we want to punch hole for the range (loff_t offset, loff_t length),
> then we finally will get one hole block-size aligned, right?
>
> [code in ext4_ext_punch_hole]
> first_block = (offset + sb->s_blocksize - 1) >>
> EXT4_BLOCK_SIZE_BITS(sb);
> stop_block = (offset + length) >> EXT4_BLOCK_SIZE_BITS(sb);
> [code]
>
> From the code, we'll get one hole range contained in the range [offset, length+offset-1].
>
> For example, we set the block size to 1K, and we specify the range [offset:1000, offset:2000].
> Then we'll get first_block:1,stop_block:2, with some head/tail range ignored.
> {-23------|===========|----952---}
> 1000 1024 2048 2999
> offset offset+length-1
> '==='means punched hole here.
>
> Please correct me if I'm wrong.
>
>
> Regards,

Hi,

your understanding is partially right. Indeed we're going to align
the range to the block size, but that's because blocks in this range
are going to be freed. Then, the rest of the range (non block
aligned) is going to be zeroed manually. The reason is that we can
not free those blocks, because it still contains valid data and we
can not just leave it alone, since after the punch hole finishes
we'll have to read zeroes from entire punched region.

Btw, there is a patch set on the list which reworks part of the
punch hole code patch, it starts with the path:

[PATCH 01/12 v2] Revert "ext4: remove no longer used functions in
inode.c"

I hope that helps.
-Lukas

2012-07-16 08:05:08

by Wang Sheng-Hui

[permalink] [raw]
Subject: Re: Question about extents.c/ext4_ext_punch_hole

On 2012年07月16日 14:36, Lukáš Czerner wrote:
> Indeed we're going to align
> the range to the block size, but that's because blocks in this range
> are going to be freed. Then, the rest of the range (non block
> aligned) is going to be zeroed manually. The reason is that we can
> not free those blocks, because it still contains valid data and we
> can not just leave it alone, since after the punch hole finishes
> we'll have to read zeroes from entire punched region.
>

Got it. Thanks, Lukas!