When working on the patch of adding the data=nojournal mount option, I start to wonder whether this mount option is actually needed. When we mount a filesystem that was mkfs'ed with journal, using the "noload" mount option can help specify we do not load the journal. When we mount a filesystem that was mkfs'ed without journal, we simply go into the nojournal mode. That said, I do not really feel this data=nojournal option is necessary. But I am still working on the patch to print appropriate messages when people mount a filesystem created without a journal but explicitly specify the "data=" option. Any comments? Thanks, Xiang On Mon, Jul 6, 2009 at 9:21 AM, Xiang Wang<xiangw@xxxxxxxxxx> wrote: > On Sun, Jul 5, 2009 at 8:41 PM, Theodore Tso<tytso@xxxxxxx> wrote: >> On Thu, Jul 02, 2009 at 12:01:30AM +0530, Aneesh Kumar K.V wrote: >>> >>> I looked at the patch in detail and I guess we should instead force >>> a data=writeback mode if the filesystem is created without a journal. >>> I am not sure what whould be the meaning of data=ordered/data=journal >>> without a journal. So if we find that file system doesn't have a journal >>> then either we should update the default mount option in the filesystem >>> to be of data=writeback. >> >> Here's a patch which takes your approach to solving the problem. What >> do you think? >> >> I haven't messed with dealing with the data= mount options in >> fs/ext4/super.c. That's important from a UI point of view, but we >> needed to fix ext4_jbd2.h since it was unconditionally returning 0 if >> there was no journal for all of the ext4_should_*_data() functions. >> >> I believe this should DTRT with the -o nobh mount option, but I'd >> appreciate another pair of eyes taking a look at this. > > This patch looks good to us. > > In the long run, we still think adding the data=nojournal mount option > is useful and we are working on this patch. > > Thanks, > Xiang > >> >> - Ted >> >> commit 2a73eff8ba80095a871a6b402dfd24bc454e5bdc >> Author: Theodore Ts'o <tytso@xxxxxxx> >> Date: Sun Jul 5 23:37:13 2009 -0400 >> >> ext4: fix no journal corruption with locale-gen >> >> If there is no journal, ext4_should_writeback_data() should return >> TRUE. This will fix ext4_set_aops() to set ext4_da_ops in the case of >> delayed allocation; otherwise ext4_journaled_aops gets used by >> default, which doesn't handle delayed allocation properly. >> >> The advantage of using ext4_should_writeback_data() approach is that >> it should handle nobh better as well. >> >> Thanks to Curt Wohlgemuth for investigating this problem, and Aneesh >> Kumar for suggesting this approach. >> >> Signed-off-by: "Theodore Ts'o" <tytso@xxxxxxx> >> >> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h >> index be2f426..f800134 100644 >> --- a/fs/ext4/ext4_jbd2.h >> +++ b/fs/ext4/ext4_jbd2.h >> @@ -282,7 +282,7 @@ static inline int ext4_should_order_data(struct inode *inode) >> static inline int ext4_should_writeback_data(struct inode *inode) >> { >> if (EXT4_JOURNAL(inode) == NULL) >> - return 0; >> + return 1; >> if (!S_ISREG(inode->i_mode)) >> return 0; >> if (EXT4_I(inode)->i_flags & EXT4_JOURNAL_DATA_FL) >> -- >> 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 >> > -- 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