[PATCH v2 0/3] xfs_io: enable extsize and stat -v support for ext4

[PATCH v2 0/3] xfs_io: enable extsize and stat -v support for ext4

[Ojaswin Mujoo]

Changes since patch v1 [2]

  * Patch 1:
    - Remove extra brackets 
    - RVB from Christoph

Changes since rfc [1]

 * Patch 1: Don't add ext4 specific checks
 * Patch 2: 
   - Rather than adding a check for ext4, allow FS_IOC_GETXATTR to be
     called by any FS that supports it.

** Original Cover **

With ext4 extsize hints support being worked on, enable extsize 
command to be run on FSes other than xfs so ext4 can utilize it.
Also extend stat -v to perform FS_IOC_FSGETXATTR ioctl on ext4.

No funtional changes are intended for XFS.

[1]
https://lore.kernel.org/linux-ext4/cover.1733902742.git.ojaswin@linux.ibm.com/T/#t

Ojaswin Mujoo (3):
  include/linux.h: use linux/magic.h to get XFS_SUPER_MAGIC
  xfs_io: allow foreign FSes to show FS_IOC_FSGETXATTR details
  xfs_io: add extsize command support

 include/linux.h |  3 ++-
 io/open.c       |  2 +-
 io/stat.c       | 63 ++++++++++++++++++++++++++++---------------------
 3 files changed, 39 insertions(+), 29 deletions(-)

-- 
2.43.5

[PATCH v2 1/3] include/linux.h: use linux/magic.h to get XFS_SUPER_MAGIC

[Ojaswin Mujoo]

This avoids open coding the magic number

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 include/linux.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux.h b/include/linux.h
index e9eb7bfb26a1..b3516d54c51b 100644
--- a/include/linux.h
+++ b/include/linux.h
@@ -37,6 +37,7 @@
 #endif
 #include <unistd.h>
 #include <assert.h>
+#include <linux/magic.h> /* super block magic numbers */
 
 static __inline__ int xfsctl(const char *path, int fd, int cmd, void *p)
 {
@@ -60,7 +61,7 @@ static __inline__ int platform_test_xfs_fd(int fd)
 		return 0;
 	if (!S_ISREG(statbuf.st_mode) && !S_ISDIR(statbuf.st_mode))
 		return 0;
-	return (statfsbuf.f_type == 0x58465342);	/* XFSB */
+	return statfsbuf.f_type == XFS_SUPER_MAGIC;
 }
 
 static __inline__ int platform_test_xfs_path(const char *path)
-- 
2.43.5

Re: [PATCH v2 1/3] include/linux.h: use linux/magic.h to get XFS_SUPER_MAGIC

[Darrick J. Wong]

On Thu, Dec 19, 2024 at 06:09:13PM +0530, Ojaswin Mujoo wrote:
> This avoids open coding the magic number
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Looks reasonable,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  include/linux.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux.h b/include/linux.h
> index e9eb7bfb26a1..b3516d54c51b 100644
> --- a/include/linux.h
> +++ b/include/linux.h
> @@ -37,6 +37,7 @@
>  #endif
>  #include <unistd.h>
>  #include <assert.h>
> +#include <linux/magic.h> /* super block magic numbers */
>  
>  static __inline__ int xfsctl(const char *path, int fd, int cmd, void *p)
>  {
> @@ -60,7 +61,7 @@ static __inline__ int platform_test_xfs_fd(int fd)
>  		return 0;
>  	if (!S_ISREG(statbuf.st_mode) && !S_ISDIR(statbuf.st_mode))
>  		return 0;
> -	return (statfsbuf.f_type == 0x58465342);	/* XFSB */
> +	return statfsbuf.f_type == XFS_SUPER_MAGIC;
>  }
>  
>  static __inline__ int platform_test_xfs_path(const char *path)
> -- 
> 2.43.5
> 
> 

[PATCH v2 2/3] xfs_io: allow foreign FSes to show FS_IOC_FSGETXATTR details

[Ojaswin Mujoo]

Currently with stat we only show FS_IOC_FSGETXATTR details if the
filesystem is XFS. With extsize support also coming to ext4 and possibly
other filesystems, make sure to allow foreign FSes to display these details
when "stat" or "statx" is used.

(Thanks to Dave for suggesting implementation of print_extended_info())

