On Fri, Oct 21, 2011 at 01:08:58AM +0400, Dmitry Monakhov wrote: > Current implementation has following issues: > - EOFBLOCK does not changed if necessery > - ext4_ext_rm_leaf() may return -EAGAIN due to transaction restart > - punched extent converted to uninitialized incorrectly, i dont understand > why do we ever have to do that conversion, but once we do it let's do it > in a right way. > - fsync() logic is broken because ei->i_sync_tid was not updated > - Last but not the least all: punch hole logic is sited directly > in ext4_ext_map_blocks() on 3rd level of control, IMHO one can > easily screw-up his eyes while invastigating that code. We have > nothing to hide aren't we? > > This patch performs following changes: > - Move punch hole logic to didicated function similar to uninitialized > extent handlers. > - Clear EOFBLOCK if necessery, unfortunately we can not reuse > check_eofblock_fl() function because it purpose to handle file > expansion, but in our case we have to recheck base invariant that: > clear_eof_flag = (eof_block >= last_allocated_block) > - Repeat punch hole after transaction restart. > - Update inode sync transaction id on exit. > > Signed-off-by: Dmitry Monakhov <dmonakhov@xxxxxxxxxx> I'm really sorry, but this was too complicated for me to review for this merge window. Maybe Allison or some of the people who worked on the punch code could review this? Or maybe this could be broken up into smaller patches? Or I can look at this again for the next merge window, but this is too complicated for me to understand (and the punch code is too new for me to have an deep understanding for quick reviews). Sorry I didn't review this sooner; between trying to get e2fsprogs 1.42 ready for release and kernel.org recovery efforts, I got beind on my reviews. Regards, - Ted -- 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
