On Tue, 15 May 2012, Jan Kara wrote: > On Sun 13-05-12 14:26:40, Hugh Dickins wrote: > > In looking through hole-punching implementations, I got the impression > > that perhaps ext4 and ocfs2 might need to inode_dio_wait() before they > > truncate pagecache. And that might require ext4 to hold i_mutex there? > Not necessarily before truncating page cache AFAICS Yes, DIO would hold reference to pages, so truncating cache should be safe. > but certainly before we go and remove blocks from the file. Yes, that seems more like it. > In case of ext4 doing that before > ext4_flush_completed_IO() call would look most appropriate I'd think. Good > spotting! Will you send a patch? No, Jan, I'm afraid of doing more harm than good to ext4 here - I'd much rather trust you to take that risk with my data! In particular, it's not clear to me whether ext4 needs to take i_mutex there, so inode_dio_wait() semantics are complete. And it's not clear to me precisely where it should take i_mutex if so: maybe it actually needs to take i_mutex across the whole function, I do not know. Notice (I sent Allison reminder of it yesterday) that ext4_ext_punch_hole() is already wrong in its "if (offset >= inode->i_size) return 0" at the start, which makes fallocate PUNCH_HOLE unable to remove blocks added earlier by fallocate KEEP_SIZE beyond eof. But I know nothing of ext4 block allocation rules, so that's something also where a "fix" from me would be more likely to do harm than good. Thanks a lot for taking these on, if you will. Hugh -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html