On Sat 09-02-13 12:10:15, Ted Tso wrote: > On Fri, Jan 18, 2013 at 01:00:38PM +0100, Jan Kara wrote: > > 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. > > > > Reviewed-by: Zheng Liu <wenqing.lz@xxxxxxxxxx> > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > Sorry for not noticing this earlier, but this patch is causing a > regression. It is loading to test 113 failing when dioread_nolock is > used: > > 113 [ 47.619363] EXT4-fs error (device vdb): ext4_convert_unwritten_extents_endio:3411: i > node #10951: comm kworker/u:0: Written extent modified before IO finished: extent logical block > 1024, len 1024; IO logical block 1024, len 127 > [ 47.619363] > [ 47.623239] EXT4-fs warning (device vdb): ext4_convert_unwritten_extents:4522: inode #10951: > block 1024: len 127: ext4_ext_map_blocks returned -5 > [ 47.628975] EXT4-fs (vdb): failed to convert unwritten extents to written extents -- potenti > al data loss! (inode 10951, offset 4194304, size 520192, error -5) Hum, yes. Thanks for letting us know. I was able to reproduce this and I'm looking into it now. I guess it is a similar problem as Dmitry fixed in his fix... > As a result, I am considering whether or not I should to drop the > following patches from the ext4 tree: > > e63dd9c ext4: disable merging of uninitialized extents > de39534 ext4: remove unnecessary wait for extent conversion in ext4_fallocate() > 37bf0a8 ext4: ext4_split_extent should take care of extent zeroout > > I know that these patches fix other potential races which causes data > loss, but they've been around for a while, and in practice seem to be > relatively rarely hit. The third patch is a fix which shouldn't cause any issues. So you can take just that one and leave the other two aside until we are able to resolve the issue. Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- 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
