xfs: make fatal assert failures conditional in debug mode
v3 cover xfs: make fatal assert failures conditional in debug mode
Hi all,
This version makes fatal asserts runtime and build time configurable. Patch 1 creates the runtime tunable and patch 2 creates a Kconfig option to control the default value of the runtime tunable. As before, default DEBUG mode behavior is preserved (i.e., assert failures BUG() the kernel).
Brian
v3:
- Add DEBUG mode sysfs tunable for runtime configuration.
- Refactor compile time flag to set default value of runtime flag.
v2: http://www.spinics.net/lists/linux-xfs/msg06520.html
- Clean up the Kconfig option help text.
v1: http://www.spinics.net/lists/linux-xfs/msg06498.html
- Use a new config option rather than reuse XFS_WARN.
- Disable BUG() in DEBUG mode by default and flip the logic of the new config option.
rfc: http://www.spinics.net/lists/linux-xfs/msg06390.html
Brian Foster (2):
xfs: define bug_on_assert debug mode sysfs tunable
xfs: define fatal assert build time tunable
fs/xfs/Kconfig | 13 +++++++++++++ fs/xfs/xfs.h | 4 ++++ fs/xfs/xfs_globals.c | 5 +++++ fs/xfs/xfs_message.c | 5 ++++- fs/xfs/xfs_sysctl.h | 1 + fs/xfs/xfs_sysfs.c | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 60 insertions(+), 1 deletion(-)
Signature · 1 line
2.7.4
PATCH v3 1/2 xfs: define bug_on_assert debug mode sysfs tunable
In DEBUG mode, assert failures unconditionally trigger a kernel BUG. This is useful in diagnostic situations to panic a system and collect detailed state information at the time of a failure.
This can also cause problems in cases where DEBUG mode code is desired but it is preferable not trigger kernel BUGs on assert failure. For example, during development of new code or during certain xfstests tests that intentionally cause corruption and test the kernel for survival (but otherwise may expect to trigger assert failures).
To provide additional flexibility, create the <sysfs>/fs/xfs/debug/bug_on_assert tunable to configure assert failure behavior at runtime. This tunable is only available in DEBUG mode and is enabled by default to preserve existing default behavior. When disabled, assert failures in DEBUG mode result in kernel warnings.
--- fs/xfs/xfs_globals.c | 2 ++ fs/xfs/xfs_message.c | 5 ++++- fs/xfs/xfs_sysctl.h | 1 + fs/xfs/xfs_sysfs.c | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 1 deletion(-)
fs/xfs/xfs_globals.c +2 -0
@@ -47,4 +47,6 @@ xfs_param_t xfs_params = {struct xfs_globals xfs_globals = { .log_recovery_delay =0 ,/* no delay by default */ + .bug_on_assert = true,/* historical default in DEBUG +* mode */ };
fs/xfs/xfs_message.c +4 -1
@@ -110,7 +110,10 @@ assfail(char *expr, char *file, int line) {xfs_emerg (NULL ,"Assertion failed: %s, file: %s, line: %d" , expr, file, line); -BUG (); +if (xfs_globals.bug_on_assert) +BUG (); +else +WARN_ON (1 ); }void
fs/xfs/xfs_sysctl.h +1 -0
@@ -95,6 +95,7 @@ extern xfs_param_t xfs_params;struct xfs_globals {int log_recovery_delay;/* log recovery delay (secs) */ +bool bug_on_assert;/* BUG() the kernel on assert failure */ };extern struct xfs_globals xfs_globals;
fs/xfs/xfs_sysfs.c +33 -0
@@ -146,6 +146,38 @@ struct kobj_type xfs_mp_ktype = {/* debug */ STATIC ssize_t +bug_on_assert_store ( +struct kobject *kobject, +const char *buf, +size_t count) +{ +int ret; +int val; + + ret =kstrtoint (buf,0 , &val); +if (ret) +return ret; + +if (val ==1 ) + xfs_globals.bug_on_assert = true; +else if (val ==0 ) + xfs_globals.bug_on_assert = false; +else +return -EINVAL ; + +return count; +} + +STATIC ssize_t +bug_on_assert_show ( +struct kobject *kobject, +char *buf) +{ +return snprintf (buf,PAGE_SIZE ,"%d\n" , xfs_globals.bug_on_assert ?1 :0 ); +} +XFS_SYSFS_ATTR_RW (bug_on_assert); + +STATIC ssize_t log_recovery_delay_store( struct kobject *kobject, const char *buf,
@@ -176,6 +208,7 @@ log_recovery_delay_show(XFS_SYSFS_ATTR_RW (log_recovery_delay);static struct attribute *xfs_dbg_attrs[] = { +ATTR_LIST (bug_on_assert),ATTR_LIST (log_recovery_delay),NULL , };
Signature · 1 line
2.7.4
PATCH v3 2/2 xfs: define fatal assert build time tunable
In reply to Brian FosterWhile configurable at runtime, the DEBUG mode assert failure behavior is usually either desired or not for a particular situation. For example, developers using kernel modules may prefer for fatal asserts to remain disabled across module reloads while QE engineers doing broad regression testing may prefer to have fatal asserts enabled on boot to facilitate data collection for bug reports.
To provide a compromise/convenience for developers, create a Kconfig option that sets the default value of the DEBUG mode 'bug_on_assert' sysfs tunable. The default behavior remains to trigger kernel BUGs on assert failures to preserve existing behavior across kernel configuration updates with DEBUG mode enabled.
--- fs/xfs/Kconfig | 13 +++++++++++++ fs/xfs/xfs.h | 4 ++++ fs/xfs/xfs_globals.c | 7 +++++-- 3 files changed, 22 insertions(+), 2 deletions(-)
fs/xfs/Kconfig +13 -0
@@ -96,3 +96,16 @@ config XFS_DEBUG not useful unless you are debugging a particular problem. Say N unless you are an XFS developer, or you play one on TV. + +config XFS_ASSERT_FATAL + bool "XFS fatal asserts" + default y + depends on XFS_FS && XFS_DEBUG + help + Set the default DEBUG mode ASSERT failure behavior. + + Say Y here to cause DEBUG mode ASSERT failures to result in fatal + errors that BUG() the kernel by default. If you say N, ASSERT failures + result in warnings. + + This behavior can be modified at runtime via sysfs.
fs/xfs/xfs.h +4 -0
@@ -24,6 +24,10 @@#define XFS_BUF_LOCK_TRACKING 1#endif +#ifdef CONFIG_XFS_ASSERT_FATAL +#define XFS_ASSERT_FATAL 1 +#endif +#ifdef CONFIG_XFS_WARN #define XFS_WARN 1#endif
fs/xfs/xfs_globals.c +5 -2
@@ -47,6 +47,9 @@ xfs_param_t xfs_params = {struct xfs_globals xfs_globals = { .log_recovery_delay =0 ,/* no delay by default */ - .bug_on_assert = true,/* historical default in DEBUG -* mode */ +#ifdef XFS_ASSERT_FATAL + .bug_on_assert = true,/* assert failures BUG() */ +#else + .bug_on_assert = false,/* assert failures WARN() */ +#endif };
Signature · 1 line
2.7.4
Re: [PATCH v3 2/2] xfs: define fatal assert build time tunable
Quoted 62 lines · Brian Foster
While configurable at runtime, the DEBUG mode assert failure behavior is usually either desired or not for a particular situation. For example, developers using kernel modules may prefer for fatal asserts to remain disabled across module reloads while QE engineers doing broad regression testing may prefer to have fatal asserts enabled on boot to facilitate data collection for bug reports.
To provide a compromise/convenience for developers, create a
Kconfigoption that sets the default value of the DEBUG mode 'bug_on_assert' sysfs tunable. The default behavior remains to trigger kernel BUGs on assert failures to preserve existing behavior across kernel configuration updates with DEBUG mode enabled.Signed-off-by Brian Foster <[email protected]>--- fs/xfs/Kconfig | 13 +++++++++++++ fs/xfs/xfs.h | 4 ++++ fs/xfs/xfs_globals.c | 7 +++++-- 3 files changed, 22 insertions(+), 2 deletions(-)fs/xfs/Kconfig +13
-0@@ -96,3 +96,16 @@ config XFS_DEBUG not useful unless you are debugging a particular problem. Say N unless you are an XFS developer, or you play one on TV. + +config XFS_ASSERT_FATAL + bool "XFS fatal asserts" + default y + depends on XFS_FS && XFS_DEBUG + help + Set the default DEBUG mode ASSERT failure behavior. + + Say Y here to cause DEBUG mode ASSERT failures to result in fatal + errors that BUG() the kernel by default. If you say N, ASSERT failures + result in warnings. + + This behavior can be modified at runtime via sysfs.fs/xfs/xfs.h +4
-0@@ -24,6 +24,10 @@#define XFS_BUF_LOCK_TRACKING 1#endif +#ifdef CONFIG_XFS_ASSERT_FATAL +#define XFS_ASSERT_FATAL 1 +#endif +#ifdef CONFIG_XFS_WARN #define XFS_WARN 1#endif fs/xfs/xfs_globals.c +0
-0
fs/xfs/xfs_globals.c xfs_param_t xfs_params = { · line 50
Quoted 6 lines
fs/xfs/xfs_globals.c +1
-2@@ -47,6 +47,9 @@ xfs_param_t xfs_params = {struct xfs_globals xfs_globals = { .log_recovery_delay =0 ,/* no delay by default */ - .bug_on_assert = true,/* historical default in DEBUG -* mode */ +#ifdef XFS_ASSERT_FATAL
Minor nit to pick, but if you're only testing that XFS_ASSERT_FATAL is defined, you could just '#define XFS_ASSERT_FATAL' without providing a value.... or '#if XFS_ASSERT_FATAL' here.
OTOH XFS_WARN also does that, and we reach the same ends regardless of which way we do it. I'm willing to pull this in for 4.13 as-is, if nobody objects.
--D
Quoted 12 lines
fs/xfs/xfs_globals.c +4
-0+ .bug_on_assert = true, /* assert failures BUG() */ +#else + .bug_on_assert = false, /* assert failures WARN() */ +#endif };Signature · 6 lines
2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html