Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: "Jan Beulich" <jbeulich@suse.com>, "Jürgen Groß" <jgross@suse.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>, "Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	xen-devel@lists.xenproject.org,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v2 1/2] xen: add config option to include failing condition in BUG_ON() message
Date: Thu, 16 Jan 2020 18:54:45 +0000
Message-ID: <aedc415a-5a54-5f54-8daa-14685eb78276@citrix.com> (raw)
In-Reply-To: <251bd26c-1937-e70b-3ec3-6ebb10e879dc@suse.com>

On 14/01/2020 16:12, Jan Beulich wrote:
> On 14.01.2020 17:00, Jürgen Groß wrote:
>> On 14.01.20 16:47, Jan Beulich wrote:
>>> On 09.01.2020 14:48, Juergen Gross wrote:
>>>> --- a/xen/Kconfig.debug
>>>> +++ b/xen/Kconfig.debug
>>>> @@ -81,6 +81,12 @@ config PERF_ARRAYS
>>>>   	---help---
>>>>   	  Enables software performance counter array histograms.
>>>>   
>>>> +config DEBUG_BUGVERBOSE
>>>> +	bool "Verbose BUG_ON messages"
>>>> +	default DEBUG
>>>> +	---help---
>>>> +	  In case a BUG_ON triggers additionally print the triggering
>>>> +	  condition on the console.
>>>>   
>>>>   config VERBOSE_DEBUG
>>> While I can see reasons to put this here, doing so means the option
>>> will be unavailable in non-EXPERT release builds. Is it intended to
>>> be that way?
>> I can move it either to xen/Kconfig or in Kconfig.debug out of the
>> "if expert" section if you want.
> I think this would be better, but give others a chance to voice
> opinions.

TBH, I don't think anyone will be interested in not having the strings. 
The change is what? a couple of hundred bytes?  That is a fraction of
the size of some functions we have.

Personally, I wouldn't even bother having the option.

>
>>>> --- a/xen/include/xen/lib.h
>>>> +++ b/xen/include/xen/lib.h
>>>> @@ -8,7 +8,12 @@
>>>>   #include <xen/string.h>
>>>>   #include <asm/bug.h>
>>>>   
>>>> +#define BUG_ON_VERBOSE(p) do { if (unlikely(p)) BUG_VERBOSE(#p);  } while (0)
>>>> +#ifdef CONFIG_DEBUG_BUGVERBOSE
>>>> +#define BUG_ON(p)  BUG_ON_VERBOSE(p)
>>> Looks like this will fail to build on Arm? Also - any particular
>> Uh, shame on me!
>>
>>> reason for the introduction of the separate BUG_ON_VERBOSE(),
>>> when BUG_ON() could directly use BUG_VERBOSE()? I don't think we
>>> want to encourage use of BUG_ON_VERBOSE() elsewhere ...
>> I wanted to offer that option. If you want me to remove it I wouldn't
>> mind.
> As above - unless there are good reasons (making others to agree
> with you to have it), I'd prefer to not see it being independently
> usable, at least for the time being.

I'd agree with the wish to not have a new flavour of BUG_ON().

People writing code aren't going to want the complexity of thinking
about it, and people who care about the presence/absence of messages
will care about it globally, not on a per-use bases.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-09 13:48 [Xen-devel] [PATCH v2 0/2] xen: fix CONFIG_DEBUG_LOCKS Juergen Gross
2020-01-09 13:48 ` [Xen-devel] [PATCH v2 1/2] xen: add config option to include failing condition in BUG_ON() message Juergen Gross
2020-01-14 15:47   ` Jan Beulich
2020-01-14 16:00     ` Jürgen Groß
2020-01-14 16:12       ` Jan Beulich
2020-01-16 18:54         ` Andrew Cooper [this message]
2020-01-17  8:39           ` Jan Beulich
2020-01-17  8:53             ` Jürgen Groß
2020-01-09 13:48 ` [Xen-devel] [PATCH v2 2/2] xen: make CONFIG_DEBUG_LOCKS usable without CONFIG_DEBUG Juergen Gross
2020-01-09 14:13   ` Jan Beulich

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aedc415a-5a54-5f54-8daa-14685eb78276@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=julien@xen.org \
    --cc=konrad.wilk@oracle.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git