Re: [PATCH RFC] xfs: support debug mode with assert warnings

[Brian Foster <bfoster@redhat.com>]

On Thu, May 04, 2017 at 04:57:25AM -0700, Christoph Hellwig wrote:
> On Tue, May 02, 2017 at 08:14:25AM -0400, Brian Foster wrote:
> > As noted above, this is just a reuse of the flag. XFS_WARN and XFS_DEBUG
> > are currently mutually exclusive. The former enables warnings on assert
> > failures. The latter enables BUG()'s on assert failures and the
> > additional, typical debug mode code. The Kconfig hack above simply pops
> > up a conditional option when debug mode is enabled that effectively
> > allows setting both XFS_WARN and XFS_DEBUG at the same time. The header
> > files interpret this as debug mode enabled with an override for the
> > assert failures to warn rather than BUG().
> 
> Oh, I didn't know Kconfig allows the same symbol to be define twice.
> But even if that's ok I'd say it's rather odd.
> 

Yes, I just slapped it together quickly to float the idea.

> > Note that this is just a hack and we can organize the Kconfig options
> > however we want. For example, we could call this XFS_DEBUG_WARN and
> > continue to consider it a debug mode sub-flag, or we could turn the
> > debug mode option into a multi-mode selector (i.e., Debug modes: "None,"
> > "Warn only," "Debug mode," "Debug mode w/ non-fatal asserts"). I played
> > around a bit with the latter but it seems like a bit of overkill to me.
> 
> Maybe we need something like:
> 
>  XFS_WARN (as-is)
>  XFS_WARN_BUG (XFS_WARN + BUG_ON on assert)
>  XFS_DEBUG (everyhing under XFS_DEBUG currently that's not related to
> 	ASSERT)

That allows asserts to BUG() on an XFS_WARN kernel, which is not quite
what I want to accomplish here (and I don't think that's really needed
for an XFS_WARN kernel). I'm not quite following if/how that allows to
disable assert BUG()s on an XFS_DEBUG kernel. Would XFS_DEBUG now never
BUG() unless XFS_WARN_BUG is defined as well?

If so, that sounds reasonable to me. I may just suggest tweaking it to
something like this:

XFS_WARN (as-is)
XFS_DEBUG (DEBUG code modified to warn on assert failure by default)
XFS_ASSERT_BUG (depends on XFS_DEBUG, enables BUG() on assert failure)

WARN and DEBUG remain mutually exclusive, the default assert behavior
for DEBUG changes to a warning rather than BUG(), and the latter is
enabled by a new conditional XFS_ASSERT_BUG config option. My only
slight concern is that changes default behavior for distros that might
create debug builds/packages, but I can see whether we can mitigate that
by setting 'default y' for XFS_ASSERT_BUG so long as it is only
available in XFS_DEBUG mode. Thoughts?

Brian

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html