On Mon, Aug 26, 2024 at 8:56 AM Lv Fei(吕飞) <feilv@xxxxxxxxxxxx> wrote: > > > > > 发件人: Amir Goldstein [mailto:amir73il@xxxxxxxxx] > > 发送时间: 2024年8月23日 19:43 > > 收件人: Lv Fei(吕飞) <feilv@xxxxxxxxxxxx> > > 抄送: miklos@xxxxxxxxxx; linux-unionfs@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Xu Lianghu(徐良虎) <lianghuxu@asrmicro.> com> > > 主题: Re: [PATCH V2] ovl: fsync after metadata copy-up via mount option "fsync=strict" > > > > On Fri, Aug 23, 2024 at 11:51 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > > On Mon, Jul 22, 2024 at 3:56 PM 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 > > There is a typo here, "opion" should be "option", please help correct before merge. > No problem, but I am still waiting for Miklos to comment on this option. > > > > > 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. > > > > > > > > As for timing, we are in the middle of the merge window for > > > > 6.11-rc1, so we have some time before this can be considered for 6.12. > > > > I will be on vacation for most of this development cycle, so either > > > > Miklos will be able to queue it for 6.12 or I may be able to do near > > > > the end of the 6.11 cycle. > > > > > > > > > > Miklos, > > > > > > Please let me know what you think of this approach to handle ubifs upper. > > > If you like it, I can queue this up for v6.12. > > > > > > Thanks, > > > Amir. > > > > > > > > > > > > --- > > > > > V1 -> V2: > > > > > 1. change open flags from "O_LARGEFILE | O_WRONLY" to "O_RDONLY". > > > > > 2. change mount option to "fsync=ordered/strict/volatile". > > > > > 3. ovl_should_sync_strict() implies ovl_should_sync(). > > > > > 4. remove redundant ovl_should_sync_strict from ovl_copy_up_meta_inode_data. > > > > > 5. update commit log. > > > > > 6. update documentation overlayfs.rst. > > > > > > > > > Hi Fei, > > > > I started to test this patch and it occured to me that we have no test coverage for the "volatile" feature. > > > > Filesystem durability tests are not easy to write and I know that you tested your own use case, so I will not ask you to write a regression test as a condition for merge, but if you are willing to help, it would be very nice to add this test coverage. > > OK, I can have a try, need some time to study test suite. This is a new thing for me. > Whenever you can. > > > > There is already one overlayfs test in fstests (overlay/078) which tests behavior of overlayfs copy up during power cut (a.k.a shutdown). > > Do you mean overlay/078 in kernel/git/brauner/xfstests-dev.git ? > Yes overlay/078, but upstream repo is git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git > > > > One thing that I do request is that you confirm that you tested that the legacy "volatile" mount option still works as before. > > Yes, I tested basic function of "volatile" mount option with this patch. > Thanks, Amir.