Message ID | 1361808463-25471-2-git-send-email-dmonakhov@openvz.org |
---|---|
State | Accepted, archived |
Headers | show |
On Mon 25-02-13 20:07:40, Dmitry Monakhov wrote: > Derived from Jan's patch:http://permalink.gmane.org/gmane.comp.file-systems.ext4/36470 > > Merging of uninitialized extents creates all sorts of interesting race > possibilities when writeback / DIO races with fallocate. Thus > ext4_convert_unwritten_extents_endio() has to deal with a case where > extent to be converted needs to be split out first. That isn't nice > for two reasons: > > 1) It may need allocation of extent tree block so ENOSPC is possible. > 2) It complicates end_io handling code > > So we disable merging of uninitialized extents which allows us to simplify > the code. Extents will get merged after they are converted to initialized > ones. Looks good. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org> > --- > fs/ext4/extents.c | 8 +++++--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 3bd3ca5..1d37f2d 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -1584,10 +1584,12 @@ ext4_can_extents_be_merged(struct inode *inode, struct ext4_extent *ex1, > unsigned short ext1_ee_len, ext2_ee_len, max_len; > > /* > - * Make sure that either both extents are uninitialized, or > - * both are _not_. > + * Make sure that both extents are initialized. We don't merge > + * uninitialized extents so that we can be sure that end_io code has > + * the extent that was written properly split out and conversion to > + * initialized is trivial. > */ > - if (ext4_ext_is_uninitialized(ex1) ^ ext4_ext_is_uninitialized(ex2)) > + if (ext4_ext_is_uninitialized(ex1) || ext4_ext_is_uninitialized(ex2)) > return 0; > > if (ext4_ext_is_uninitialized(ex1)) > -- > 1.7.1 >
On Mon, Feb 25, 2013 at 08:07:40PM +0400, Dmitry Monakhov wrote: > Derived from Jan's patch:http://permalink.gmane.org/gmane.comp.file-systems.ext4/36470 > > Merging of uninitialized extents creates all sorts of interesting race > possibilities when writeback / DIO races with fallocate. Thus > ext4_convert_unwritten_extents_endio() has to deal with a case where > extent to be converted needs to be split out first. That isn't nice > for two reasons: > > 1) It may need allocation of extent tree block so ENOSPC is possible. > 2) It complicates end_io handling code > > So we disable merging of uninitialized extents which allows us to simplify > the code. Extents will get merged after they are converted to initialized > ones. > > Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org> After applied this patch, xfstests #275 will print a warning message. kernel:EXT4-fs (sda2): failed to convert unwritten extents to written extents -- potential data loss! (inode 13, offset 1537212416, size 524288, error -28) But IMHO we don't need to worry about it because it is hard to be trigger. I hit it because I run xfstests #275 several times. So just a note here. Regards, - Zheng -- 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
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 3bd3ca5..1d37f2d 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -1584,10 +1584,12 @@ ext4_can_extents_be_merged(struct inode *inode, struct ext4_extent *ex1, unsigned short ext1_ee_len, ext2_ee_len, max_len; /* - * Make sure that either both extents are uninitialized, or - * both are _not_. + * Make sure that both extents are initialized. We don't merge + * uninitialized extents so that we can be sure that end_io code has + * the extent that was written properly split out and conversion to + * initialized is trivial. */ - if (ext4_ext_is_uninitialized(ex1) ^ ext4_ext_is_uninitialized(ex2)) + if (ext4_ext_is_uninitialized(ex1) || ext4_ext_is_uninitialized(ex2)) return 0; if (ext4_ext_is_uninitialized(ex1))
Derived from Jan's patch:http://permalink.gmane.org/gmane.comp.file-systems.ext4/36470 Merging of uninitialized extents creates all sorts of interesting race possibilities when writeback / DIO races with fallocate. Thus ext4_convert_unwritten_extents_endio() has to deal with a case where extent to be converted needs to be split out first. That isn't nice for two reasons: 1) It may need allocation of extent tree block so ENOSPC is possible. 2) It complicates end_io handling code So we disable merging of uninitialized extents which allows us to simplify the code. Extents will get merged after they are converted to initialized ones. Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org> --- fs/ext4/extents.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)