| Message ID | 20220211213628.GA1919658@xavier-xps (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] tmpfs: support for file creation time | expand |
On Mon, Feb 28, 2022 at 12:43 AM Xavier Roche <xavier.roche@algolia.com> wrote: > > Various filesystems (including ext4) now support file creation time. > This patch adds such support for tmpfs-based filesystems. What's the odd huge-page noise in this patch? Other oddities: Reply-To: b954973a-b8d1-cab8-63bd-6ea8063de3@google.com WHAT? And finally - if we really want to treat btime as a first-class entity and expect things like tmpfs to support it, then we should just bite the bullet and put it in 'struct inode' along with the other times. Linus
On Wed, 2 Mar 2022 11:43:02 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Mon, Feb 28, 2022 at 12:43 AM Xavier Roche <xavier.roche@algolia.com> wrote: > > > > Various filesystems (including ext4) now support file creation time. > > This patch adds such support for tmpfs-based filesystems. > > What's the odd huge-page noise in this patch? I can't see such changes? This v3 patch is the v2 patch plus Hugh's changes (https://lkml.kernel.org/r/b954973a-b8d1-cab8-63bd-6ea8063de3@google.com). I won't be merging v3 because this changelog lacks appropriate decription of Hugh's changes and lacks a Link: tag to Hugh's change.
On Wed, 2 Mar 2022, Linus Torvalds wrote: > On Mon, Feb 28, 2022 at 12:43 AM Xavier Roche <xavier.roche@algolia.com> wrote: > > > > Various filesystems (including ext4) now support file creation time. > > This patch adds such support for tmpfs-based filesystems. Please ignore this patch for now: I presume Xavier did not understand the "from akpm to Linus in next merge window" flow, and thought he had to resend the patch to you. > > What's the odd huge-page noise in this patch? That was one of the fixups I added, which akpm is holding in a -fix patch to be merged in, and maybe he'll include my comment from it: 3. Using shmem_getattr() on other file types than regular requires that shmem_is_huge() check type, to stop incorrect HPAGE_PMD_SIZE blksize. (shmem_getattr() was only on regular files before: Xavier's patch added it to directories etc, to provide btime for them too; but shmem_getattr() can also include a dubious adjustment to blksize.) > > Other oddities: > > Reply-To: b954973a-b8d1-cab8-63bd-6ea8063de3@google.com > > WHAT? No comment from me. > > And finally - if we really want to treat btime as a first-class entity > and expect things like tmpfs to support it, then we should just bite > the bullet and put it in 'struct inode' along with the other times. I've no objection if someone does that later. I've no investment in btime myself (my instinctive reaction was, is this thing worth another 16 bytes of inode space?), but Xavier thought it worth doing, and Andrew worth taking, so go with the flow. Ah, Andrew has replied meanwhile, I hope I'm not contradicting! Hugh
On Wed, Mar 02, 2022 at 12:17:30PM -0800, Hugh Dickins wrote: > Please ignore this patch for now: I presume Xavier did not understand > the "from akpm to Linus in next merge window" flow, and thought he had > to resend the patch to you. I will resend a fixed v4 version in a moment, sorry for the noise (and I indeed did not fully understand the flow). > > And finally - if we really want to treat btime as a first-class entity > > and expect things like tmpfs to support it, then we should just bite > > the bullet and put it in 'struct inode' along with the other times. > I've no objection if someone does that later. I might give it a try if this is something that can be of interest. The idea of having btime in 'struct inode' would make the btime a first-class citizen, allowing to have more consistent (w.r.t filesystem types) behavior. This would also mean allowing to _change_ it, typically to allow archivers to set the creation time as they do for {a,c,m}time. Currently, birth time semantic is bound to the current filesystem's life cycle and as such is irrelevant after a restore, or a 'tar xf'. The only gray area to me is whether or not we "can" always change this property without unforeseen consequences, typically for ext4.
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h index e65b80ed09e7..29787767c3b9 100644 --- a/include/linux/shmem_fs.h +++ b/include/linux/shmem_fs.h @@ -25,6 +25,7 @@ struct shmem_inode_info { struct simple_xattrs xattrs; /* list of xattrs */ atomic_t stop_eviction; /* hold when working on inode */ struct inode vfs_inode; + struct timespec64 i_crtime; /* file creation time */ }; struct shmem_sb_info { diff --git a/mm/shmem.c b/mm/shmem.c index a09b29ec2b45..5a3907712c4f 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -1061,6 +1061,12 @@ static int shmem_getattr(struct user_namespace *mnt_userns, if (shmem_is_huge(NULL, inode, 0)) stat->blksize = HPAGE_PMD_SIZE; + if ((request_mask & STATX_BTIME)) { + stat->result_mask |= STATX_BTIME; + stat->btime.tv_sec = info->i_crtime.tv_sec; + stat->btime.tv_nsec = info->i_crtime.tv_nsec; + } + return 0; } @@ -2265,6 +2271,7 @@ static struct inode *shmem_get_inode(struct super_block *sb, const struct inode atomic_set(&info->stop_eviction, 0); info->seals = F_SEAL_SEAL; info->flags = flags & VM_NORESERVE; + info->i_crtime = inode->i_mtime; INIT_LIST_HEAD(&info->shrinklist); INIT_LIST_HEAD(&info->swaplist); simple_xattrs_init(&info->xattrs); @@ -3196,6 +3203,7 @@ static ssize_t shmem_listxattr(struct dentry *dentry, char *buffer, size_t size) #endif /* CONFIG_TMPFS_XATTR */ static const struct inode_operations shmem_short_symlink_operations = { + .getattr = shmem_getattr, .get_link = simple_get_link, #ifdef CONFIG_TMPFS_XATTR .listxattr = shmem_listxattr, @@ -3203,6 +3211,7 @@ static const struct inode_operations shmem_short_symlink_operations = { }; static const struct inode_operations shmem_symlink_inode_operations = { + .getattr = shmem_getattr, .get_link = shmem_get_link, #ifdef CONFIG_TMPFS_XATTR .listxattr = shmem_listxattr, @@ -3790,6 +3799,7 @@ static const struct inode_operations shmem_inode_operations = { static const struct inode_operations shmem_dir_inode_operations = { #ifdef CONFIG_TMPFS + .getattr = shmem_getattr, .create = shmem_create, .lookup = simple_lookup, .link = shmem_link, @@ -3811,6 +3821,7 @@ static const struct inode_operations shmem_dir_inode_operations = { }; static const struct inode_operations shmem_special_inode_operations = { + .getattr = shmem_getattr, #ifdef CONFIG_TMPFS_XATTR .listxattr = shmem_listxattr, #endif