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.
> Also, maybe the message would be more useful if it included the
> conflicting resource as well as the region you're requesting? Many of
> the other callers of request_resource_conflict() have something like
> this:
>
> dev_err(dev, "resource collision: %pR conflicts with %s %pR\n",
> new, conflict->name, conflict);
Seems sane. I was just trying to change as little as possible.
