On Tue, Oct 03, 2017 at 03:59:30PM -0400, Theodore Ts'o wrote: > On Tue, Oct 03, 2017 at 11:53:12AM -0700, Luis R. Rodriguez wrote: > > @@ -4926,7 +4926,7 @@ static int ext4_unfreeze(struct super_block *sb) > > ext4_set_feature_journal_needs_recovery(sb); > > } > > > > - ext4_commit_super(sb, 1); > > + ext4_commit_super(sb, 0); > > return 0; > > } > > > > After we remove add the NEEDS_RECOVERY flag, we need to make sure > recovery flag is pushed out to disk before any other changes are > allowed to be pushed out to disk. That's why we originally did the > update synchronously. I see. I had to try to dig through linux-history to see why this was done, and I actually could not find an exact commit which explained what you just did. Thanks! Hrm, so on freeze we issue the same commit as well, so is a sync *really* needed on thaw? > There are other ways we could fulfill this requirements, but doing a > synchronous update is the simplest way to handle this. Was it > necessary to change this given the other changes you are making the fs > freeze implementation? No, it was am empirical evaluation done at testing, I observed bio_submit() stalls, we never get completion from the device. Even if we call thaw at the very end of resume, after the devices should be up, we still end up in the same situation. Given how I order freezing filesystems after freezing userspace I do believe we should thaw filesystems prior unfreezing userspace, its why I placed the call where it is now. So this is something I was hoping others could help grok/iron out. I'd prefer if we could live with thaw'ing unchanged, but this requires to figure this issue out. Why we can't implicate bio_submit() on fs thaw as-is in this implementation. Luis
