Re: [PATCH v2] xfs: allow setting full range of panic tags

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Jan 26, 2023 at 04:29:10PM +1100, Donald Douwsma wrote:
> xfs will not allow combining other panic masks with
> XFS_PTAG_VERIFIER_ERROR.
> 
>  sysctl fs.xfs.panic_mask=511
>  sysctl: setting key "fs.xfs.panic_mask": Invalid argument
>  fs.xfs.panic_mask = 511
> 
> Update to the maximum value that can be set to allow the full range of
> masks.
> 
> Fixes: d519da41e2b7 ("xfs: Introduce XFS_PTAG_VERIFIER_ERROR panic mask")
> Signed-off-by: Donald Douwsma <ddouwsma@redhat.com>
> ---
>  Documentation/admin-guide/xfs.rst |  2 +-
>  fs/xfs/xfs_error.h                | 13 ++++++++++++-
>  fs/xfs/xfs_globals.c              |  3 ++-
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/xfs.rst b/Documentation/admin-guide/xfs.rst
> index 8de008c0c5ad..e2561416391c 100644
> --- a/Documentation/admin-guide/xfs.rst
> +++ b/Documentation/admin-guide/xfs.rst
> @@ -296,7 +296,7 @@ The following sysctls are available for the XFS filesystem:
>  		XFS_ERRLEVEL_LOW:       1
>  		XFS_ERRLEVEL_HIGH:      5
>  
> -  fs.xfs.panic_mask		(Min: 0  Default: 0  Max: 256)
> +  fs.xfs.panic_mask		(Min: 0  Default: 0  Max: 511)
>  	Causes certain error conditions to call BUG(). Value is a bitmask;
>  	OR together the tags which represent errors which should cause panics:
>  
> diff --git a/fs/xfs/xfs_error.h b/fs/xfs/xfs_error.h
> index dbe6c37dc697..a015f7b370dc 100644
> --- a/fs/xfs/xfs_error.h
> +++ b/fs/xfs/xfs_error.h
> @@ -75,7 +75,7 @@ extern int xfs_errortag_clearall(struct xfs_mount *mp);
>  
>  /*
>   * XFS panic tags -- allow a call to xfs_alert_tag() be turned into
> - *			a panic by setting xfs_panic_mask in a sysctl.
> + *			a panic by setting fs.xfs.panic_mask in a sysctl.
>   */
>  #define		XFS_NO_PTAG			0u
>  #define		XFS_PTAG_IFLUSH			(1u << 0)
> @@ -88,6 +88,17 @@ extern int xfs_errortag_clearall(struct xfs_mount *mp);
>  #define		XFS_PTAG_FSBLOCK_ZERO		(1u << 7)
>  #define		XFS_PTAG_VERIFIER_ERROR		(1u << 8)
>  
> +#define		XFS_MAX_PTAG ( \

The ptag values are a bitmask, not a continuous integer range, so the
name should have "MASK" in it, e.g.

#define			XFS_PTAG_MASK	(XFS_PTAG_IFLUSH | \
					 XFS_PTAG_LOGRES | \
					...

and follow the customary style where the macro definition lines are
indented from the name.

Otherwise this looks fine.

--D

> +			XFS_PTAG_IFLUSH | \
> +			XFS_PTAG_LOGRES | \
> +			XFS_PTAG_AILDELETE | \
> +			XFS_PTAG_ERROR_REPORT | \
> +			XFS_PTAG_SHUTDOWN_CORRUPT | \
> +			XFS_PTAG_SHUTDOWN_IOERROR | \
> +			XFS_PTAG_SHUTDOWN_LOGERROR | \
> +			XFS_PTAG_FSBLOCK_ZERO | \
> +			XFS_PTAG_VERIFIER_ERROR)
> +
>  #define XFS_PTAG_STRINGS \
>  	{ XFS_NO_PTAG,			"none" }, \
>  	{ XFS_PTAG_IFLUSH,		"iflush" }, \
> diff --git a/fs/xfs/xfs_globals.c b/fs/xfs/xfs_globals.c
> index 4d0a98f920ca..ff129acce8e6 100644
> --- a/fs/xfs/xfs_globals.c
> +++ b/fs/xfs/xfs_globals.c
> @@ -4,6 +4,7 @@
>   * All Rights Reserved.
>   */
>  #include "xfs.h"
> +#include "xfs_error.h"
>  
>  /*
>   * Tunable XFS parameters.  xfs_params is required even when CONFIG_SYSCTL=n,
> @@ -15,7 +16,7 @@ xfs_param_t xfs_params = {
>  			  /*	MIN		DFLT		MAX	*/
>  	.sgid_inherit	= {	0,		0,		1	},
>  	.symlink_mode	= {	0,		0,		1	},
> -	.panic_mask	= {	0,		0,		256	},
> +	.panic_mask	= {	0,		0,		XFS_MAX_PTAG},
>  	.error_level	= {	0,		3,		11	},
>  	.syncd_timer	= {	1*100,		30*100,		7200*100},
>  	.stats_clear	= {	0,		0,		1	},
> -- 
> 2.31.1
>