On Fri, Feb 22, 2013 at 10:20:42AM -0500, Theodore Ts'o wrote: > On Fri, Feb 22, 2013 at 09:18:12PM +0800, Zheng Liu wrote: > > > If there was a mode that I'd be tempted to get rid of, it would be the > > > combined data=ordered/data=writeback modes. The main reason why we > > > keep it is because of a concern of buggy applications that depend on > > > the implied fsync() of data=ordered mode at each commit. However, > > > ext4 has been around for long enough that I think most of the buggy > > > applications have been fixed by now. And of course, those buggy > > > applications will lose in the same way when they are using btrfs and > > > xfs. Something to consider.... > > > > IMHO, the application shouldn't depend on a kernel feature. So maybe it > > is time to highlight this buggy usage. > > Oh, agreed. The question is how many people want us to keep the ext3 > semantics to support those buggy applications. To the extent that > distros are considering using ext4 to support ext3 file systems, there > might be a desire to maintain (as closely as possible) ext3 semantics, > even those that support buggy applications. The primary problem is > that the when comes down to application developers versus file system > developers, the application developers vastly outnumber us. :-) Yes, as file system developers we always need to meet the application developers' requirement. So it seems that we still need to keep it in ext4. :-) > > > Just one minor comment below. Otherwise the patch looks good to me, and > > it can pass in xfstests with 'data=ordered' and 'data=writeback'. > > I hadn't bothered testing it yet because I'm focused on testing > and cleaning up the set of patches for the merge window --- and this > change is clearly for the next merge window. Thanks for testing it! I guess you are busy testing patches for the merge window. One thing I need to let you know is this patch [1]. I really think it should be applied for this merge window because it fixes a security hole due to my fault. As commit log describes, a non-privilege user could cause system crash using a truncate(1) command. So please check it. 1. http://www.spinics.net/lists/linux-ext4/msg36784.html > > > > trace_ext4_ordered_write_end(inode, pos, len, copied); > > > > Here this function needs to be renamed with trace_ext4_write_end(). > > Yes, agreed. > > I also need to get rid of trace_ext4_writeback_write_end() in > include/trace/events/ext4.h. > > The other thing that needs to be done --- probably in a separate > commit, just to make things easier to review for correctness, is now > that we've folded ext4_writeback_write_end() and ext4_ordered_write_end() > into a single function, we now have a single user of > ext4_generic_write_end(), so we can now fold ext4_generic_write_end() > into ext4_write_end(). Yes, we can take a close look at in next merge window. Thanks, - Zheng -- 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
