Also, at high level I realized that in order to allow fast commits being invoked from kjournald thread, the whole patch set has become more complicated that it needs to be. In other words, if we only support "asynchronous fast commits" in this patch set and worry about integrating it with journald thread later, we can simplify this series a whole lot and yet retain mostly all the functionality. Besides that adding support of fast commits in kjournald thread would just be an in memory change. So, just to summarize on this, 1) we will have fsync() result in only the inode in question being fast committed in async fashion. 2) ext4_nfs_commit_metadata() would result in all the changed inodes result in fast commit in async fashion as well. 3) We could very well use fast commits for normal jbd2 periodic commits as well. But it's not clear if that will add any value, so we'll leave it out from this patch series. Do you agree with this? On Wed, Oct 30, 2019 at 10:34 PM harshad shirwadkar <harshadshirwadkar@xxxxxxxxx> wrote: > > Thanks good point. I was trying to imitate how a jbd2 commit I guess. > There's no reason really to do this in atomic way. I'll fix this in > next version. > > On Thu, Oct 17, 2019 at 6:56 PM Theodore Y. Ts'o <tytso@xxxxxxx> wrote: > > > > On Tue, Oct 01, 2019 at 12:41:01AM -0700, Harshad Shirwadkar wrote: > > > + > > > +Multiple fast commit blocks are a part of one sub-transaction. To > > > +indicate the last block in a fast commit transaction, fc_flags field > > > +in the last block in every subtransaction is marked with "LAST" (0x1) > > > +flag. A subtransaction is valid only if all the following conditions > > > +are met: > > > + > > > +1) SUBTID of all blocks is either equal to or greater than SUBTID of > > > + the previous fast commit block. > > > +2) For every sub-transaction, last block is marked with LAST flag. > > > +3) There are no invalid blocks in between. > > > > I'm wondering why we need to support multiple inodes being modified in > > a single transaction. As we currently have defined what can be done, > > all updates to an inode should be free standing and not dependent on a > > change to another inode, right? And today, one block only modifies > > one inode. > > > > The only reason why we might want to define a sub-transaction as being > > composed of multiple inodes, which must all be updated in an > > all-or-nothing fashion, is the swap boot inode ioctl, and if that's > > the only one, I wonder if it's worth the extra complexity. > > > > Am I missing anything? > > > > - Ted
