On Sun, Feb 27, 2022 at 6:28 AM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote:
>
> 在 2022/2/27 0:38, Amir Goldstein 写道:
> > On Sat, Feb 26, 2022 at 5:21 PM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote:
> >> Calling fsync for parent directory in copy-up to
> >> ensure the change get synced.
> > It is not clear to me that this change is really needed
> > What if the reported problem?
>
> I found this issue by eyeball scan when I was looking for
> the places which need to mark overlay inode dirty in change.
>
> However, I think there are still some real world cases will be impacted
> by this kind of issue,
> for example, using docker build to make new docker image and power
> failure makes new
> image inconsistant.
A very good example where the fsync of parent will be counter productive.
The efficient way of building a docker image would be:
1. Write/copy up all the files
2. Writeback all the upper inodes
3. syncfs() upper fs
2 and 3 should happen on overlayfs umount as long as we properly
marked all the copied up overlayfs inodes dirty.
So the question is not if parent dir needs fsync, but if it needs to be
dirtied.
>
>
> >
> > Besides this can impact performance in some workloads.
> >
> > The difference between parent copy up and file copy up is that
> > failing to fsync to copied up data and linking/moving the upper file
> > into place may result in corrupted data after power failure if temp
> > file data is not synced.
> >
> > Failing the fsync the parent dir OTOH may result in revert to
> > lower file data after power failure.
> >
> > The thing is, although POSIX gives you no such guarantee, with
> > ext4/xfs fsync of the upper file itself will guarantee that parents
> > will be present after power failure (see [1]).
>
> In the new test case (079) which I posted, I've tried xfs as underlying
> fs and found the parent of
> copy-up file didn't present after power failure. Am I missing something?
>
I think you are.
The test does not reproduce an inconsistency.
The test reproduced changes that did not persist to storage after
power failure and that is the expected behavior when a user does
not fsync after making changes - unless the 'sync' or 'dirsync' mount
options have been used.
I think your test will become correct if you use -o sync for the overlayfs
mount and your patch will become correct if you do:
+ if (inode_needs_sync(d_inode(dentry)) {
+ parent_file = ovl_path_open(&parentpath, O_DIRECTORY|O_RDONLY);
Thanks,
Amir.
