Re: [PATCH 2/5] mm/resource: move HMM pr_debug() deeper into resource code

From: Michael Ellerman
Date: Mon Jan 28 2019 - 20:34:59 EST


Dave Hansen <dave.hansen@xxxxxxxxx> writes:
> On 1/25/19 1:18 PM, Bjorn Helgaas wrote:
>> On Thu, Jan 24, 2019 at 5:21 PM Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> wrote:
>>> diff -puN kernel/resource.c~move-request_region-check kernel/resource.c
>>> --- a/kernel/resource.c~move-request_region-check 2019-01-24 15:13:14.453199539 -0800
>>> +++ b/kernel/resource.c 2019-01-24 15:13:14.458199539 -0800
>>> @@ -1123,6 +1123,16 @@ struct resource * __request_region(struc
>>> conflict = __request_resource(parent, res);
>>> if (!conflict)
>>> break;
>>> + /*
>>> + * mm/hmm.c reserves physical addresses which then
>>> + * become unavailable to other users. Conflicts are
>>> + * not expected. Be verbose if one is encountered.
>>> + */
>>> + if (conflict->desc == IORES_DESC_DEVICE_PRIVATE_MEMORY) {
>>> + pr_debug("Resource conflict with unaddressable "
>>> + "device memory at %#010llx !\n",
>>> + (unsigned long long)start);
>>
>> I don't object to the change, but are you really OK with this being a
>> pr_debug() message that is only emitted when enabled via either the
>> dynamic debug mechanism or DEBUG being defined? From the comments, it
>> seems more like a KERN_INFO sort of message.
>
> I left it consistent with the original message that was in the code.
> I'm happy to change it, though, if the consumers of it (Jerome,
> basically) want something different.

At least using pr_debug() doesn't match the comment, ie. the comment
says "Be verbose" but pr_debug() is silent by default.

cheers