Suggested-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 io/stat.c | 63 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 36 insertions(+), 27 deletions(-)

diff --git a/io/stat.c b/io/stat.c
index 326f2822e276..3ce3308d0562 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -98,30 +98,45 @@ print_file_info(void)
 }
 
 static void
-print_xfs_info(int verbose)
+print_extended_info(int verbose)
 {
-	struct dioattr	dio;
-	struct fsxattr	fsx, fsxa;
-
-	if ((xfsctl(file->name, file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0 ||
-	    (xfsctl(file->name, file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
-		perror("FS_IOC_FSGETXATTR");
-	} else {
-		printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
-		printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1);
-		printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid);
-		printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize);
-		printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize);
-		printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents);
-		printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
+	struct dioattr dio = {};
+	struct fsxattr fsx = {}, fsxa = {};
+
+	if ((ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) {
+		perror("FS_IOC_GETXATTR");
+		exitcode = 1;
+		return;
 	}
+
+	printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
+	printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1);
+	printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid);
+	printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize);
+	printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize);
+	printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents);
+
+	/* Only XFS supports FS_IOC_FSGETXATTRA and XFS_IOC_DIOINFO */
+	if (file->flags & IO_FOREIGN)
+		return;
+
+	if ((ioctl(file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
+		perror("XFS_IOC_GETXATTRA");
+		exitcode = 1;
+		return;
+	}
+
+	printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
+
 	if ((xfsctl(file->name, file->fd, XFS_IOC_DIOINFO, &dio)) < 0) {
 		perror("XFS_IOC_DIOINFO");
-	} else {
-		printf(_("dioattr.mem = 0x%x\n"), dio.d_mem);
-		printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz);
-		printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz);
+		exitcode = 1;
+		return;
 	}
+
+	printf(_("dioattr.mem = 0x%x\n"), dio.d_mem);
+	printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz);
+	printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz);
 }
 
 int
@@ -167,10 +182,7 @@ stat_f(
 		printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
 	}
 
-	if (file->flags & IO_FOREIGN)
-		return 0;
-
-	print_xfs_info(verbose);
+	print_extended_info(verbose);
 
 	return 0;
 }
@@ -440,10 +452,7 @@ statx_f(
 				ctime((time_t *)&stx.stx_btime.tv_sec));
 	}
 
-	if (file->flags & IO_FOREIGN)
-		return 0;
-
-	print_xfs_info(verbose);
+	print_extended_info(verbose);
 
 	return 0;
 }
-- 
2.43.5

Re: [PATCH v2 2/3] xfs_io: allow foreign FSes to show FS_IOC_FSGETXATTR details

[Darrick J. Wong]

