* [PATCH] xen: Kconfig: Add DEBUG_XEN config option
@ 2017-02-01 11:19 Andrii Anisov
2017-02-01 12:17 ` Juergen Gross
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Anisov @ 2017-02-01 11:19 UTC (permalink / raw)
To: xen-devel; +Cc: julien.grall, sstabellini, andrii_anisov
From: Andrii Anisov <andrii_anisov@epam.com>
Add a debug option to enable xen drivers debug code.
Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
---
drivers/xen/Kconfig | 8 ++++++++
drivers/xen/Makefile | 2 ++
2 files changed, 10 insertions(+)
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index f15bb3b7..733c433 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -298,4 +298,12 @@ config XEN_SYMS
config XEN_HAVE_VPMU
bool
+config DEBUG_XEN
+ bool "XEN Drivers debug"
+ depends on DEBUG_KERNEL
+ help
+ Say Y here if you want to enable XEN drivers debug code.
+
+ If you are unsure about this, say N here.
+
endmenu
diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
index 8feab810..c886e9d 100644
--- a/drivers/xen/Makefile
+++ b/drivers/xen/Makefile
@@ -42,3 +42,5 @@ xen-evtchn-y := evtchn.o
xen-gntdev-y := gntdev.o
xen-gntalloc-y := gntalloc.o
xen-privcmd-y := privcmd.o
+
+ccflags-$(CONFIG_DEBUG_XEN) := -DDEBUG
\ No newline at end of file
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] xen: Kconfig: Add DEBUG_XEN config option
2017-02-01 11:19 [PATCH] xen: Kconfig: Add DEBUG_XEN config option Andrii Anisov
@ 2017-02-01 12:17 ` Juergen Gross
2017-02-01 12:55 ` Julien Grall
2017-02-01 12:58 ` Andrii Anisov
0 siblings, 2 replies; 6+ messages in thread
From: Juergen Gross @ 2017-02-01 12:17 UTC (permalink / raw)
To: Andrii Anisov, xen-devel
Cc: julien.grall, sstabellini, andrii_anisov, Boris Ostrovsky
On 01/02/17 12:19, Andrii Anisov wrote:
> From: Andrii Anisov <andrii_anisov@epam.com>
>
> Add a debug option to enable xen drivers debug code.
>
> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
In future you might want to add the maintainers to the recipient list.
> ---
> drivers/xen/Kconfig | 8 ++++++++
> drivers/xen/Makefile | 2 ++
> 2 files changed, 10 insertions(+)
>
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index f15bb3b7..733c433 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -298,4 +298,12 @@ config XEN_SYMS
> config XEN_HAVE_VPMU
> bool
>
> +config DEBUG_XEN
> + bool "XEN Drivers debug"
> + depends on DEBUG_KERNEL
> + help
> + Say Y here if you want to enable XEN drivers debug code.
> +
> + If you are unsure about this, say N here.
So is this really a sensible thing to do? I don't see the value for
such a global config option possibly switching so many different
drivers to debug mode.
In reality you want to do something like "debug" for a single driver
only during time of development. This won't need a global config
option but just a "#define" in the driver which is active while
developing and testing and which should be removed or commented out
when the final submission of the driver is done.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen: Kconfig: Add DEBUG_XEN config option
2017-02-01 12:17 ` Juergen Gross
@ 2017-02-01 12:55 ` Julien Grall
2017-02-01 13:12 ` Juergen Gross
2017-02-01 12:58 ` Andrii Anisov
1 sibling, 1 reply; 6+ messages in thread
From: Julien Grall @ 2017-02-01 12:55 UTC (permalink / raw)
To: Juergen Gross, Andrii Anisov, xen-devel
Cc: Boris Ostrovsky, sstabellini, andrii_anisov
Hi Juergen,
On 01/02/2017 12:17, Juergen Gross wrote:
> On 01/02/17 12:19, Andrii Anisov wrote:
>> From: Andrii Anisov <andrii_anisov@epam.com>
>>
>> Add a debug option to enable xen drivers debug code.
>>
>> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
>
> In future you might want to add the maintainers to the recipient list.
>
>> ---
>> drivers/xen/Kconfig | 8 ++++++++
>> drivers/xen/Makefile | 2 ++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>> index f15bb3b7..733c433 100644
>> --- a/drivers/xen/Kconfig
>> +++ b/drivers/xen/Kconfig
>> @@ -298,4 +298,12 @@ config XEN_SYMS
>> config XEN_HAVE_VPMU
>> bool
>>
>> +config DEBUG_XEN
>> + bool "XEN Drivers debug"
>> + depends on DEBUG_KERNEL
>> + help
>> + Say Y here if you want to enable XEN drivers debug code.
>> +
>> + If you are unsure about this, say N here.
>
> So is this really a sensible thing to do? I don't see the value for
> such a global config option possibly switching so many different
> drivers to debug mode.
>
> In reality you want to do something like "debug" for a single driver
> only during time of development. This won't need a global config
> option but just a "#define" in the driver which is active while
> developing and testing and which should be removed or commented out
> when the final submission of the driver is done.
This Kconfig was suggested in the context of [1]. The number of people
working on swiotlb is very limited, but the check added is really useful
in debug build to catch potential misuse for anyone.
Do you have any other idea to turn this check on for debug build??
Cheers,
[1] https://lists.xen.org/archives/html/xen-devel/2017-01/msg03448.html
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen: Kconfig: Add DEBUG_XEN config option
2017-02-01 12:55 ` Julien Grall
@ 2017-02-01 13:12 ` Juergen Gross
2017-02-01 14:11 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 6+ messages in thread
From: Juergen Gross @ 2017-02-01 13:12 UTC (permalink / raw)
To: Julien Grall, Andrii Anisov, xen-devel
Cc: Boris Ostrovsky, sstabellini, andrii_anisov
On 01/02/17 13:55, Julien Grall wrote:
> Hi Juergen,
>
> On 01/02/2017 12:17, Juergen Gross wrote:
>> On 01/02/17 12:19, Andrii Anisov wrote:
>>> From: Andrii Anisov <andrii_anisov@epam.com>
>>>
>>> Add a debug option to enable xen drivers debug code.
>>>
>>> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
>>
>> In future you might want to add the maintainers to the recipient list.
>>
>>> ---
>>> drivers/xen/Kconfig | 8 ++++++++
>>> drivers/xen/Makefile | 2 ++
>>> 2 files changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>>> index f15bb3b7..733c433 100644
>>> --- a/drivers/xen/Kconfig
>>> +++ b/drivers/xen/Kconfig
>>> @@ -298,4 +298,12 @@ config XEN_SYMS
>>> config XEN_HAVE_VPMU
>>> bool
>>>
>>> +config DEBUG_XEN
>>> + bool "XEN Drivers debug"
>>> + depends on DEBUG_KERNEL
>>> + help
>>> + Say Y here if you want to enable XEN drivers debug code.
>>> +
>>> + If you are unsure about this, say N here.
>>
>> So is this really a sensible thing to do? I don't see the value for
>> such a global config option possibly switching so many different
>> drivers to debug mode.
>>
>> In reality you want to do something like "debug" for a single driver
>> only during time of development. This won't need a global config
>> option but just a "#define" in the driver which is active while
>> developing and testing and which should be removed or commented out
>> when the final submission of the driver is done.
>
> This Kconfig was suggested in the context of [1]. The number of people
> working on swiotlb is very limited, but the check added is really useful
> in debug build to catch potential misuse for anyone.
>
> Do you have any other idea to turn this check on for debug build??
I think for this use case we would want either a more specific config
option name (e.g. DEBUG_XEN_SWIOTLB) or a more specific description
what it is doing, like e.g.:
"Say Y here if you want to enable XEN drivers debug code adding more
sanity checks to Xen drivers eventually crashing the kernel in case
of detected inconsistencies. Enabling this option might slow down
the kernel. It is not appropriate for production.
If you are unsure about this, say N here."
This would make clear that this option should not be used to spam the
console with thousands of messages by dozens of different drivers,
but for catching potential bugs early, and shipping a kernel with this
option enabled is not a good idea.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen: Kconfig: Add DEBUG_XEN config option
2017-02-01 13:12 ` Juergen Gross
@ 2017-02-01 14:11 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-02-01 14:11 UTC (permalink / raw)
To: Juergen Gross
Cc: sstabellini, andrii_anisov, Andrii Anisov, Julien Grall,
xen-devel, Boris Ostrovsky
On Wed, Feb 01, 2017 at 02:12:13PM +0100, Juergen Gross wrote:
> On 01/02/17 13:55, Julien Grall wrote:
> > Hi Juergen,
> >
> > On 01/02/2017 12:17, Juergen Gross wrote:
> >> On 01/02/17 12:19, Andrii Anisov wrote:
> >>> From: Andrii Anisov <andrii_anisov@epam.com>
> >>>
> >>> Add a debug option to enable xen drivers debug code.
> >>>
> >>> Signed-off-by: Andrii Anisov <andrii_anisov@epam.com>
> >>
> >> In future you might want to add the maintainers to the recipient list.
> >>
> >>> ---
> >>> drivers/xen/Kconfig | 8 ++++++++
> >>> drivers/xen/Makefile | 2 ++
> >>> 2 files changed, 10 insertions(+)
> >>>
> >>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> >>> index f15bb3b7..733c433 100644
> >>> --- a/drivers/xen/Kconfig
> >>> +++ b/drivers/xen/Kconfig
> >>> @@ -298,4 +298,12 @@ config XEN_SYMS
> >>> config XEN_HAVE_VPMU
> >>> bool
> >>>
> >>> +config DEBUG_XEN
> >>> + bool "XEN Drivers debug"
> >>> + depends on DEBUG_KERNEL
> >>> + help
> >>> + Say Y here if you want to enable XEN drivers debug code.
> >>> +
> >>> + If you are unsure about this, say N here.
> >>
> >> So is this really a sensible thing to do? I don't see the value for
> >> such a global config option possibly switching so many different
> >> drivers to debug mode.
> >>
> >> In reality you want to do something like "debug" for a single driver
> >> only during time of development. This won't need a global config
> >> option but just a "#define" in the driver which is active while
> >> developing and testing and which should be removed or commented out
> >> when the final submission of the driver is done.
> >
> > This Kconfig was suggested in the context of [1]. The number of people
> > working on swiotlb is very limited, but the check added is really useful
> > in debug build to catch potential misuse for anyone.
> >
> > Do you have any other idea to turn this check on for debug build??
>
> I think for this use case we would want either a more specific config
> option name (e.g. DEBUG_XEN_SWIOTLB) or a more specific description
> what it is doing, like e.g.:
I really don't want to add this in the Xen SWIOTLB.
It is kind of pointless - I would prefer if the debug code had
just #if 0 with a comment explaining why it is such and what it
can be used for.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen: Kconfig: Add DEBUG_XEN config option
2017-02-01 12:17 ` Juergen Gross
2017-02-01 12:55 ` Julien Grall
@ 2017-02-01 12:58 ` Andrii Anisov
1 sibling, 0 replies; 6+ messages in thread
From: Andrii Anisov @ 2017-02-01 12:58 UTC (permalink / raw)
To: Juergen Gross
Cc: xen-devel, Julien Grall, Stefano Stabellini, Andrii Anisov,
Boris Ostrovsky
Dear Juergen,
> In future you might want to add the maintainers to the recipient list.
Yep, I would take care to not miss this point.
And +1 to Julien's comment.
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-01 14:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-01 11:19 [PATCH] xen: Kconfig: Add DEBUG_XEN config option Andrii Anisov
2017-02-01 12:17 ` Juergen Gross
2017-02-01 12:55 ` Julien Grall
2017-02-01 13:12 ` Juergen Gross
2017-02-01 14:11 ` Konrad Rzeszutek Wilk
2017-02-01 12:58 ` Andrii Anisov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).