[PATCH -next] xfs: Fix unused variable 'mp' warning

[PATCH -next] xfs: Fix unused variable 'mp' warning

[Shaokun Zhang]

There is a warning on arm64 platform:
  CC [M]  fs/xfs/xfs_ioctl32.o
fs/xfs/xfs_ioctl32.c: In function ‘xfs_file_compat_ioctl’:
fs/xfs/xfs_ioctl32.c:441:20: warning: unused variable ‘mp’ [-Wunused-variable]
  441 |  struct xfs_mount *mp = ip->i_mount;
      |                    ^~
  LD [M]  fs/xfs/xfs.o

Fix this warning.

Fixes: f736d93d76d3 ("xfs: support idmapped mounts")
Cc: "Darrick J. Wong" <djwong@kernel.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Christian Brauner <christian.brauner@ubuntu.com> 
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 fs/xfs/xfs_ioctl32.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 926427b19573..fd590c0b5d3b 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -438,7 +438,6 @@ xfs_file_compat_ioctl(
 {
 	struct inode		*inode = file_inode(filp);
 	struct xfs_inode	*ip = XFS_I(inode);
-	struct xfs_mount	*mp = ip->i_mount;
 	void			__user *arg = compat_ptr(p);
 	int			error;
 
@@ -446,6 +445,8 @@ xfs_file_compat_ioctl(
 
 	switch (cmd) {
 #if defined(BROKEN_X86_ALIGNMENT)
+	struct xfs_mount	*mp = ip->i_mount;
+
 	case XFS_IOC_ALLOCSP_32:
 	case XFS_IOC_FREESP_32:
 	case XFS_IOC_ALLOCSP64_32:
-- 
2.7.4

Re: [PATCH -next] xfs: Fix unused variable 'mp' warning

[Christian Brauner]

On Wed, Feb 03, 2021 at 04:39:18PM +0800, Shaokun Zhang wrote:
> There is a warning on arm64 platform:
>   CC [M]  fs/xfs/xfs_ioctl32.o
> fs/xfs/xfs_ioctl32.c: In function ‘xfs_file_compat_ioctl’:
> fs/xfs/xfs_ioctl32.c:441:20: warning: unused variable ‘mp’ [-Wunused-variable]
>   441 |  struct xfs_mount *mp = ip->i_mount;
>       |                    ^~
>   LD [M]  fs/xfs/xfs.o
> 
> Fix this warning.
> 
> Fixes: f736d93d76d3 ("xfs: support idmapped mounts")
> Cc: "Darrick J. Wong" <djwong@kernel.org>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Christian Brauner <christian.brauner@ubuntu.com> 
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---

Want me to take that on top of the series, Christoph?

Christian

Re: [PATCH -next] xfs: Fix unused variable 'mp' warning

[Christoph Hellwig]

I don't think declaring a variable inside a switch statement is a good
idea.  This is what I had lying around but never got around finishing
up and submitting:

---
From 5e79886f08ca4dd96c9a508a380dfeb73cd4b529 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Wed, 3 Feb 2021 13:38:27 +0100
Subject: xfs: remove the possibly unused mp variable in xfs_file_compat_ioctl

The mp variable in xfs_file_compat_ioctl is only used when
BROKEN_X86_ALIGNMENT is define.  Remove it and just open code the
dereference in a few places.

Fixes: f736d93d76d3 ("xfs: support idmapped mounts")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl32.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index e11139e18021c1..daf73cb53a05bb 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -420,7 +420,6 @@ xfs_file_compat_ioctl(
 {
 	struct inode		*inode = file_inode(filp);
 	struct xfs_inode	*ip = XFS_I(inode);
-	struct xfs_mount	*mp = ip->i_mount;
 	void			__user *arg = compat_ptr(p);
 	int			error;
 
@@ -429,7 +428,7 @@ xfs_file_compat_ioctl(
 	switch (cmd) {
 #if defined(BROKEN_X86_ALIGNMENT)
 	case XFS_IOC_FSGEOMETRY_V1_32:
-		return xfs_compat_ioc_fsgeometry_v1(mp, arg);
+		return xfs_compat_ioc_fsgeometry_v1(ip->i_mount, arg);
 	case XFS_IOC_FSGROWFSDATA_32: {
 		struct xfs_growfs_data	in;
 
@@ -438,7 +437,7 @@ xfs_file_compat_ioctl(
 		error = mnt_want_write_file(filp);
 		if (error)
 			return error;
-		error = xfs_growfs_data(mp, &in);
+		error = xfs_growfs_data(ip->i_mount, &in);
 		mnt_drop_write_file(filp);
 		return error;
 	}
@@ -450,7 +449,7 @@ xfs_file_compat_ioctl(
 		error = mnt_want_write_file(filp);
 		if (error)
 			return error;
-		error = xfs_growfs_rt(mp, &in);
+		error = xfs_growfs_rt(ip->i_mount, &in);
 		mnt_drop_write_file(filp);
 		return error;
 	}
@@ -480,7 +479,7 @@ xfs_file_compat_ioctl(
 	case XFS_IOC_FSBULKSTAT_32:
 	case XFS_IOC_FSBULKSTAT_SINGLE_32:
 	case XFS_IOC_FSINUMBERS_32:
-		return xfs_compat_ioc_fsbulkstat(mp, cmd, arg);
+		return xfs_compat_ioc_fsbulkstat(ip->i_mount, cmd, arg);
 	case XFS_IOC_FD_TO_HANDLE_32:
 	case XFS_IOC_PATH_TO_HANDLE_32:
 	case XFS_IOC_PATH_TO_FSHANDLE_32: {
-- 
2.29.2

Re: [PATCH -next] xfs: Fix unused variable 'mp' warning

[Christian Brauner]

On Wed, Feb 03, 2021 at 01:41:17PM +0100, Christoph Hellwig wrote:
> I don't think declaring a variable inside a switch statement is a good

Yeah. (I would think that the compiler would complain about this.)

> idea.  This is what I had lying around but never got around finishing
> up and submitting:
> 
> ---
> From 5e79886f08ca4dd96c9a508a380dfeb73cd4b529 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Wed, 3 Feb 2021 13:38:27 +0100
> Subject: xfs: remove the possibly unused mp variable in xfs_file_compat_ioctl
> 
> The mp variable in xfs_file_compat_ioctl is only used when
> BROKEN_X86_ALIGNMENT is define.  Remove it and just open code the
> dereference in a few places.
> 
> Fixes: f736d93d76d3 ("xfs: support idmapped mounts")
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_ioctl32.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index e11139e18021c1..daf73cb53a05bb 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -420,7 +420,6 @@ xfs_file_compat_ioctl(
>  {
>  	struct inode		*inode = file_inode(filp);
>  	struct xfs_inode	*ip = XFS_I(inode);
> -	struct xfs_mount	*mp = ip->i_mount;
>  	void			__user *arg = compat_ptr(p);
>  	int			error;
>  
> @@ -429,7 +428,7 @@ xfs_file_compat_ioctl(
>  	switch (cmd) {
>  #if defined(BROKEN_X86_ALIGNMENT)
>  	case XFS_IOC_FSGEOMETRY_V1_32:
> -		return xfs_compat_ioc_fsgeometry_v1(mp, arg);
> +		return xfs_compat_ioc_fsgeometry_v1(ip->i_mount, arg);
>  	case XFS_IOC_FSGROWFSDATA_32: {
>  		struct xfs_growfs_data	in;
>  
> @@ -438,7 +437,7 @@ xfs_file_compat_ioctl(
>  		error = mnt_want_write_file(filp);
>  		if (error)
>  			return error;
> -		error = xfs_growfs_data(mp, &in);
> +		error = xfs_growfs_data(ip->i_mount, &in);
>  		mnt_drop_write_file(filp);
>  		return error;
>  	}
> @@ -450,7 +449,7 @@ xfs_file_compat_ioctl(
>  		error = mnt_want_write_file(filp);
>  		if (error)
>  			return error;
> -		error = xfs_growfs_rt(mp, &in);
> +		error = xfs_growfs_rt(ip->i_mount, &in);
>  		mnt_drop_write_file(filp);
>  		return error;
>  	}
> @@ -480,7 +479,7 @@ xfs_file_compat_ioctl(
>  	case XFS_IOC_FSBULKSTAT_32:
>  	case XFS_IOC_FSBULKSTAT_SINGLE_32:
>  	case XFS_IOC_FSINUMBERS_32:
> -		return xfs_compat_ioc_fsbulkstat(mp, cmd, arg);
> +		return xfs_compat_ioc_fsbulkstat(ip->i_mount, cmd, arg);

In the final version of you conversion (after the file_user_ns()
introduction) we simply pass down the fp so the patch needs to be?

If you're happy with it I can apply it on top. I don't want to rebase
this late. I can also send it separate as a reply in case this too much
in the body of this mail.

Patch passes cross-compilation for arm64 and native x864-64 and xfstests
pass too:

---
From a364f6e9de91cea671765cbd0e33fb823ebbba3c Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Wed, 3 Feb 2021 14:34:16 +0100
Subject: [PATCH] xfs: remove the possibly unused mp variable in
 xfs_file_compat_ioctl

The mp variable in xfs_file_compat_ioctl is only used when
BROKEN_X86_ALIGNMENT is define.  Remove it and just open code the
dereference in a few places.

Fixes: f736d93d76d3 ("xfs: support idmapped mounts")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_ioctl32.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 926427b19573..33c09ec8e6c0 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -438,7 +438,6 @@ xfs_file_compat_ioctl(
 {
 	struct inode		*inode = file_inode(filp);
 	struct xfs_inode	*ip = XFS_I(inode);
-	struct xfs_mount	*mp = ip->i_mount;
 	void			__user *arg = compat_ptr(p);
 	int			error;
 
@@ -458,7 +457,7 @@ xfs_file_compat_ioctl(
 		return xfs_ioc_space(filp, &bf);
 	}
 	case XFS_IOC_FSGEOMETRY_V1_32:
-		return xfs_compat_ioc_fsgeometry_v1(mp, arg);
+		return xfs_compat_ioc_fsgeometry_v1(ip->i_mount, arg);
 	case XFS_IOC_FSGROWFSDATA_32: {
 		struct xfs_growfs_data	in;
 
@@ -467,7 +466,7 @@ xfs_file_compat_ioctl(
 		error = mnt_want_write_file(filp);
 		if (error)
 			return error;
-		error = xfs_growfs_data(mp, &in);
+		error = xfs_growfs_data(ip->i_mount, &in);
 		mnt_drop_write_file(filp);
 		return error;
 	}
@@ -479,7 +478,7 @@ xfs_file_compat_ioctl(
 		error = mnt_want_write_file(filp);
 		if (error)
 			return error;
-		error = xfs_growfs_rt(mp, &in);
+		error = xfs_growfs_rt(ip->i_mount, &in);
 		mnt_drop_write_file(filp);
 		return error;
 	}

base-commit: f736d93d76d3e97d6986c6d26c8eaa32536ccc5c
-- 
2.30.0

Re: [PATCH -next] xfs: Fix unused variable 'mp' warning

[Christoph Hellwig]

On Wed, Feb 03, 2021 at 02:47:34PM +0100, Christian Brauner wrote:
> In the final version of you conversion (after the file_user_ns()
> introduction) we simply pass down the fp so the patch needs to be?
> 
> If you're happy with it I can apply it on top. I don't want to rebase
> this late. I can also send it separate as a reply in case this too much
> in the body of this mail.
> 
> Patch passes cross-compilation for arm64 and native x864-64 and xfstests
> pass too:

Let's wait for an ACK from Darrick, but I'd be fine with this.

Re: [PATCH -next] xfs: Fix unused variable 'mp' warning

[Christian Brauner]

On Wed, Feb 03, 2021 at 03:30:17PM +0100, Christoph Hellwig wrote:
> On Wed, Feb 03, 2021 at 02:47:34PM +0100, Christian Brauner wrote:
> > In the final version of you conversion (after the file_user_ns()
> > introduction) we simply pass down the fp so the patch needs to be?
> > 
> > If you're happy with it I can apply it on top. I don't want to rebase
> > this late. I can also send it separate as a reply in case this too much
> > in the body of this mail.
> > 
> > Patch passes cross-compilation for arm64 and native x864-64 and xfstests
> > pass too:
> 
> Let's wait for an ACK from Darrick, but I'd be fine with this.

Sounds good!
Christian

Re: [PATCH -next] xfs: Fix unused variable 'mp' warning

[Darrick J. Wong]

On Wed, Feb 03, 2021 at 03:41:25PM +0100, Christian Brauner wrote:
> On Wed, Feb 03, 2021 at 03:30:17PM +0100, Christoph Hellwig wrote:
> > On Wed, Feb 03, 2021 at 02:47:34PM +0100, Christian Brauner wrote:
> > > In the final version of you conversion (after the file_user_ns()
> > > introduction) we simply pass down the fp so the patch needs to be?
> > > 
> > > If you're happy with it I can apply it on top. I don't want to rebase
> > > this late. I can also send it separate as a reply in case this too much
> > > in the body of this mail.
> > > 
> > > Patch passes cross-compilation for arm64 and native x864-64 and xfstests
> > > pass too:
> > 
> > Let's wait for an ACK from Darrick, but I'd be fine with this.
> 
> Sounds good!
> Christian

Seems fine to me, but can one of you please send this as a proper patch?
:)

FWIW I really would prefer we make the tooling smart enough to shut up
about "unused" variables and "dead" code simply because we #define'd
their usage out of existence.  But since refactoring gcc/clang is
probably even more of an ocean-boiling approach I'll just invite you all
to join the XFS quota refactoring party. :P

--D

Re: [PATCH -next] xfs: Fix unused variable 'mp' warning

[Christian Brauner]

On Wed, Feb 03, 2021 at 09:16:33AM -0800, Darrick J. Wong wrote:
> On Wed, Feb 03, 2021 at 03:41:25PM +0100, Christian Brauner wrote:
> > On Wed, Feb 03, 2021 at 03:30:17PM +0100, Christoph Hellwig wrote:
> > > On Wed, Feb 03, 2021 at 02:47:34PM +0100, Christian Brauner wrote:
> > > > In the final version of you conversion (after the file_user_ns()
> > > > introduction) we simply pass down the fp so the patch needs to be?
> > > > 
> > > > If you're happy with it I can apply it on top. I don't want to rebase
> > > > this late. I can also send it separate as a reply in case this too much
> > > > in the body of this mail.
> > > > 
> > > > Patch passes cross-compilation for arm64 and native x864-64 and xfstests
> > > > pass too:
> > > 
> > > Let's wait for an ACK from Darrick, but I'd be fine with this.
> > 
> > Sounds good!
> > Christian
> 
> Seems fine to me, but can one of you please send this as a proper patch?
> :)

Yeah, for sure!

> 
> FWIW I really would prefer we make the tooling smart enough to shut up
> about "unused" variables and "dead" code simply because we #define'd
> their usage out of existence.  But since refactoring gcc/clang is
> probably even more of an ocean-boiling approach I'll just invite you all
> to join the XFS quota refactoring party. :P

In 2021 that might indeed qualify as a party... :)

Christian