All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Dilger <adilger@dilger.ca>
To: David Howells <dhowells@redhat.com>
Cc: linux-fsdevel@vger.kernel.org, linux-afs@vger.kernel.org,
	linux-nfs@vger.kernel.org, samba-technical@lists.samba.org,
	linux-kernel@vger.kernel.org, linux-ext4@vger.kernel.org
Subject: Re: [PATCH 3/6] statx: Ext4: Return enhanced file attributes
Date: Mon, 2 May 2016 16:48:13 -0600	[thread overview]
Message-ID: <ABA29DAB-BDD2-44E9-8E2B-AE227A823ECD@dilger.ca> (raw)
In-Reply-To: <20160429125759.23636.10635.stgit@warthog.procyon.org.uk>

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

On Apr 29, 2016, at 6:57 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 i_btime, setting STATX_BTIME.
> 
> (2) The inode i_version as st_version if a file with I_VERSION set or a
>     directory, setting STATX_VERSION.
> 
> (3) FS_xxx_FL flags are returned as for ioctl(FS_IOC_GETFLAGS), setting
>     STATX_IOC_FLAGS.

I don't see where this is implemented in this patch, and it should be
removed from the commit message to avoid confusion.

> 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 ~]# ./samples/statx/test-statx /usr
> 	statx(/usr) = 0
> 	results=37ef
> 	  Size: 4096            Blocks: 16         IO Block: 4096    directory
> 	Device: 08:02           Inode: 1572865     Links: 14
> 	Access: (0755/drwxr-xr-x)  Uid:     0   Gid:     0
> 	Access: 2015-11-03 16:12:30.000000000+0000
> 	Modify: 2013-10-18 15:29:18.000000000+0100
> 	Change: 2013-10-18 15:29:18.000000000+0100
> 	Data version: 2fh
> 	Inode flags: 00000000 (-------- -------- -------- --------)
> 	IO-blocksize: blksize=4096

One minor strangeness in this commit message is that the above usage example
doesn't include nanoseconds or Birth time for ext4, when this should be
supported for all ext4 filesystems?  Was this example run on an older ext3
filesystem (or some filesystem upgraded from ext3)?  Any ext4-formatted fs
should have the larger inode size to handle these extra fields.

Also a bit strange that "IO Block: 4096" and "IO-blocksize: blksize=4096"
is listed twice, but that seems to be an issue with the test-statx tool
and not necessarily the data being returned.

Cheers, Andreas

> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
> fs/ext4/ext4.h    |    2 ++
> fs/ext4/file.c    |    2 +-
> fs/ext4/inode.c   |   30 +++++++++++++++++++++++++++---
> fs/ext4/namei.c   |    2 ++
> fs/ext4/symlink.c |    2 ++
> 5 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 349afebe21ee..2f25eaa63f39 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2549,6 +2549,8 @@ extern int  ext4_getattr(struct vfsmount *mnt, struct dentry *dentry,
> 				struct kstat *stat);
> extern void ext4_evict_inode(struct inode *);
> extern void ext4_clear_inode(struct inode *);
> +extern int  ext4_file_getattr(struct vfsmount *mnt, struct dentry *dentry,
> +			      struct kstat *stat);
> 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 fa2208bae2e1..45c7b8644d0e 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -715,7 +715,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,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 981a1fc30eaa..309b6cff8afc 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5100,11 +5100,35 @@ err_out:
> int ext4_getattr(struct vfsmount *mnt, struct dentry *dentry,
> 		 struct kstat *stat)
> {
> -	struct inode *inode;
> -	unsigned long long delalloc_blocks;
> +	struct inode *inode = d_inode(dentry);
> +	struct ext4_inode *raw_inode;
> +	struct ext4_inode_info *ei = EXT4_I(inode);
> +
> +	stat->result_mask |= STATX_GEN;
> +	stat->gen = inode->i_generation;
> +
> +	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;
> +	}
> +
> +	if (S_ISDIR(inode->i_mode) || IS_I_VERSION(inode)) {
> +		stat->result_mask |= STATX_VERSION;
> +		stat->version = inode->i_version;
> +	}
> 
> -	inode = d_inode(dentry);
> 	generic_fillattr(inode, stat);
> +	return 0;
> +}
> +
> +int ext4_file_getattr(struct vfsmount *mnt, struct dentry *dentry,
> +		      struct kstat *stat)
> +{
> +	struct inode *inode = dentry->d_inode;
> +	u64 delalloc_blocks;
> +
> +	ext4_getattr(mnt, dentry, stat);
> 
> 	/*
> 	 * 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 48e4b8907826..9b5fdc971e22 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3882,6 +3882,7 @@ const struct inode_operations ext4_dir_inode_operations = {
> 	.tmpfile	= ext4_tmpfile,
> 	.rename2	= ext4_rename2,
> 	.setattr	= ext4_setattr,
> +	.getattr	= ext4_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> @@ -3893,6 +3894,7 @@ const struct inode_operations ext4_dir_inode_operations = {
> 
> const struct inode_operations ext4_special_inode_operations = {
> 	.setattr	= ext4_setattr,
> +	.getattr	= ext4_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c
> index 75ed5c2f0c16..54015f3d7516 100644
> --- a/fs/ext4/symlink.c
> +++ b/fs/ext4/symlink.c
> @@ -105,6 +105,7 @@ const struct inode_operations ext4_symlink_inode_operations = {
> 	.readlink	= generic_readlink,
> 	.get_link	= page_get_link,
> 	.setattr	= ext4_setattr,
> +	.getattr	= ext4_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> @@ -115,6 +116,7 @@ const struct inode_operations ext4_fast_symlink_inode_operations = {
> 	.readlink	= generic_readlink,
> 	.get_link	= simple_get_link,
> 	.setattr	= ext4_setattr,
> +	.getattr	= ext4_getattr,
> 	.setxattr	= generic_setxattr,
> 	.getxattr	= generic_getxattr,
> 	.listxattr	= ext4_listxattr,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2016-05-02 22:48 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-29 12:57 [RFC][PATCH 0/6] Enhanced file stat system call David Howells
2016-04-29 12:57 ` [PATCH 1/6] statx: Add a system call to make enhanced file info available David Howells
2016-05-02 22:46   ` Andreas Dilger
2016-05-02 22:46     ` Andreas Dilger
2016-05-03 15:53   ` David Howells
2016-05-04 22:56   ` Dave Chinner
2016-05-05  0:09     ` NeilBrown
2016-05-05  0:09       ` NeilBrown
2016-05-05 19:48       ` Jeff Layton
2016-05-06 18:07         ` J. Bruce Fields
2016-05-06 18:07           ` J. Bruce Fields
2016-05-05 20:04       ` David Howells
2016-05-05 20:04         ` David Howells
2016-05-06  1:39         ` Dave Chinner
2016-05-06  1:39           ` Dave Chinner
2016-05-06  1:39           ` Dave Chinner
2016-05-06 18:29     ` J. Bruce Fields
2016-05-09  1:45       ` Dave Chinner
2016-05-09  2:46         ` J. Bruce Fields
2016-05-04 23:56   ` NeilBrown
2016-05-08  8:35   ` Christoph Hellwig
2016-05-08  8:35     ` Christoph Hellwig
2016-05-09 12:02     ` Jeff Layton
2016-05-09 12:02       ` Jeff Layton
2016-05-10  7:00       ` Christoph Hellwig
2016-05-10  7:00         ` Christoph Hellwig
2016-05-10 13:21         ` Jeff Layton
2016-05-10 13:21           ` Jeff Layton
2016-05-09 12:57   ` David Howells
2016-05-09 12:57     ` David Howells
2016-05-09 13:23     ` Trond Myklebust
2016-05-09 13:23       ` Trond Myklebust
2016-05-09 13:23       ` Trond Myklebust
2016-05-10  7:04     ` Christoph Hellwig
2016-05-10  8:25     ` David Howells
2016-05-12  9:11       ` Christoph Hellwig
2016-05-13 15:28         ` Arnd Bergmann
2016-05-13 15:28           ` Arnd Bergmann
2016-05-23  8:22           ` Christoph Hellwig
2016-05-23  9:33           ` David Howells
2016-05-18 10:55         ` David Howells
2016-05-09 13:00   ` David Howells
2016-05-09 13:00     ` David Howells
2016-05-09 13:38   ` David Howells
2016-05-10  7:08     ` Christoph Hellwig
2016-05-10  8:43     ` David Howells
2016-05-12  9:12       ` Christoph Hellwig
2016-05-09 13:40   ` David Howells
2016-04-29 12:57 ` [PATCH 2/6] statx: AFS: Return enhanced file attributes David Howells
2016-04-29 12:57 ` [PATCH 3/6] statx: Ext4: " David Howells
2016-05-02 22:48   ` Andreas Dilger [this message]
2016-05-03 20:24   ` David Howells
2016-05-03 20:24     ` David Howells
2016-05-08  8:38   ` Christoph Hellwig
2016-05-08  8:38     ` Christoph Hellwig
2016-04-29 12:58 ` [PATCH 4/6] statx: NFS: " David Howells
2016-05-02 22:48   ` Andreas Dilger
2016-04-29 12:58 ` [PATCH 5/6] statx: Make windows attributes available for CIFS, NTFS and FAT to use David Howells
2016-05-02 22:52   ` Andreas Dilger
2016-10-03 21:03     ` Steve French
2016-10-03 21:03       ` Steve French
2016-05-03 20:23   ` David Howells
2016-05-08  8:39   ` Christoph Hellwig
2016-05-08  8:39     ` Christoph Hellwig
2016-04-29 12:58 ` [PATCH 6/6] statx: CIFS: Return enhanced attributes David Howells
2016-04-30 21:05 ` [RFC][PATCH 0/6] Enhanced file stat system call Jeff Layton
2016-04-30 21:05   ` Jeff Layton
2016-05-04 13:46 ` Arnd Bergmann
2016-05-04 13:46   ` Arnd Bergmann
2016-05-05 22:54   ` Steve French
2016-05-06  2:00     ` Steve French
2016-05-09 13:09       ` Arnd Bergmann
2016-05-09 13:09         ` Arnd Bergmann
2016-05-13 14:28         ` Richard Sharpe
2016-05-13 14:28           ` Richard Sharpe
2016-05-13 15:08           ` Arnd Bergmann

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=ABA29DAB-BDD2-44E9-8E2B-AE227A823ECD@dilger.ca \
    --to=adilger@dilger.ca \
    --cc=dhowells@redhat.com \
    --cc=linux-afs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=samba-technical@lists.samba.org \
    /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 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.