[PATCHSET 0/2] xfs: random pending stuff for 5.13

[PATCHSET 0/2] xfs: random pending stuff for 5.13

[Darrick J. Wong]

Hi all,

A couple of assorted fixes for crashers and uapi regressions.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes-5.13

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes-5.13
---
 fs/xfs/libxfs/xfs_fs.h |    4 ++++
 fs/xfs/scrub/common.c  |    4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

[PATCH 1/2] xfs: fix deadlock retry tracepoint arguments

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

sc->ip is the inode that's being scrubbed, which means that it's not set
for scrub types that don't involve inodes.  If one of those scrubbers
(e.g. inode btrees) returns EDEADLOCK, we'll trip over the null pointer.
Fix that by reporting either the file being examined or the file that
was used to call scrub.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/scrub/common.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index aa874607618a..be38c960da85 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -74,7 +74,9 @@ __xchk_process_error(
 		return true;
 	case -EDEADLOCK:
 		/* Used to restart an op with deadlock avoidance. */
-		trace_xchk_deadlock_retry(sc->ip, sc->sm, *error);
+		trace_xchk_deadlock_retry(
+				sc->ip ? sc->ip : XFS_I(file_inode(sc->file)),
+				sc->sm, *error);
 		break;
 	case -EFSBADCRC:
 	case -EFSCORRUPTED:

Re: [PATCH 1/2] xfs: fix deadlock retry tracepoint arguments

[Brian Foster]

On Wed, May 12, 2021 at 06:01:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> sc->ip is the inode that's being scrubbed, which means that it's not set
> for scrub types that don't involve inodes.  If one of those scrubbers
> (e.g. inode btrees) returns EDEADLOCK, we'll trip over the null pointer.
> Fix that by reporting either the file being examined or the file that
> was used to call scrub.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/scrub/common.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
> index aa874607618a..be38c960da85 100644
> --- a/fs/xfs/scrub/common.c
> +++ b/fs/xfs/scrub/common.c
> @@ -74,7 +74,9 @@ __xchk_process_error(
>  		return true;
>  	case -EDEADLOCK:
>  		/* Used to restart an op with deadlock avoidance. */
> -		trace_xchk_deadlock_retry(sc->ip, sc->sm, *error);
> +		trace_xchk_deadlock_retry(
> +				sc->ip ? sc->ip : XFS_I(file_inode(sc->file)),
> +				sc->sm, *error);
>  		break;
>  	case -EFSBADCRC:
>  	case -EFSCORRUPTED:
> 

Re: [PATCH 1/2] xfs: fix deadlock retry tracepoint arguments

[Christoph Hellwig]

On Wed, May 12, 2021 at 06:01:34PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> sc->ip is the inode that's being scrubbed, which means that it's not set
> for scrub types that don't involve inodes.  If one of those scrubbers
> (e.g. inode btrees) returns EDEADLOCK, we'll trip over the null pointer.
> Fix that by reporting either the file being examined or the file that
> was used to call scrub.

Without an indication of which one we trace this is a little weird,
isn't it?  Still better than a crash, though..

Re: [PATCH 1/2] xfs: fix deadlock retry tracepoint arguments

[Darrick J. Wong]

On Wed, May 19, 2021 at 02:20:22PM +0100, Christoph Hellwig wrote:
> On Wed, May 12, 2021 at 06:01:34PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > sc->ip is the inode that's being scrubbed, which means that it's not set
> > for scrub types that don't involve inodes.  If one of those scrubbers
> > (e.g. inode btrees) returns EDEADLOCK, we'll trip over the null pointer.
> > Fix that by reporting either the file being examined or the file that
> > was used to call scrub.
> 
> Without an indication of which one we trace this is a little weird,
> isn't it?  Still better than a crash, though..

The scrub type is also encoded in the tracepoint, so we can tell that
the inode number is meaningless.

--D

[PATCH 2/2] xfs: restore old ioctl definitions

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

These ioctl definitions in xfs_fs.h are part of the userspace ABI and
were mistakenly removed during the 5.13 merge window.

Fixes: 9fefd5db08ce ("xfs: convert to fileattr")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_fs.h |    4 ++++
 1 file changed, 4 insertions(+)


diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
index a83bdd0c47a8..bde2b4c64dbe 100644
--- a/fs/xfs/libxfs/xfs_fs.h
+++ b/fs/xfs/libxfs/xfs_fs.h
@@ -770,6 +770,8 @@ struct xfs_scrub_metadata {
 /*
  * ioctl commands that are used by Linux filesystems
  */
+#define XFS_IOC_GETXFLAGS	FS_IOC_GETFLAGS
+#define XFS_IOC_SETXFLAGS	FS_IOC_SETFLAGS
 #define XFS_IOC_GETVERSION	FS_IOC_GETVERSION
 
 /*
@@ -780,6 +782,8 @@ struct xfs_scrub_metadata {
 #define XFS_IOC_ALLOCSP		_IOW ('X', 10, struct xfs_flock64)
 #define XFS_IOC_FREESP		_IOW ('X', 11, struct xfs_flock64)
 #define XFS_IOC_DIOINFO		_IOR ('X', 30, struct dioattr)
+#define XFS_IOC_FSGETXATTR	FS_IOC_FSGETXATTR
+#define XFS_IOC_FSSETXATTR	FS_IOC_FSSETXATTR
 #define XFS_IOC_ALLOCSP64	_IOW ('X', 36, struct xfs_flock64)
 #define XFS_IOC_FREESP64	_IOW ('X', 37, struct xfs_flock64)
 #define XFS_IOC_GETBMAP		_IOWR('X', 38, struct getbmap)

Re: [PATCH 2/2] xfs: restore old ioctl definitions

[Brian Foster]

On Wed, May 12, 2021 at 06:01:39PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> These ioctl definitions in xfs_fs.h are part of the userspace ABI and
> were mistakenly removed during the 5.13 merge window.
> 
> Fixes: 9fefd5db08ce ("xfs: convert to fileattr")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Maybe we should add a comment if these need to stick around unused in
the kernel code..? Otherwise LGTM:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_fs.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> index a83bdd0c47a8..bde2b4c64dbe 100644
> --- a/fs/xfs/libxfs/xfs_fs.h
> +++ b/fs/xfs/libxfs/xfs_fs.h
> @@ -770,6 +770,8 @@ struct xfs_scrub_metadata {
>  /*
>   * ioctl commands that are used by Linux filesystems
>   */
> +#define XFS_IOC_GETXFLAGS	FS_IOC_GETFLAGS
> +#define XFS_IOC_SETXFLAGS	FS_IOC_SETFLAGS
>  #define XFS_IOC_GETVERSION	FS_IOC_GETVERSION
>  
>  /*
> @@ -780,6 +782,8 @@ struct xfs_scrub_metadata {
>  #define XFS_IOC_ALLOCSP		_IOW ('X', 10, struct xfs_flock64)
>  #define XFS_IOC_FREESP		_IOW ('X', 11, struct xfs_flock64)
>  #define XFS_IOC_DIOINFO		_IOR ('X', 30, struct dioattr)
> +#define XFS_IOC_FSGETXATTR	FS_IOC_FSGETXATTR
> +#define XFS_IOC_FSSETXATTR	FS_IOC_FSSETXATTR
>  #define XFS_IOC_ALLOCSP64	_IOW ('X', 36, struct xfs_flock64)
>  #define XFS_IOC_FREESP64	_IOW ('X', 37, struct xfs_flock64)
>  #define XFS_IOC_GETBMAP		_IOWR('X', 38, struct getbmap)
> 

Re: [PATCH 2/2] xfs: restore old ioctl definitions

[Christoph Hellwig]

On Wed, May 12, 2021 at 06:01:39PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> These ioctl definitions in xfs_fs.h are part of the userspace ABI and
> were mistakenly removed during the 5.13 merge window.
> 
> Fixes: 9fefd5db08ce ("xfs: convert to fileattr")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>