| Message ID | 20230125050138.372749-1-ddouwsma@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | xfs: allow setting full range of panic tags | expand |
On Wed, Jan 25, 2023 at 04:01:38PM +1100, Donald Douwsma wrote: > xfs will not allow combining other panic values 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: d519da41e2b78 ("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_globals.c | 2 +- > 2 files changed, 2 insertions(+), 2 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_globals.c b/fs/xfs/xfs_globals.c > index 4d0a98f920ca..e0e9494a8251 100644 > --- a/fs/xfs/xfs_globals.c > +++ b/fs/xfs/xfs_globals.c > @@ -15,7 +15,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, 511 }, Why not fix this by defining an XFS_PTAG_ALL_MASK that combines all valid flags and use that here? That way we eliminate this class of bug. Looking at d519da41e2b78, the maintainers suck at noticing these kinds of mistakes. --D > .error_level = { 0, 3, 11 }, > .syncd_timer = { 1*100, 30*100, 7200*100}, > .stats_clear = { 0, 0, 1 }, > -- > 2.31.1 >
On 26/01/2023 03:13, Darrick J. Wong wrote: > On Wed, Jan 25, 2023 at 04:01:38PM +1100, Donald Douwsma wrote: >> xfs will not allow combining other panic values 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: d519da41e2b78 ("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_globals.c | 2 +- >> 2 files changed, 2 insertions(+), 2 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_globals.c b/fs/xfs/xfs_globals.c >> index 4d0a98f920ca..e0e9494a8251 100644 >> --- a/fs/xfs/xfs_globals.c >> +++ b/fs/xfs/xfs_globals.c >> @@ -15,7 +15,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, 511 }, > > Why not fix this by defining an XFS_PTAG_ALL_MASK that combines all > valid flags and use that here? That way we eliminate this class of bug. Sure, perhaps XFS_MAX_PTAG to fit with its use in xfs_params? > > Looking at d519da41e2b78, the maintainers suck at noticing these kinds > of mistakes. The interface is problematic in the field too, folks were using 256 to mean all, and wondered why they weren't hitting anything, including XFS_PTAG_SHUTDOWN_CORRUPT. I'll OR together the masks to be clear with respect to the documentation. Don > > --D > >> .error_level = { 0, 3, 11 }, >> .syncd_timer = { 1*100, 30*100, 7200*100}, >> .stats_clear = { 0, 0, 1 }, >> -- >> 2.31.1 >> >
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_globals.c b/fs/xfs/xfs_globals.c index 4d0a98f920ca..e0e9494a8251 100644 --- a/fs/xfs/xfs_globals.c +++ b/fs/xfs/xfs_globals.c @@ -15,7 +15,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, 511 }, .error_level = { 0, 3, 11 }, .syncd_timer = { 1*100, 30*100, 7200*100}, .stats_clear = { 0, 0, 1 },
xfs will not allow combining other panic values 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: d519da41e2b78 ("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_globals.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)