mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "koraynilay" <koray.fra@gmail.com>
To: "Amir Goldstein" <amir73il@gmail.com>,
	"koraynilay" <koray.fra@gmail.com>
Cc: "David Sterba" <dsterba@suse.com>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Namjae Jeon" <linkinjeon@kernel.org>,
	"Sungjong Seo" <sj1557.seo@samsung.com>,
	"Konstantin Komarov" <almaz.alexandrovich@paragon-software.com>,
	"Chris Mason" <mason@kernel.org>, "Jan Kara" <jack@suse.cz>,
	"Yuezhang Mo" <yuezhang.mo@sony.com>,
	"Chuck Lever" <cel@kernel.org>,
	"Darrick J . Wong" <djwong@kernel.org>,
	"Sean Smith" <defendthedisabled@gmail.com>,
	"Andrey Albershteyn" <aalbersh@kernel.org>,
	"Pedro Falcato" <pfalcato@suse.de>,
	"Pali Rohár" <pali@kernel.org>,
	linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, exfat@lists.linux.dev,
	ntfs3@lists.linux.dev, linux-api@vger.kernel.org,
	"Theodore Tso" <tytso@mit.edu>
Subject: Re: [RFC PATCH v3 1/5] vfs: add provenance time (ptime)
Date: Mon, 09 Nov 2026 23:34:49 +0100	[thread overview]
Message-ID: <DMR1E7XB3AT6.3UI3YHHBI8O9Z@gmail.com> (raw)
In-Reply-To: <CAOQ4uxj82K4z+2OiY5W1g7NyWAFOFjqqWnZSS3prytW36j1LgA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 11750 bytes --]

