Re: [PATCH] namei: implemented RENAME_NEWER flag for renameat2() conditional replace

From: Amir Goldstein <amir73il@gmail.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: James Yonan <james@openvpn.net>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH] namei: implemented RENAME_NEWER flag for renameat2() conditional replace
Date: Tue, 28 Jun 2022 21:34:49 +0300	[thread overview]
Message-ID: <CAOQ4uxgoZe8UUftRKf=b--YmrKJ4wdDX99y7G8U2WTuuVsyvdA@mail.gmail.com> (raw)
In-Reply-To: <Yrs7lh6hG44ERoiM@ZenIV>

On Tue, Jun 28, 2022 at 8:44 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Mon, Jun 27, 2022 at 04:11:07PM -0600, James Yonan wrote:
>
> >           && d_is_positive(new_dentry)
> >           && timespec64_compare(&d_backing_inode(old_dentry)->i_mtime,
> >                                 &d_backing_inode(new_dentry)->i_mtime) <= 0)
> >               goto exit5;
> >
> > It's pretty cool in a way that a new atomic file operation can even be
> > implemented in just 5 lines of code, and it's thanks to the existing
> > locking infrastructure around file rename/move that these operations
> > become almost trivial.  Unfortunately, every fs must approve a new
> > renameat2() flag, so it bloats the patch a bit.
>
> How is it atomic and what's to stabilize ->i_mtime in that test?
> Confused...

Good point.
RENAME_EXCHANGE_WITH_NEWER would have been better
in that regard.

And you'd have to check in vfs_rename() after lock_two_nondirectories()

Thanks,
Amir.