On Wed, Sep 13, 2023 at 4:33 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > Nathan reported that he was seeing the new warning in > setattr_copy_mgtime pop when starting podman containers. Overlayfs is > trying to set the atime and mtime via notify_change without also > setting the ctime. > > POSIX states that when the atime and mtime are updated via utimes() that > we must also update the ctime to the current time. The situation with > overlayfs copy-up is analogous, so add ATTR_CTIME to the bitmask. > notify_change will fill in the value. > IDGI, if ctime always needs to be set along with ATIME / MTIME, why not let notify_change() set the bit instead of assert and fix all the callers? But maybe I am missing something. Anyway, I have no objection to the ovl patch. It's fine by me if Christian applies it to the vfs.ctime branch with my ACK. Thanks, Amir. > Reported-by: Nathan Chancellor <nathan@xxxxxxxxxx> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > --- > The new WARN_ON_ONCE in setattr_copy_mgtime caught a bug! Fix up > overlayfs to ensure that the ctime on the upper inode is also updated > when copying up the atime and mtime. > --- > fs/overlayfs/copy_up.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c > index d1761ec5866a..ada3fcc9c6d5 100644 > --- a/fs/overlayfs/copy_up.c > +++ b/fs/overlayfs/copy_up.c > @@ -337,7 +337,7 @@ static int ovl_set_timestamps(struct ovl_fs *ofs, struct dentry *upperdentry, > { > struct iattr attr = { > .ia_valid = > - ATTR_ATIME | ATTR_MTIME | ATTR_ATIME_SET | ATTR_MTIME_SET, > + ATTR_ATIME | ATTR_MTIME | ATTR_ATIME_SET | ATTR_MTIME_SET | ATTR_CTIME, > .ia_atime = stat->atime, > .ia_mtime = stat->mtime, > }; > > --- > base-commit: 9cb8e7c86ac793862e7bea7904b3426942bbd7ef > change-id: 20230913-ctime-299173760dd9 > > Best regards, > -- > Jeff Layton <jlayton@xxxxxxxxxx> >