On Mon, Aug 26, 2024 at 5:59 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > On Mon, 22 Jul 2024 at 15:56, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > On Mon, Jul 22, 2024 at 1:14 PM Fei Lv <feilv@xxxxxxxxxxxx> wrote: > > > > > > For upper filesystem which does not enforce ordering on storing of > > > metadata changes(e.g. ubifs), when overlayfs file is modified for > > > the first time, copy up will create a copy of the lower file and > > > its parent directories in the upper layer. Permission lost of the > > > new upper parent directory was observed during power-cut stress test. > > > > > > Fix by adding new mount opion "fsync=strict", make sure data/metadata of > > > copied up directory written to disk before renaming from tmp to final > > > destination. > > > > > > Signed-off-by: Fei Lv <feilv@xxxxxxxxxxxx> > > > > Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx> > > > > but I'd also like to wait for an ACK from Miklos on this feature. > > I'm okay with this. I'm a little confused about sync=strict mode, > since most copy ups will have vfs_fsync() called twice. Is this what > we want, or could this be consolidated into a single fsync? > Maybe it could, but remember that ubifs strict mode is the odd case if we have an extra fsync for the odd case, I think code simplicity is a more important factor. > Also is it worth optimizing away the fsync on the directory in cases > the filesystem is well behaved? Maybe we should just move the > vfs_fsync() call into ovl_copy_up_metadata() and omit the complexity > related to the additional mount option? > Hmm. Maybe you are confused by the commit message that only mentions fsync of the parent directory (same as the reported reproducer), but the strict mode fsync also affects metacopy, not only parent dir copy up. > To me it feels that it shouldn't matter in terms of performance, but > if reports of performance regressions come in, we can still make this > optional. > I think that the case of chown -R with metacopy is going to be terribly crippled if every metacopy gets and fsync. Thanks, Amir.
