| Message ID | 148966413352.3279.15659219505999889147.stgit@warthog.procyon.org.uk |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mar 16, 2017, at 5:35 AM, David Howells <dhowells@redhat.com> wrote: > > Return enhanced file attributes from the Ext4 filesystem. This includes > the following: > > (1) The inode creation time (i_crtime) as stx_btime, setting STATX_BTIME. > > (2) Certain FS_xxx_FL flags are mapped to stx_attribute flags. > > This requires that all ext4 inodes have a getattr call, not just some of > them, so to this end, split the ext4_getattr() function and only call part > of it where appropriate. > > Example output: > > [root@andromeda ~]# touch foo > [root@andromeda ~]# chattr +ai foo > [root@andromeda ~]# /tmp/test-statx foo > statx(foo) = 0 > results=fff > Size: 0 Blocks: 0 IO Block: 4096 regular file > Device: 08:12 Inode: 2101950 Links: 1 > Access: (0644/-rw-r--r--) Uid: 0 Gid: 0 > Access: 2016-02-11 17:08:29.031795451+0000 > Modify: 2016-02-11 17:08:29.031795451+0000 > Change: 2016-02-11 17:11:11.987790114+0000 > Birth: 2016-02-11 17:08:29.031795451+0000 > Attributes: 0000000000000030 (-------- -------- -------- -------- -------- -------- -------- --ai----) > > Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Andreas Dilger <adilger@dilger.ca> > --- > > fs/ext4/ext4.h | 1 + > fs/ext4/file.c | 2 +- > fs/ext4/inode.c | 36 +++++++++++++++++++++++++++++++++--- > fs/ext4/namei.c | 2 ++ > fs/ext4/symlink.c | 2 ++ > 5 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index f493af666591..fb69ee2388db 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -2466,6 +2466,7 @@ extern int ext4_setattr(struct dentry *, struct iattr *); > extern int ext4_getattr(const struct path *, struct kstat *, u32, unsigned int); > extern void ext4_evict_inode(struct inode *); > extern void ext4_clear_inode(struct inode *); > +extern int ext4_file_getattr(const struct path *, struct kstat *, u32, unsigned int); (style) should this wrap at 80 columns? > extern int ext4_sync_inode(handle_t *, struct inode *); > extern void ext4_dirty_inode(struct inode *, int); > extern int ext4_change_inode_journal_flag(struct inode *, int); > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index 8210c1f43556..cefa9835f275 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -744,7 +744,7 @@ const struct file_operations ext4_file_operations = { > > const struct inode_operations ext4_file_inode_operations = { > .setattr = ext4_setattr, > - .getattr = ext4_getattr, > + .getattr = ext4_file_getattr, > .listxattr = ext4_listxattr, > .get_acl = ext4_get_acl, > .set_acl = ext4_set_acl, > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 7385e6a6b6cb..5450e1eade1d 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -5390,11 +5390,41 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr) > int ext4_getattr(const struct path *path, struct kstat *stat, > u32 request_mask, unsigned int query_flags) > { > - struct inode *inode; > - unsigned long long delalloc_blocks; > + struct inode *inode = d_inode(path->dentry); > + struct ext4_inode *raw_inode; > + struct ext4_inode_info *ei = EXT4_I(inode); > + unsigned int flags; > + > + if (EXT4_FITS_IN_INODE(raw_inode, ei, i_crtime)) { > + stat->result_mask |= STATX_BTIME; > + stat->btime.tv_sec = ei->i_crtime.tv_sec; > + stat->btime.tv_nsec = ei->i_crtime.tv_nsec; > + } > + > + ext4_get_inode_flags(ei); > + flags = ei->i_flags & EXT4_FL_USER_VISIBLE; > + if (flags & EXT4_APPEND_FL) > + stat->attributes |= STATX_ATTR_APPEND; > + if (flags & EXT4_COMPR_FL) > + stat->attributes |= STATX_ATTR_COMPRESSED; > + if (flags & EXT4_ENCRYPT_FL) > + stat->attributes |= STATX_ATTR_ENCRYPTED; > + if (flags & EXT4_IMMUTABLE_FL) > + stat->attributes |= STATX_ATTR_IMMUTABLE; > + if (flags & EXT4_NODUMP_FL) > + stat->attributes |= STATX_ATTR_NODUMP; > > - inode = d_inode(path->dentry); > generic_fillattr(inode, stat); > + return 0; > +} > + > +int ext4_file_getattr(const struct path *path, struct kstat *stat, > + u32 request_mask, unsigned int query_flags) > +{ > + struct inode *inode = d_inode(path->dentry); > + u64 delalloc_blocks; > + > + ext4_getattr(path, stat, request_mask, query_flags); > > /* > * If there is inline data in the inode, the inode will normally not > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 6ad612c576fc..07e5e1405771 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -3912,6 +3912,7 @@ const struct inode_operations ext4_dir_inode_operations = { > .tmpfile = ext4_tmpfile, > .rename = ext4_rename2, > .setattr = ext4_setattr, > + .getattr = ext4_getattr, > .listxattr = ext4_listxattr, > .get_acl = ext4_get_acl, > .set_acl = ext4_set_acl, > @@ -3920,6 +3921,7 @@ const struct inode_operations ext4_dir_inode_operations = { > > const struct inode_operations ext4_special_inode_operations = { > .setattr = ext4_setattr, > + .getattr = ext4_getattr, > .listxattr = ext4_listxattr, > .get_acl = ext4_get_acl, > .set_acl = ext4_set_acl, > diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c > index 73b184d161fc..4c6b23a0603e 100644 > --- a/fs/ext4/symlink.c > +++ b/fs/ext4/symlink.c > @@ -91,11 +91,13 @@ const struct inode_operations ext4_encrypted_symlink_inode_operations = { > const struct inode_operations ext4_symlink_inode_operations = { > .get_link = page_get_link, > .setattr = ext4_setattr, > + .getattr = ext4_getattr, > .listxattr = ext4_listxattr, > }; > > const struct inode_operations ext4_fast_symlink_inode_operations = { > .get_link = simple_get_link, > .setattr = ext4_setattr, > + .getattr = ext4_getattr, > .listxattr = ext4_listxattr, > }; > Cheers, Andreas
Hi David, On Thu, Mar 16, 2017 at 11:35:33AM +0000, David Howells wrote: > + > + ext4_get_inode_flags(ei); > + flags = ei->i_flags & EXT4_FL_USER_VISIBLE; > + if (flags & EXT4_APPEND_FL) > + stat->attributes |= STATX_ATTR_APPEND; > + if (flags & EXT4_COMPR_FL) > + stat->attributes |= STATX_ATTR_COMPRESSED; > + if (flags & EXT4_ENCRYPT_FL) > + stat->attributes |= STATX_ATTR_ENCRYPTED; > + if (flags & EXT4_IMMUTABLE_FL) > + stat->attributes |= STATX_ATTR_IMMUTABLE; > + if (flags & EXT4_NODUMP_FL) > + stat->attributes |= STATX_ATTR_NODUMP; > > - inode = d_inode(path->dentry); > generic_fillattr(inode, stat); > + return 0; > +} I think it's the wrong approach to be calling ext4_get_inode_flags() here to sync the generic inode flags (inode->i_flags) to the ext4-specific inode flags (ei->i_flags). I know the GETFLAGS and FSGETXATTR ioctls do it too, but I think it's a mistake --- at least, when done so without holding the inode lock. The issue is that flag syncs can occur in both directions concurrently and cause an update to be lost. For example, with thread 1 doing a stat() and thread 2 doing the SETFLAGS ioctl to set the APPEND flag: Thread 1, in ext4_get_inode_flags() Thread 2, in ext4_ioctl_setflags() ----------------------------------- ----------------------------------- Read inode->i_flags; S_APPEND is clear Set EXT4_APPEND_FL in ei->i_flags Clear EXT4_APPEND_FL in ei->i_flags Read ei->i_flags; EXT4_APPEND_FL is clear Clear S_APPEND in inode->i_flags So the flag set by SETFLAGS gets lost. This bug probably hasn't really been noticable with GETFLAGS and FSGETXATTR since they're rarely used, but stat() on the other hand is very common, and I'm worried that with this change people would start seeing this race more often. Ultimately this needs to be addressed in ext4 more fully, but how about for ->getattr() just skipping the call to ext4_get_inode_flags() and instead populating the generic attributes like STATX_ATTR_APPEND and STATX_ATTR_IMMUTABLE from the generic inode flags, rather than from the ext4-specific flags? Actually, it could even be done in generic_fillattr(), so that all filesystems benefit. > diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c > index 73b184d161fc..4c6b23a0603e 100644 > --- a/fs/ext4/symlink.c > +++ b/fs/ext4/symlink.c > @@ -91,11 +91,13 @@ const struct inode_operations ext4_encrypted_symlink_inode_operations = { > const struct inode_operations ext4_symlink_inode_operations = { > .get_link = page_get_link, > .setattr = ext4_setattr, > + .getattr = ext4_getattr, > .listxattr = ext4_listxattr, > }; > > const struct inode_operations ext4_fast_symlink_inode_operations = { > .get_link = simple_get_link, > .setattr = ext4_setattr, > + .getattr = ext4_getattr, > .listxattr = ext4_listxattr, > }; > getattr needs to be added to ext4_encrypted_symlink_inode_operations too. - Eric
On Mar 16, 2017, at 11:47 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > On Thu, Mar 16, 2017 at 11:35:33AM +0000, David Howells wrote: >> + >> + ext4_get_inode_flags(ei); >> + flags = ei->i_flags & EXT4_FL_USER_VISIBLE; >> + if (flags & EXT4_APPEND_FL) >> + stat->attributes |= STATX_ATTR_APPEND; >> + if (flags & EXT4_COMPR_FL) >> + stat->attributes |= STATX_ATTR_COMPRESSED; >> + if (flags & EXT4_ENCRYPT_FL) >> + stat->attributes |= STATX_ATTR_ENCRYPTED; >> + if (flags & EXT4_IMMUTABLE_FL) >> + stat->attributes |= STATX_ATTR_IMMUTABLE; >> + if (flags & EXT4_NODUMP_FL) >> + stat->attributes |= STATX_ATTR_NODUMP; >> >> - inode = d_inode(path->dentry); >> generic_fillattr(inode, stat); >> + return 0; >> +} > > I think it's the wrong approach to be calling ext4_get_inode_flags() here to > sync the generic inode flags (inode->i_flags) to the ext4-specific inode flags > (ei->i_flags). I know the GETFLAGS and FSGETXATTR ioctls do it too, but I > think it's a mistake --- at least, when done so without holding the inode lock. > The issue is that flag syncs can occur in both directions concurrently and > cause an update to be lost. For example, with thread 1 doing a stat() and > thread 2 doing the SETFLAGS ioctl to set the APPEND flag: > > Thread 1, ext4_get_inode_flags() Thread 2, ext4_ioctl_setflags() > ----------------------------------- --------------------------- > Read inode->i_flags; S_APPEND clear > Set EXT4_APPEND_FL in ei->i_flags > Clear EXT4_APPEND_FL in ei->i_flags > Read ei->i_flags; EXT4_APPEND_FL clear > Clear S_APPEND in inode->i_flags > > So the flag set by SETFLAGS gets lost. This bug probably hasn't really been > noticable with GETFLAGS and FSGETXATTR since they're rarely used, but stat() on > the other hand is very common, and I'm worried that with this change people > would start seeing this race more often. > > Ultimately this needs to be addressed in ext4 more fully, but how about for > ->getattr() just skipping the call to ext4_get_inode_flags() and instead > populating the generic attributes like STATX_ATTR_APPEND and > STATX_ATTR_IMMUTABLE from the generic inode flags, rather than from the > ext4-specific flags? Actually, it could even be done in generic_fillattr(), so > that all filesystems benefit. Wouldn't it make more sense to just extract the ext4 flags directly from ei->i_flags? I think ext4_get_inode_flags() is only really useful when the VFS inode flags are changed and they need to be propagated to the ext4 inode. I guess the other more significant question is when/where are the VFS inode flags changed that they need to be propagated into the ext4 disk inode? The main avenue for changing these attribute flags that I know about is via EXT4_IOC_SETFLAGS (FS_IOC_SETFLAGS), and there is one place that I could find in fs/nsfs.c that sets S_IMMUTABLE but I don't think that propagates down to an ext4 disk inode. It seems like the use of ext4_get_inode_flags() is largely superfluous. The original commit ff9ddf7e847 indicates this was for quota inodes only. I think it can be removed from EXT4_IOC_FSGETXATTR, and EXT4_IOC_GETFLAGS and made conditional in ext4_do_update_inode(): #ifdef CONFIG_QUOTA for (i = 0; i < EXT4_MAXQUOTAS; i++) { if (unlikely(sb_dqopt(sb)->files[i] == inode)) ext4_get_inode_flags(EXT4_I(inode)); } #endif Jan, what do you think? Cheers, Andreas > >> diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c >> index 73b184d161fc..4c6b23a0603e 100644 >> --- a/fs/ext4/symlink.c >> +++ b/fs/ext4/symlink.c >> @@ -91,11 +91,13 @@ const struct inode_operations ext4_encrypted_symlink_inode_operations = { >> const struct inode_operations ext4_symlink_inode_operations = { >> .get_link = page_get_link, >> .setattr = ext4_setattr, >> + .getattr = ext4_getattr, >> .listxattr = ext4_listxattr, >> }; >> >> const struct inode_operations ext4_fast_symlink_inode_operations = { >> .get_link = simple_get_link, >> .setattr = ext4_setattr, >> + .getattr = ext4_getattr, >> .listxattr = ext4_listxattr, >> }; >> > > getattr needs to be added to ext4_encrypted_symlink_inode_operations too. > > - Eric Cheers, Andreas
On Fri 17-03-17 14:19:22, Andreas Dilger wrote: > On Mar 16, 2017, at 11:47 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > > On Thu, Mar 16, 2017 at 11:35:33AM +0000, David Howells wrote: > >> + > >> + ext4_get_inode_flags(ei); > >> + flags = ei->i_flags & EXT4_FL_USER_VISIBLE; > >> + if (flags & EXT4_APPEND_FL) > >> + stat->attributes |= STATX_ATTR_APPEND; > >> + if (flags & EXT4_COMPR_FL) > >> + stat->attributes |= STATX_ATTR_COMPRESSED; > >> + if (flags & EXT4_ENCRYPT_FL) > >> + stat->attributes |= STATX_ATTR_ENCRYPTED; > >> + if (flags & EXT4_IMMUTABLE_FL) > >> + stat->attributes |= STATX_ATTR_IMMUTABLE; > >> + if (flags & EXT4_NODUMP_FL) > >> + stat->attributes |= STATX_ATTR_NODUMP; > >> > >> - inode = d_inode(path->dentry); > >> generic_fillattr(inode, stat); > >> + return 0; > >> +} > > > > I think it's the wrong approach to be calling ext4_get_inode_flags() here to > > sync the generic inode flags (inode->i_flags) to the ext4-specific inode flags > > (ei->i_flags). I know the GETFLAGS and FSGETXATTR ioctls do it too, but I > > think it's a mistake --- at least, when done so without holding the inode lock. > > The issue is that flag syncs can occur in both directions concurrently and > > cause an update to be lost. For example, with thread 1 doing a stat() and > > thread 2 doing the SETFLAGS ioctl to set the APPEND flag: > > > > Thread 1, ext4_get_inode_flags() Thread 2, ext4_ioctl_setflags() > > ----------------------------------- --------------------------- > > Read inode->i_flags; S_APPEND clear > > Set EXT4_APPEND_FL in ei->i_flags > > Clear EXT4_APPEND_FL in ei->i_flags > > Read ei->i_flags; EXT4_APPEND_FL clear > > Clear S_APPEND in inode->i_flags > > > > So the flag set by SETFLAGS gets lost. This bug probably hasn't really been > > noticable with GETFLAGS and FSGETXATTR since they're rarely used, but stat() on > > the other hand is very common, and I'm worried that with this change people > > would start seeing this race more often. > > > > Ultimately this needs to be addressed in ext4 more fully, but how about for > > ->getattr() just skipping the call to ext4_get_inode_flags() and instead > > populating the generic attributes like STATX_ATTR_APPEND and > > STATX_ATTR_IMMUTABLE from the generic inode flags, rather than from the > > ext4-specific flags? Actually, it could even be done in generic_fillattr(), so > > that all filesystems benefit. > > Wouldn't it make more sense to just extract the ext4 flags directly from > ei->i_flags? I think ext4_get_inode_flags() is only really useful when > the VFS inode flags are changed and they need to be propagated to the ext4 > inode. > > I guess the other more significant question is when/where are the VFS inode > flags changed that they need to be propagated into the ext4 disk inode? > The main avenue for changing these attribute flags that I know about is via > EXT4_IOC_SETFLAGS (FS_IOC_SETFLAGS), and there is one place that I could > find in fs/nsfs.c that sets S_IMMUTABLE but I don't think that propagates > down to an ext4 disk inode. Yes, you seem to be right. And actually I have checked and XFS does not bother to copy inode->i_flags to its on-disk flags so it seems generally we are not expected to reflect inode->i_flags in on-disk state. > It seems like the use of ext4_get_inode_flags() is largely superfluous. > The original commit ff9ddf7e847 indicates this was for quota inodes only. > I think it can be removed from EXT4_IOC_FSGETXATTR, and EXT4_IOC_GETFLAGS > and made conditional in ext4_do_update_inode(): > > #ifdef CONFIG_QUOTA > for (i = 0; i < EXT4_MAXQUOTAS; i++) { > if (unlikely(sb_dqopt(sb)->files[i] == inode)) > ext4_get_inode_flags(EXT4_I(inode)); > } > #endif > > Jan, what do you think? So I think even better would be to have ext4_quota_on() and ext4_quota_off() just update the flags as needed and avoid doing it anywhere else... I'll have a look into it. Honza
On Thu, Mar 16, 2017 at 11:35:33AM +0000, David Howells wrote: > Return enhanced file attributes from the Ext4 filesystem. This includes > the following: Just as the comment to a similar patch from Darrick for XFS: Please add test cases that verify this information. Especially in the light of the races mentioned in the discussion later. If we can't get tests for statx before 4.11 is getting close to release I fear we'll have to revert it - untested syscalls have a tendency to be broken and create problems for the future.
Eric Biggers <ebiggers3@gmail.com> wrote: > Ultimately this needs to be addressed in ext4 more fully, but how about for > ->getattr() just skipping the call to ext4_get_inode_flags() and instead > populating the generic attributes like STATX_ATTR_APPEND and > STATX_ATTR_IMMUTABLE from the generic inode flags, rather than from the > ext4-specific flags? Actually, it could even be done in generic_fillattr(), so > that all filesystems benefit. For the moment, taking Andreas's comments into account, I'll just drop the call to ext4_get_inode_flags() and just assume ei->i_flags is correct. I'm not sure whether to push the APPEND and IMMUTABLE attribute transfers to the generic code - it makes sense for all filesystems that support such flags and not for those that don't:-/ And, true, the same could be said of the AUTOMMOUNT attribute. David
On Sun 19-03-17 12:12:37, Jan Kara wrote: > On Fri 17-03-17 14:19:22, Andreas Dilger wrote: > > On Mar 16, 2017, at 11:47 PM, Eric Biggers <ebiggers3@gmail.com> wrote: > > > On Thu, Mar 16, 2017 at 11:35:33AM +0000, David Howells wrote: > > >> + > > >> + ext4_get_inode_flags(ei); > > >> + flags = ei->i_flags & EXT4_FL_USER_VISIBLE; > > >> + if (flags & EXT4_APPEND_FL) > > >> + stat->attributes |= STATX_ATTR_APPEND; > > >> + if (flags & EXT4_COMPR_FL) > > >> + stat->attributes |= STATX_ATTR_COMPRESSED; > > >> + if (flags & EXT4_ENCRYPT_FL) > > >> + stat->attributes |= STATX_ATTR_ENCRYPTED; > > >> + if (flags & EXT4_IMMUTABLE_FL) > > >> + stat->attributes |= STATX_ATTR_IMMUTABLE; > > >> + if (flags & EXT4_NODUMP_FL) > > >> + stat->attributes |= STATX_ATTR_NODUMP; > > >> > > >> - inode = d_inode(path->dentry); > > >> generic_fillattr(inode, stat); > > >> + return 0; > > >> +} > > > > > > I think it's the wrong approach to be calling ext4_get_inode_flags() here to > > > sync the generic inode flags (inode->i_flags) to the ext4-specific inode flags > > > (ei->i_flags). I know the GETFLAGS and FSGETXATTR ioctls do it too, but I > > > think it's a mistake --- at least, when done so without holding the inode lock. > > > The issue is that flag syncs can occur in both directions concurrently and > > > cause an update to be lost. For example, with thread 1 doing a stat() and > > > thread 2 doing the SETFLAGS ioctl to set the APPEND flag: > > > > > > Thread 1, ext4_get_inode_flags() Thread 2, ext4_ioctl_setflags() > > > ----------------------------------- --------------------------- > > > Read inode->i_flags; S_APPEND clear > > > Set EXT4_APPEND_FL in ei->i_flags > > > Clear EXT4_APPEND_FL in ei->i_flags > > > Read ei->i_flags; EXT4_APPEND_FL clear > > > Clear S_APPEND in inode->i_flags > > > > > > So the flag set by SETFLAGS gets lost. This bug probably hasn't really been > > > noticable with GETFLAGS and FSGETXATTR since they're rarely used, but stat() on > > > the other hand is very common, and I'm worried that with this change people > > > would start seeing this race more often. > > > > > > Ultimately this needs to be addressed in ext4 more fully, but how about for > > > ->getattr() just skipping the call to ext4_get_inode_flags() and instead > > > populating the generic attributes like STATX_ATTR_APPEND and > > > STATX_ATTR_IMMUTABLE from the generic inode flags, rather than from the > > > ext4-specific flags? Actually, it could even be done in generic_fillattr(), so > > > that all filesystems benefit. > > > > Wouldn't it make more sense to just extract the ext4 flags directly from > > ei->i_flags? I think ext4_get_inode_flags() is only really useful when > > the VFS inode flags are changed and they need to be propagated to the ext4 > > inode. > > > > I guess the other more significant question is when/where are the VFS inode > > flags changed that they need to be propagated into the ext4 disk inode? > > The main avenue for changing these attribute flags that I know about is via > > EXT4_IOC_SETFLAGS (FS_IOC_SETFLAGS), and there is one place that I could > > find in fs/nsfs.c that sets S_IMMUTABLE but I don't think that propagates > > down to an ext4 disk inode. > > Yes, you seem to be right. And actually I have checked and XFS does not > bother to copy inode->i_flags to its on-disk flags so it seems generally we > are not expected to reflect inode->i_flags in on-disk state. > > > It seems like the use of ext4_get_inode_flags() is largely superfluous. > > The original commit ff9ddf7e847 indicates this was for quota inodes only. > > I think it can be removed from EXT4_IOC_FSGETXATTR, and EXT4_IOC_GETFLAGS > > and made conditional in ext4_do_update_inode(): > > > > #ifdef CONFIG_QUOTA > > for (i = 0; i < EXT4_MAXQUOTAS; i++) { > > if (unlikely(sb_dqopt(sb)->files[i] == inode)) > > ext4_get_inode_flags(EXT4_I(inode)); > > } > > #endif > > > > Jan, what do you think? > > So I think even better would be to have ext4_quota_on() and > ext4_quota_off() just update the flags as needed and avoid doing it > anywhere else... I'll have a look into it. FYI I have just sent out patches to do this... Honza
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index f493af666591..fb69ee2388db 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -2466,6 +2466,7 @@ extern int ext4_setattr(struct dentry *, struct iattr *); extern int ext4_getattr(const struct path *, struct kstat *, u32, unsigned int); extern void ext4_evict_inode(struct inode *); extern void ext4_clear_inode(struct inode *); +extern int ext4_file_getattr(const struct path *, struct kstat *, u32, unsigned int); extern int ext4_sync_inode(handle_t *, struct inode *); extern void ext4_dirty_inode(struct inode *, int); extern int ext4_change_inode_journal_flag(struct inode *, int); diff --git a/fs/ext4/file.c b/fs/ext4/file.c index 8210c1f43556..cefa9835f275 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -744,7 +744,7 @@ const struct file_operations ext4_file_operations = { const struct inode_operations ext4_file_inode_operations = { .setattr = ext4_setattr, - .getattr = ext4_getattr, + .getattr = ext4_file_getattr, .listxattr = ext4_listxattr, .get_acl = ext4_get_acl, .set_acl = ext4_set_acl, diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 7385e6a6b6cb..5450e1eade1d 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -5390,11 +5390,41 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr) int ext4_getattr(const struct path *path, struct kstat *stat, u32 request_mask, unsigned int query_flags) { - struct inode *inode; - unsigned long long delalloc_blocks; + struct inode *inode = d_inode(path->dentry); + struct ext4_inode *raw_inode; + struct ext4_inode_info *ei = EXT4_I(inode); + unsigned int flags; + + if (EXT4_FITS_IN_INODE(raw_inode, ei, i_crtime)) { + stat->result_mask |= STATX_BTIME; + stat->btime.tv_sec = ei->i_crtime.tv_sec; + stat->btime.tv_nsec = ei->i_crtime.tv_nsec; + } + + ext4_get_inode_flags(ei); + flags = ei->i_flags & EXT4_FL_USER_VISIBLE; + if (flags & EXT4_APPEND_FL) + stat->attributes |= STATX_ATTR_APPEND; + if (flags & EXT4_COMPR_FL) + stat->attributes |= STATX_ATTR_COMPRESSED; + if (flags & EXT4_ENCRYPT_FL) + stat->attributes |= STATX_ATTR_ENCRYPTED; + if (flags & EXT4_IMMUTABLE_FL) + stat->attributes |= STATX_ATTR_IMMUTABLE; + if (flags & EXT4_NODUMP_FL) + stat->attributes |= STATX_ATTR_NODUMP; - inode = d_inode(path->dentry); generic_fillattr(inode, stat); + return 0; +} + +int ext4_file_getattr(const struct path *path, struct kstat *stat, + u32 request_mask, unsigned int query_flags) +{ + struct inode *inode = d_inode(path->dentry); + u64 delalloc_blocks; + + ext4_getattr(path, stat, request_mask, query_flags); /* * If there is inline data in the inode, the inode will normally not diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 6ad612c576fc..07e5e1405771 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -3912,6 +3912,7 @@ const struct inode_operations ext4_dir_inode_operations = { .tmpfile = ext4_tmpfile, .rename = ext4_rename2, .setattr = ext4_setattr, + .getattr = ext4_getattr, .listxattr = ext4_listxattr, .get_acl = ext4_get_acl, .set_acl = ext4_set_acl, @@ -3920,6 +3921,7 @@ const struct inode_operations ext4_dir_inode_operations = { const struct inode_operations ext4_special_inode_operations = { .setattr = ext4_setattr, + .getattr = ext4_getattr, .listxattr = ext4_listxattr, .get_acl = ext4_get_acl, .set_acl = ext4_set_acl, diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c index 73b184d161fc..4c6b23a0603e 100644 --- a/fs/ext4/symlink.c +++ b/fs/ext4/symlink.c @@ -91,11 +91,13 @@ const struct inode_operations ext4_encrypted_symlink_inode_operations = { const struct inode_operations ext4_symlink_inode_operations = { .get_link = page_get_link, .setattr = ext4_setattr, + .getattr = ext4_getattr, .listxattr = ext4_listxattr, }; const struct inode_operations ext4_fast_symlink_inode_operations = { .get_link = simple_get_link, .setattr = ext4_setattr, + .getattr = ext4_getattr, .listxattr = ext4_listxattr, };
Return enhanced file attributes from the Ext4 filesystem. This includes the following: (1) The inode creation time (i_crtime) as stx_btime, setting STATX_BTIME. (2) Certain FS_xxx_FL flags are mapped to stx_attribute flags. This requires that all ext4 inodes have a getattr call, not just some of them, so to this end, split the ext4_getattr() function and only call part of it where appropriate. Example output: [root@andromeda ~]# touch foo [root@andromeda ~]# chattr +ai foo [root@andromeda ~]# /tmp/test-statx foo statx(foo) = 0 results=fff Size: 0 Blocks: 0 IO Block: 4096 regular file Device: 08:12 Inode: 2101950 Links: 1 Access: (0644/-rw-r--r--) Uid: 0 Gid: 0 Access: 2016-02-11 17:08:29.031795451+0000 Modify: 2016-02-11 17:08:29.031795451+0000 Change: 2016-02-11 17:11:11.987790114+0000 Birth: 2016-02-11 17:08:29.031795451+0000 Attributes: 0000000000000030 (-------- -------- -------- -------- -------- -------- -------- --ai----) Signed-off-by: David Howells <dhowells@redhat.com> --- fs/ext4/ext4.h | 1 + fs/ext4/file.c | 2 +- fs/ext4/inode.c | 36 +++++++++++++++++++++++++++++++++--- fs/ext4/namei.c | 2 ++ fs/ext4/symlink.c | 2 ++ 5 files changed, 39 insertions(+), 4 deletions(-)