Re: [Xen-devel] [PATCH] xen: Kconfig: Add DEBUG_XEN config option

[Juergen Gross <jgross@xxxxxxxx>]

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@xxxxxxxx>
>>>
>>> Add a debug option to enable xen drivers debug code.
>>>
>>> Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
>>
>> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel