| Message ID | 1321495405-9583-4-git-send-email-xiaoqiangnk@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On 2011-11-16, at 19:03, Yongqiang Yang <xiaoqiangnk@gmail.com> wrote: > Punch hole should never call ext4_ext_insert_extent, so this patch > removes code related to it from ext4_ext_insert_extent. Is that still true if punch hole is used beyond the end of the file? I don't recall whether the semantics of this mean to extend the file size or to shrink it? > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> > --- > fs/ext4/extents.c | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 6888d1a..720070d 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -1737,8 +1737,6 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > * There is no free space in the found leaf. > * We're gonna add a new leaf in the tree. > */ > - if (flag & EXT4_GET_BLOCKS_PUNCH_OUT_EXT) > - flags = EXT4_MB_USE_ROOT_BLOCKS; > err = ext4_ext_create_new_leaf(handle, inode, flags, path, newext); > if (err) > goto cleanup; > -- > 1.7.5.1 > > -- > 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 -- 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
On Fri, Nov 18, 2011 at 12:56 AM, Andreas Dilger <adilger@dilger.ca> wrote: > On 2011-11-16, at 19:03, Yongqiang Yang <xiaoqiangnk@gmail.com> wrote: > >> Punch hole should never call ext4_ext_insert_extent, so this patch >> removes code related to it from ext4_ext_insert_extent. > > Is that still true if punch hole is used beyond the end of the file? I don't recall whether the semantics of this mean to extend the file size or to shrink it? Add Allison to cc list. Punch hole is defined to 'free blocks' in documentation, so the part beyond the end of a file should be just ignored. Both ext4 and btrfs act this way. Yongqiang. > >> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> >> --- >> fs/ext4/extents.c | 2 -- >> 1 files changed, 0 insertions(+), 2 deletions(-) >> >> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c >> index 6888d1a..720070d 100644 >> --- a/fs/ext4/extents.c >> +++ b/fs/ext4/extents.c >> @@ -1737,8 +1737,6 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, >> * There is no free space in the found leaf. >> * We're gonna add a new leaf in the tree. >> */ >> - if (flag & EXT4_GET_BLOCKS_PUNCH_OUT_EXT) >> - flags = EXT4_MB_USE_ROOT_BLOCKS; >> err = ext4_ext_create_new_leaf(handle, inode, flags, path, newext); >> if (err) >> goto cleanup; >> -- >> 1.7.5.1 >> >> -- >> 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 >
On 11/16/2011 07:03 PM, Yongqiang Yang wrote: > Punch hole should never call ext4_ext_insert_extent, so this patch > removes code related to it from ext4_ext_insert_extent. > > Signed-off-by: Yongqiang Yang<xiaoqiangnk@gmail.com> > --- > fs/ext4/extents.c | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 6888d1a..720070d 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -1737,8 +1737,6 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, > * There is no free space in the found leaf. > * We're gonna add a new leaf in the tree. > */ > - if (flag& EXT4_GET_BLOCKS_PUNCH_OUT_EXT) > - flags = EXT4_MB_USE_ROOT_BLOCKS; > err = ext4_ext_create_new_leaf(handle, inode, flags, path, newext); > if (err) > goto cleanup; Hi Yongqiang, Actually I believe it does end up inserting an extent if an extent gets split. For example, we punch a hole in the middle of an extent, so we first split the extent into three pieces, and remove the middle piece. Because inserting the extra extents can require extra blocks, the operation may temporarily consume blocks. The problem that this causes is that if the file system is really full, it can fail with ENOSPC, even though punch hole is an operation that is supposed to free blocks. The above EXT4_MB_USE_ROOT_BLOCKS flag was put in so that when we punch holes, we can borrow reserved blocks to complete the operation to avoid the ENOSPC problem. I had a script to catch this and added to xfstests (test 256). So unless there was a change somewhere that I missed, it does not make sense to me to take it out at this point. Allison Henderson -- 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
On Fri, Dec 2, 2011 at 4:55 AM, Allison Henderson <achender@linux.vnet.ibm.com> wrote: > On 11/16/2011 07:03 PM, Yongqiang Yang wrote: >> >> Punch hole should never call ext4_ext_insert_extent, so this patch >> removes code related to it from ext4_ext_insert_extent. >> >> Signed-off-by: Yongqiang Yang<xiaoqiangnk@gmail.com> >> --- >> fs/ext4/extents.c | 2 -- >> 1 files changed, 0 insertions(+), 2 deletions(-) >> >> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c >> index 6888d1a..720070d 100644 >> --- a/fs/ext4/extents.c >> +++ b/fs/ext4/extents.c >> @@ -1737,8 +1737,6 @@ int ext4_ext_insert_extent(handle_t *handle, struct >> inode *inode, >> * There is no free space in the found leaf. >> * We're gonna add a new leaf in the tree. >> */ >> - if (flag& EXT4_GET_BLOCKS_PUNCH_OUT_EXT) >> - flags = EXT4_MB_USE_ROOT_BLOCKS; >> err = ext4_ext_create_new_leaf(handle, inode, flags, path, newext); >> if (err) >> goto cleanup; > > Hi Yongqiang, > > Actually I believe it does end up inserting an extent if an extent gets > split. For example, we punch a hole in the middle of an extent, so we first > split the extent into three pieces, and remove the middle piece. > > Because inserting the extra extents can require extra blocks, the operation > may temporarily consume blocks. The problem that this causes is that if the > file system is really full, it can fail with ENOSPC, even though punch hole > is an operation that is supposed to free blocks. The above > EXT4_MB_USE_ROOT_BLOCKS flag was put in so that when we punch holes, we can > borrow reserved blocks to complete the operation to avoid the ENOSPC > problem. I had a script to catch this and added to xfstests (test 256). So > unless there was a change somewhere that I missed, it does not make sense to > me to take it out at this point. Got it. Thank you for your explanation. Yongqiang. > > Allison Henderson > >
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 6888d1a..720070d 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -1737,8 +1737,6 @@ int ext4_ext_insert_extent(handle_t *handle, struct inode *inode, * There is no free space in the found leaf. * We're gonna add a new leaf in the tree. */ - if (flag & EXT4_GET_BLOCKS_PUNCH_OUT_EXT) - flags = EXT4_MB_USE_ROOT_BLOCKS; err = ext4_ext_create_new_leaf(handle, inode, flags, path, newext); if (err) goto cleanup;
Punch hole should never call ext4_ext_insert_extent, so this patch removes code related to it from ext4_ext_insert_extent. Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> --- fs/ext4/extents.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)