On Mon Nov 9, 2026 at 3:35 PM CET, Amir Goldstein wrote:
> On Sat, Nov 7, 2026 at 6:09 PM koraynilay <koray.fra@gmail.com> wrote:
>>
>> On Sat Nov 7, 2026 at 5:48 PM CET, Amir Goldstein wrote:
>> > On Sat, Nov 7, 2026 at 5:04 PM koraynilay <koray.fra@gmail.com> wrote:
>> >>
>> >> On Mon Nov 2, 2026 at 3:00 PM CET, Amir Goldstein wrote:
>> >> > On Sun, Nov 1, 2026 at 2:57 AM koraynilay <koray.fra@gmail.com> wrote:
>> >> >>
>> >> >> Add provenance time (ptime), indicating the time when a file or
>> >> >> directory was created. This is user-settable via file_setattr() and
>> >> >> retrievable via statx(2) (using STATX_PTIME) or file_getattr().
>> >> >>
>> >> >> The semantics of when to preserve this new timestamp are the same as
>> >> >> mtime, the idea being that there is a correspondence like this:
>> >> >>
>> >> >> ctime <-> btime
>> >> >> mtime <-> ptime
>> >> >>
>> >> >> so every operation (copying, moving, ...) that currently preserves mtime
>> >> >> should also preserve ptime.
>> >> >>
>> >> >> Furthermore, in order to avoid the legacy FS_IOC_FSSETXATTR accepting
>> >> >> FS_XFLAG_PTIME, add an FS_FILE_ATTR_XFLAGS_MASK mask to support new
>> >> >> xflags, and use it in fileattr_to_file_attr()/file_attr_to_fileattr() to
>> >> >> allow ptime to be used with the file_setattr()/file_getattr() syscalls.
>> >> >>
>> >> >> [ This is a re-implementation of the initial patch[1] by Sean Smith
>> >> >>   <defendthedisabled@gmail.com>, following the advice[2] from Darrick J.
>> >> >>   Wong <djwong@kernel.org> of using file_setattr() instead of updating
>> >> >>   utimensat() ]
>> >> >>
>> >> >> [1]: https://lore.kernel.org/20260405195007.1306-2-DefendTheDisabled@gmail.com
>> >> >> [2]: https://lore.kernel.org/20260407014129.GC6192@frogsfrogsfrogs
>> >> >>
>> >> >> Assisted-by: Gemini:3.8-flash-high antigravity-cli-1.4.7
>> >> >> Signed-off-by: koraynilay <koray.fra@gmail.com>
>> >> >> ---
>> >> >>  fs/file_attr.c            | 33 +++++++++++++++++++++++++++++++--
>> >> >>  fs/stat.c                 |  2 ++
>> >> >>  include/linux/fileattr.h  |  5 +++++
>> >> >>  include/linux/stat.h      |  1 +
>> >> >>  include/uapi/linux/fs.h   |  8 +++++++-
>> >> >>  include/uapi/linux/stat.h |  7 ++++++-
>> >> >>  6 files changed, 52 insertions(+), 4 deletions(-)
>> >> >>
>> >> >> diff --git a/fs/file_attr.c b/fs/file_attr.c
>> >> >> index 833e03810b24..c0645cb22bf3 100644
>> >> >> --- a/fs/file_attr.c
>> >> >> +++ b/fs/file_attr.c
>> >> >> @@ -102,7 +102,7 @@ EXPORT_SYMBOL(vfs_fileattr_get);
>> >> >>  static void fileattr_to_file_attr(const struct file_kattr *fa,
>> >> >>                                   struct file_attr *fattr)
>> >> >>  {
>> >> >> -       __u32 mask = FS_XFLAGS_MASK;
>> >> >> +       __u32 mask = FS_FILE_ATTR_XFLAGS_MASK;
>> >> >>
>> >> >>         memset(fattr, 0, sizeof(struct file_attr));
>> >> >>         fattr->fa_xflags = fa->fsx_xflags & mask;
>> >> >> @@ -110,6 +110,8 @@ static void fileattr_to_file_attr(const struct file_kattr *fa,
>> >> >>         fattr->fa_nextents = fa->fsx_nextents;
>> >> >>         fattr->fa_projid = fa->fsx_projid;
>> >> >>         fattr->fa_cowextsize = fa->fsx_cowextsize;
>> >> >> +       fattr->fa_ptime_sec = fa->fsx_ptime.tv_sec;
>> >> >> +       fattr->fa_ptime_nsec = fa->fsx_ptime.tv_nsec;
>> >> >>  }
>> >> >>
>> >> >>  /**
>> >> >> @@ -141,15 +143,20 @@ EXPORT_SYMBOL(copy_fsxattr_to_user);
>> >> >>  static int file_attr_to_fileattr(const struct file_attr *fattr,
>> >> >>                                  struct file_kattr *fa)
>> >> >>  {
>> >> >> -       __u64 mask = FS_XFLAGS_MASK;
>> >> >> +       __u64 mask = FS_FILE_ATTR_XFLAGS_MASK;
>> >> >
>> >> > That is not the correct way to extend this API.
>> >> > This fixed mask was added as a placeholder for
>> >> > "We intend to add support for more xflags to selective filesystems":
>> >> > https://lore.kernel.org/linux-fsdevel/20250329143312.1350603-2-amir73il@gmail.com/
>> >> >
>> >> > Please see this POC patch for filesystems to publish their supported
>> >> > xflags mask:
>> >> > https://lore.kernel.org/linux-fsdevel/20250329143312.1350603-3-amir73il@gmail.com/
>> >>
>
> Re-reading and answering your original questions:
>
>> >> Quick question, should _mask also be used for user-space -> VFS to advertise
>> >> what it wants to change?
>
> Yes absolutely.
>
>> >> This would be useful for on/off flags,
>
> Exactly.
>
> All the Windows filesystems that want ptime, would most likely want the support
> on/off HIDDEN/SYSTEM/ARCHIVE flags as well.

(But these should be deferred to other patches/to the FSs themselves
right?)

>> >> but for ptime both fa->xflags and
>> >> fa->xflags_mask would have the same meaning of "I want to change ptime,
>> >> look at fa->ptime".
>
> There needs to be no FS_XLAG_PTIME in fa->xflags,
> just as there is no FS_XFLAG_PROJID in fa->xflags today.
>
> However, since btrfs does not support setting projid, when we introduce
> fa->xflags_mask, it would be elegant IMO to introduce the (new) flag
> FS_XFLAG_PROJID which would be valid in fa->xflags_mask.
>
> To be clear, currently upstream chattr (e2fsprogs) can set projid via
> FS_IOC_FSSETXATTR ioctl (no xflags_mask) and upstream
> xfs_io -c chattr (xfsprogs) can set projid via  file_setattr() syscall
> (no xflags_mask).
>
> But if either utils were to be modified to support setting ptime, then it
> would need to use file_setattr() syscall and set both FS_XLAG_PTIME
> and FS_XFLAG_PROJID in  xflags_mask when it wants to set a non-zero
> value to those fields.
>
> Specifically, the invocation of chattr -p <projid>, will no longer need to do
> FS_IOC_FSGETXATTR to read all the unrelated flags for setting the projid.

Ah yes, this would be a good change I think.

>> >> Should we make _mask the only way to advertise what the user wants to
>> >> change,
>
> Yes, unless _mask is zero, which then means set all the legacy values
> which are frozen in the constant FS_XFLAGS_MASK.
>
>> >> ignore xflags | PTIME in user-space -> VFS and use it only in FS
>> >> -> VFS -> user-space to say "this fs supports PTIME for this file"
>
> Close, but not exactly.
>
> For GET, semantics are clear and follow statx precedents:
> xflags_mask always means "supported" (like stx_attributes_mask).
>
> For boolean flags, xflags is the value
> For integer fields, an xflags bit means "value provided" (like stx_mask).
>
> For SET, when xflags_mask!=0, a flag of 1 means "I want to set this value".
>
> For boolean flags, if the flag's bit in xflags_mask is 0, the flag is
> left untouched
> (e.g. a chattr -p that only sets the project ID doesn't touch IMMUTABLE).
>
> For integer fields, the meaning of xflags_mask 1 and xflags 0 would be
> "I want to unset the value" which is not the same as "I want to set the value
> to 0", because as you wrote, 0 is a valid value for ptime.
>
> The question of whether a specific field can be unset or not is up for the
> filesystem to decide, not the VFS.
>
> For example, XFS already unset the value of cowextsize when flag
> FS_XFLAG_COWEXTSIZE is 0, although the same could be done
> with a 0 value to fa_cowextsize.
>
> VFS does enforce that xflags & ~xflags_mask is EINVAL.
>
>> >> (which will be always once support is added to an FS, so it would just
>> >> be "this FS supports PTIME")?
>
> It does not have to be an FS-wide property. some fs could have ptime
> (or another attribute) support only on some files types.

Right.

>> >>
>> >
>> > Sorry I was not able to parse your question.
>> >
>> > I think the mask should mean "this FS supports PTIME"
>> > regardless if the user asked to get/set it or not.
>>
>> Yeah sorry, I'm just a bit confused because of all the similar names and
>> concepts.
>
> Me too... I needed to re-read all the threads from Feb 2025...

I also had to go back to those threads, since at least those have more
parsable function names ahah.

>>
>> (assuming btrfs supports ptime and ext4 doesn't)
>>
>> * filesystem -> VFS -> user-space, while in fileattr_to_file_attr():
>>
>>   for btrfs, for all files[1]:
>>         fa->fa_xflags & FS_XFLAG_PTIME == 1
>>         fa->fa_xflags_mask & FS_XFLAG_PTIME == 1
>
> Yes.
>
>>
>>   for ext4, for all files:
>>         fa->fa_xflags & FS_XFLAG_PTIME == 0
>>         fa->fa_xflags_mask & FS_XFLAG_PTIME == 0
>
> Yes.
>
>>
>> * user-space -> VFS -> filesystem, while in file_attr_to_fileattr():
>>
>>         fattr->fa_xflags & FS_XFLAG_PTIME == ignored
>
> This is debatable. IMO it should be 1 for value, 0 to unset existing value,
> similar to removexattr() if you want to look at it this way.

So in the case of ptime, fattr->fa_xflags & FS_XFLAG_PTIME should be
ignored, while for fattr->fa_xflags & FS_XFLAG_SOME_NEW_THING it will
need to be handled as

== 1: set it taking the value from ->some_new_thing
== 0: unset the value

>>         fattr->fa_xflags_mask & FS_XFLAG_PTIME == 1 if user wants to set it, otherwise 0
>
> Yes. but now the user has to set any legacy field it wants to set in
> fa_xflags_mask as well, because fa_xflags_mask is no longer zero.
> so we need to introduce flag constants for legacy fields that do not
> have xflags. I think that is only FS_XFLAG_PROJID.

Should this be a patch in this series, or a separate patch sent after?

>>
>> Would this be correct?
>>
>> Or would it be better to set FS_XFLAG_PTIME only in fa_xflags_mask, and
>> never touch fa_xflags with it, which would make the above become:
>>
>>
>> * filesystem -> VFS -> user-space, while in fileattr_to_file_attr():
>>
>>   for btrfs, for all files[1]:
>>         fa->fa_xflags_mask & FS_XFLAG_PTIME == 1
>>
>>   for ext4, for all files:
>>         fa->fa_xflags_mask & FS_XFLAG_PTIME == 0
>>
>>   (and always fa->fa_xflags & FS_XFLAG_PTIME == 0)
>>
>> * user-space -> VFS -> filesystem, while in file_attr_to_fileattr():
>>
>>         (fattr->fa_xflags & FS_XFLAG_PTIME == ignored)
>>         fattr->fa_xflags_mask & FS_XFLAG_PTIME == 1 if user wants to set it, otherwise 0
>>
>
> I prefer the former.
>
>> This would mean that fa_xflags_mask is used like stx_mask both by the fs
>> (to let the user know what it supports (a.k.a. what it's returning)) and
>> by the user (to let the fs know what it's trying to set),
>
> This is actually like stx_attributes_mask not stx_mask.

Ah ok.

>> and fa_xflags
>> is used only for ON/OFF flags (like immutable).
>
> Yes, on/off for flag values (like stx_attributes), but "value exists"
> for field values (like stx_mask).
>
>>
>> Let me know if it's clearer.
>>
>
> Same.

Yes, I think I get it know.

So, just to recap:

fa->xflags_mask == 0 -> legacy behaviour before this series


fa->xflags_mask & FS_XFLAG_ABCD == 1 -> "user wants to set/change abcd"
fa->xflags & FS_XFLAG_ABCD == 1 -> "user wants kernel to take the value from fa->abcd"
fa->xflags & FS_XFLAG_ABCD == 0 -> "user wants kernel to unset abcd"

fa->xflags_mask & FS_XFLAG_ABCD == 0 -> "don't touch abcd"


and in the case of ptime this means:


fa->xflags_mask & FS_XFLAG_PTIME == 1 -> "user wants to set/change ptime"
fa->xflags & FS_XFLAG_PTIME == 0/1 -> "user wants kernel to take the value from fa->ptime" [1]

fa->xflags_mask & FS_XFLAG_PTIME == 0 -> "don't touch ptime"


Correct?

Thanks,
koraynilay

[1]: so basically ignored, it doesn't matter if the user sets xflags | PTIME
or not, but still recommended in documentation. Tho thinking more about
it, maybe it would be better to enforce it at the fs level with
-EOPNOTSUPP.


> Thanks,
> Amir.



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2026-11-09 22:34 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-01  1:55 [RFC PATCH v3 0/5] " koraynilay
2026-11-01  1:55 ` [RFC PATCH v3 1/5] vfs: " koraynilay
2026-11-02 14:00   ` Amir Goldstein
2026-11-03 11:13     ` koraynilay
2026-11-03 15:14       ` Amir Goldstein
2026-11-03 17:27         ` koraynilay
2026-11-07 16:04     ` koraynilay
2026-11-07 16:48       ` Amir Goldstein
2026-11-07 17:09         ` koraynilay
2026-11-09 14:35           ` Amir Goldstein
2026-11-09 22:34             ` koraynilay [this message]
2026-11-01  1:55 ` [RFC PATCH v3 2/5] vfs: add fileattr_has_fsx_mask() koraynilay
2026-11-01  1:55 ` [RFC PATCH v3 3/5] ntfs3: make crtime user-settable using ptime koraynilay
2026-11-02 14:24   ` Pali Rohár
2026-11-02 15:09     ` Amir Goldstein
2026-11-02 15:38       ` Pali Rohár
2026-11-03  1:19         ` Andreas Dilger
2026-11-03  8:23           ` Amir Goldstein
2026-11-03  9:49             ` Pali Rohár
2026-11-03 11:04               ` koraynilay
2026-11-03 11:38                 ` Pali Rohár
2026-11-03 12:11                   ` koraynilay
2026-11-03 12:48                     ` Pali Rohár
2026-11-03 13:37                       ` koraynilay
2026-11-03 15:10               ` Amir Goldstein
2026-11-03 15:25                 ` Pali Rohár
2026-11-03 17:07                   ` koraynilay
2026-11-03 17:28                     ` Pali Rohár
2026-11-03 17:20                 ` koraynilay
2026-11-03 17:32                   ` Pali Rohár
2026-11-03 17:40                     ` koraynilay
2026-11-01  1:55 ` [RFC PATCH v3 4/5] exfat: " koraynilay
2026-11-01  1:55 ` [RFC PATCH v3 5/5] btrfs: add provenance time (ptime) koraynilay
2026-11-02 14:54 ` [RFC PATCH v3 0/5] " Amir Goldstein
2026-11-03 11:24   ` koraynilay
2026-11-03 15:12     ` Amir Goldstein
2026-11-03 17:21       ` koraynilay

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=DMR1E7XB3AT6.3UI3YHHBI8O9Z@gmail.com \
    --to=koray.fra@gmail.com \
    --cc=aalbersh@kernel.org \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=cel@kernel.org \
    --cc=defendthedisabled@gmail.com \
    --cc=djwong@kernel.org \
    --cc=dsterba@suse.com \
    --cc=exfat@lists.linux.dev \
    --cc=jack@suse.cz \
    --cc=linkinjeon@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mason@kernel.org \
    --cc=ntfs3@lists.linux.dev \
    --cc=pali@kernel.org \
    --cc=pfalcato@suse.de \
    --cc=sj1557.seo@samsung.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    --cc=yuezhang.mo@sony.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®