On Mon, Feb 24, 2014 at 10:22:10AM +0900, Namjae Jeon wrote:

quoted

quoted

+	ret = ext4_es_remove_extent(inode, punch_start,
+				    EXT_MAX_BLOCKS - punch_start - 1);
+	if (ret) {
+		up_write(&EXT4_I(inode)->i_data_sem);
+		goto out_stop;
+	}

Doing this at first is probably a bad idea; you should do this at the
end, and then completely invalidate the es cache for that inode.  That
way, the right thing happens if you get an error in the middle
releasing the boxes and shifting the extents:

Okay, I see.

Actually, thinking about this some more, we do want to do this first,
since if we error out, we do need to make sure the extent cache is
flushed.

If there is error in the middle of extent shifting, the hole will
present between the last shifted extent and the extent at which error
happen so this will be consistent state.
IMHO even if there is error in between the shift, filesystem will be
in consistent state.
Am I missing something?

No, I was wrong about that; you're right.  The file will be in an
inconsistent statement, which will probably be highly confusing for
the application, but the file system will be fine.

So I withdraw my complaints.  I'll do a bit more testing, but so far
the patch looks fine to me.  Thanks for your reply and your work!

					- Ted

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs