All of
 help / color / mirror / Atom feed
From: James Yonan <>
To: Amir Goldstein <>, Al Viro <>
Cc: linux-fsdevel <>
Subject: Re: [PATCH] namei: implemented RENAME_NEWER flag for renameat2() conditional replace
Date: Tue, 28 Jun 2022 17:19:12 -0600	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 6/28/22 12:34, Amir Goldstein wrote:
> On Tue, Jun 28, 2022 at 8:44 PM Al Viro <> 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()

So I mean atomic in the sense that you are comparing the old and new 
mtimes inside the lock_rename/unlock_rename critical section in 
do_renameat2(), so the basic guarantees of rename still hold, i.e. that 
readers see an atomic transition from old to new files, or no transition 
(where mtime comparison results in -EEXIST return).  I understand that 
it doesn't guarantee i_mtime stability, but the application layer may 
not need that guarantee. In our case, mtime is immutable after local 
file creation and before do_renameat2() is used to move the file into place.

Re: RENAME_EXCHANGE_WITH_NEWER, that's an interesting idea.  You could 
actually implement it with minor changes in the patch, by simply 
combining RENAME_EXCHANGE|RENAME_NEWER.  Because fundamentally, all 
RENAME_NEWER does is compare mtimes and possibly return early with 
-EEXIST.  If the early return is not taken, then it becomes a plain 
rename or RENAME_EXCHANGE if that flag is also specified.


  reply	other threads:[~2022-06-28 23:19 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-27 22:11 [PATCH] namei: implemented RENAME_NEWER flag for renameat2() conditional replace James Yonan
2022-06-28  9:46 ` Amir Goldstein
2022-06-28 21:56   ` James Yonan
2022-06-29  5:15     ` Amir Goldstein
2022-06-30 16:18       ` James Yonan
2022-06-28 17:34 ` Al Viro
2022-06-28 18:34   ` Amir Goldstein
2022-06-28 23:19     ` James Yonan [this message]
2022-06-29  1:43       ` Dave Chinner
2022-06-29  2:07         ` NeilBrown
2022-06-29  2:35           ` Dave Chinner
2022-06-29  2:49             ` NeilBrown
2022-06-30 16:39             ` James Yonan
2022-07-01  9:23               ` [PATCH v2] namei: implemented RENAME_NEWER_MTIME " James Yonan
2022-07-01 10:34                 ` Amir Goldstein
2022-07-01 20:06                   ` James Yonan
2022-07-02  8:07                 ` Dave Chinner
2022-07-05 13:30                   ` [PATCH v3 1/2] RENAME_NEWER_MTIME is a new userspace-visible flag for renameat2(), and stands alongside existing flags including RENAME_NOREPLACE, RENAME_EXCHANGE, and RENAME_WHITEOUT James Yonan
2022-07-05 13:30                     ` [PATCH v3 2/2] selftests: added a new target renameat2 James Yonan
2022-07-05 13:30                     ` [PATCH man-pages] rename.2: document new renameat2() flag RENAME_NEWER_MTIME James Yonan
2022-07-05 14:03                   ` [RESEND PATCH v3 1/2] namei: implemented RENAME_NEWER_MTIME flag for renameat2() conditional replace James Yonan
2022-07-11 19:13                   ` [PATCH v4 " James Yonan
2022-07-11 19:13                     ` [PATCH v4 2/2] selftests: added a new target renameat2 James Yonan
2022-07-11 23:10                     ` [PATCH v4 1/2] namei: implemented RENAME_NEWER_MTIME flag for renameat2() conditional replace Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.