Leah pointed out that jbd2_journal_flush() function commits the currently running transaction. EXT4_IOC_CHECKPOINT calls jbd2_journal_flush() so unless I'm missing something, we're actually doing all the work that syncfs() would do from EXT4_IOC_CHECKPOINT ioctl before sending discards. Also, the journal commit that happens as a part of jbd2_journal_flush() is always a full commit. So, it sounds like we don't really need to do anything more here. Thanks Leah for pointing it out! On Fri, May 7, 2021 at 9:22 AM harshad shirwadkar <harshadshirwadkar@xxxxxxxxx> wrote: > > > > > > > + err = jbd2_journal_flush(EXT4_SB(sb)->s_journal, > > > > > > > > Huh. So we don't flush the filesystem at all, just the journal? I > > > > don't see anything in the documentation saying that syncfs() is a > > > > prerequisite. > > > > This is just for the journal, good point, I'll update the documentation. > It just occurred to me this morning that we need to ensure that a > *full* commit happens before IOC_CHECKPOINT and not a *fast* commit. > Fast commits cannot be checkpointed, they rely on full commits for > checkpoint operation. So, if a syncfs call results in a fast commit, > the following sequence of events will happen: > > * Ext4 writes fast commit information in fast commit area > > * When user calls EXT4_IOC_CHECKPOINT, the checkpoint operation would > result in checkpointing everything in the main journal, except things > written in fast commit area > > * During the discard phase of EXT4_IOC_CHECKPOINT, fast commit area > will be discarded and thus we'll lose the log updates present in the > fast commit area > > However, this isn't a problem today. Syncfs doesn't result in a fast > commit but results in a full commit. But, that can change at some > point in the future. So, unless we can either come up with syncfs() > variant that can induce a full commit (which would be a little ugly - > I don't think the user needs to know what kind of journal commit file > system is doing) or add checkpointing support in fast commits, we > should just do a full commit from the IOCTL code. > > - Harshad