On Thu, Dec 19, 2024 at 06:09:14PM +0530, Ojaswin Mujoo wrote:
> Currently with stat we only show FS_IOC_FSGETXATTR details if the
> filesystem is XFS. With extsize support also coming to ext4 and possibly
> other filesystems, make sure to allow foreign FSes to display these details
> when "stat" or "statx" is used.
> 
> (Thanks to Dave for suggesting implementation of print_extended_info())
> 
> Suggested-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Looks good to me now,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  io/stat.c | 63 +++++++++++++++++++++++++++++++------------------------
>  1 file changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/io/stat.c b/io/stat.c
> index 326f2822e276..3ce3308d0562 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -98,30 +98,45 @@ print_file_info(void)
>  }
>  
>  static void
> -print_xfs_info(int verbose)
> +print_extended_info(int verbose)
>  {
> -	struct dioattr	dio;
> -	struct fsxattr	fsx, fsxa;
> -
> -	if ((xfsctl(file->name, file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0 ||
> -	    (xfsctl(file->name, file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
> -		perror("FS_IOC_FSGETXATTR");
> -	} else {
> -		printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
> -		printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1);
> -		printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid);
> -		printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize);
> -		printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize);
> -		printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents);
> -		printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
> +	struct dioattr dio = {};
> +	struct fsxattr fsx = {}, fsxa = {};
> +
> +	if ((ioctl(file->fd, FS_IOC_FSGETXATTR, &fsx)) < 0) {
> +		perror("FS_IOC_GETXATTR");
> +		exitcode = 1;
> +		return;
>  	}
> +
> +	printf(_("fsxattr.xflags = 0x%x "), fsx.fsx_xflags);
> +	printxattr(fsx.fsx_xflags, verbose, 0, file->name, 1, 1);
> +	printf(_("fsxattr.projid = %u\n"), fsx.fsx_projid);
> +	printf(_("fsxattr.extsize = %u\n"), fsx.fsx_extsize);
> +	printf(_("fsxattr.cowextsize = %u\n"), fsx.fsx_cowextsize);
> +	printf(_("fsxattr.nextents = %u\n"), fsx.fsx_nextents);
> +
> +	/* Only XFS supports FS_IOC_FSGETXATTRA and XFS_IOC_DIOINFO */
> +	if (file->flags & IO_FOREIGN)
> +		return;
> +
> +	if ((ioctl(file->fd, XFS_IOC_FSGETXATTRA, &fsxa)) < 0) {
> +		perror("XFS_IOC_GETXATTRA");
> +		exitcode = 1;
> +		return;
> +	}
> +
> +	printf(_("fsxattr.naextents = %u\n"), fsxa.fsx_nextents);
> +
>  	if ((xfsctl(file->name, file->fd, XFS_IOC_DIOINFO, &dio)) < 0) {
>  		perror("XFS_IOC_DIOINFO");
> -	} else {
> -		printf(_("dioattr.mem = 0x%x\n"), dio.d_mem);
> -		printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz);
> -		printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz);
> +		exitcode = 1;
> +		return;
>  	}
> +
> +	printf(_("dioattr.mem = 0x%x\n"), dio.d_mem);
> +	printf(_("dioattr.miniosz = %u\n"), dio.d_miniosz);
> +	printf(_("dioattr.maxiosz = %u\n"), dio.d_maxiosz);
>  }
>  
>  int
> @@ -167,10 +182,7 @@ stat_f(
>  		printf(_("stat.ctime = %s"), ctime(&st.st_ctime));
>  	}
>  
> -	if (file->flags & IO_FOREIGN)
> -		return 0;
> -
> -	print_xfs_info(verbose);
> +	print_extended_info(verbose);
>  
>  	return 0;
>  }
> @@ -440,10 +452,7 @@ statx_f(
>  				ctime((time_t *)&stx.stx_btime.tv_sec));
>  	}
>  
> -	if (file->flags & IO_FOREIGN)
> -		return 0;
> -
> -	print_xfs_info(verbose);
> +	print_extended_info(verbose);
>  
>  	return 0;
>  }
> -- 
> 2.43.5
> 
> 

[PATCH v2 3/3] xfs_io: add extsize command support

[Ojaswin Mujoo]

extsize command is currently only supported with XFS filesystem.
Lift this restriction now that ext4 is also supporting extsize hints.

Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 io/open.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io/open.c b/io/open.c
index a30dd89a1fd5..2582ff9b862e 100644
--- a/io/open.c
+++ b/io/open.c
@@ -997,7 +997,7 @@ open_init(void)
 	extsize_cmd.args = _("[-D | -R] [extsize]");
 	extsize_cmd.argmin = 0;
 	extsize_cmd.argmax = -1;
-	extsize_cmd.flags = CMD_NOMAP_OK;
+	extsize_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
 	extsize_cmd.oneline =
 		_("get/set preferred extent size (in bytes) for the open file");
 	extsize_cmd.help = extsize_help;
-- 
2.43.5

Re: [PATCH v2 3/3] xfs_io: add extsize command support

[Darrick J. Wong]

On Thu, Dec 19, 2024 at 06:09:15PM +0530, Ojaswin Mujoo wrote:
> extsize command is currently only supported with XFS filesystem.
> Lift this restriction now that ext4 is also supporting extsize hints.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  io/open.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/io/open.c b/io/open.c
> index a30dd89a1fd5..2582ff9b862e 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -997,7 +997,7 @@ open_init(void)
>  	extsize_cmd.args = _("[-D | -R] [extsize]");
>  	extsize_cmd.argmin = 0;
>  	extsize_cmd.argmax = -1;
> -	extsize_cmd.flags = CMD_NOMAP_OK;
> +	extsize_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
>  	extsize_cmd.oneline =
>  		_("get/set preferred extent size (in bytes) for the open file");
>  	extsize_cmd.help = extsize_help;
> -- 
> 2.43.5
> 
>