On Wed, May 18, 2011 at 6:59 AM, Jiaying Zhang <jiayingz@xxxxxxxxxx> wrote: > There is a bug in commit c8d46e41 "ext4: Add flag to files with blocks > intentionally past EOF" that if we fallocate a file with FALLOC_FL_KEEP_SIZE > flag and then ftruncate the file to a size larger than the file's i_size, > any allocated but unwritten blocks will be freed but the file size is set > to the size that ftruncate specifies. > > Here is a simple test to reproduce the problem: > 1. fallocate a 12k size file with KEEP_SIZE flag > 2. write the first 4k > 3. ftruncate the file to 8k > Then 'ls -l' shows that the i_size of the file becomes 8k but debugfs > shows the file has only the first written block left. > > Below is the proposed patch to fix the bug: > > ext4: use vmtruncate() instead of ext4_truncate() in ext4_setattr(). > > Change ext4_setattr() to use vmtruncate(inode, attr->ia_size) instead > of ext4_truncate(inode) when it needs to truncate an inode so that > if the inode has EXT4_EOFBLOCKS_FL flag set and we are trying to truncate > to a size larger than the inode's i_size, we will only truncate the blocks > beyond the specified truncate size instead of all of blocks beyond i_size. > > Signed-off-by: Jiaying Zhang <jiayingz@xxxxxxxxxx> > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 3424e82..3bfad57 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -5347,8 +5347,11 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr) > } > } > /* ext4_truncate will clear the flag */ > - if ((ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS))) > - ext4_truncate(inode); > + if ((ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS))) { > + rc = vmtruncate(inode, attr->ia_size); > + if (rc) > + goto err_out; > + } Hi there, It seems that the error handling has problems. 1)We should handle error as the below code does. or 2) we can add a OR condition to the if statement below so that it can handle case here. I prefer the 2nd solution. it looks like: - /* ext4_truncate will clear the flag */ - if ((ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS))) - ext4_truncate(inode); - if ((attr->ia_valid & ATTR_SIZE) && - attr->ia_size != i_size_read(inode)) + if (((attr->ia_valid & ATTR_SIZE) && + attr->ia_size != i_size_read(inode)) || + ext4_test_inode_flag(inode, EXT4_INODE_EOFBLOCKS)) rc = vmtruncate(inode, attr->ia_size); Yongqiang. > } > > if ((attr->ia_valid & ATTR_SIZE) && > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Best Wishes Yongqiang Yang -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html