[PATCH] powerpc/mm/nvdimm: Add an informative message if we fail to allocate altmap block

[Michael Ellerman]

"Aneesh Kumar K.V" <aneesh.kumar at linux.ibm.com> writes:
> "Oliver O'Halloran" <oohall at gmail.com> writes:
>
>> On Sat, Jun 29, 2019 at 5:39 PM Aneesh Kumar K.V
>> <aneesh.kumar at linux.ibm.com> wrote:
>>>
>>> Allocation from altmap area can fail based on vmemmap page size used. Add kernel
>>> info message to indicate the failure. That allows the user to identify whether they
>>> are really using persistent memory reserved space for per-page metadata.
>>>
>>> The message looks like:
>>> [  136.587212] altmap block allocation failed, falling back to system memory
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>
>>> ---
>>>  arch/powerpc/mm/init_64.c | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
>>> index a4e17a979e45..57c0573650dc 100644
>>> --- a/arch/powerpc/mm/init_64.c
>>> +++ b/arch/powerpc/mm/init_64.c
>>> @@ -194,8 +194,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
>>>                  * fail due to alignment issues when using 16MB hugepages, so
>>>                  * fall back to system memory if the altmap allocation fail.
>>>                  */
>>> -               if (altmap)
>>> +               if (altmap) {
>>>                         p = altmap_alloc_block_buf(page_size, altmap);
>>> +                       if (!p)
>>
>>> +                               pr_info("altmap block allocation failed, " \
>>> +                                       "falling back to system memory");
>>
>> I think this is kind of misleading. If you're mapping a large amount
>> of memory you can have most of the vmemmap backing allocated from the
>> altmap and one extra block allocated from normal memory. E.g. If you
>> have 32MB of altmap space, one 16MB block will be allocated from the
>> altmap, but the 2nd 16MB block is probably unusable due to the
>> reserved pages at the start of the altmap. Maybe this should be a
>> pr_debug() so it's only printed along with the "vmemmap_populate ..."
>> message above?
>
> Will switch to pr_debug. What I really wanted was an indication of which
> pfn device failed to allocate  per page meata data in the device. But
> we really don't have device details here and we don't end up calling
> this function if there is already a 16MB mapping in DRAM for this area.
>
>>
>> Also, isn't kernel style to keep printf()s, even long ones, on one line?
>
> I was not sure. It do print to kernel log in one line.

It should be one line so it's easy to grep for the error string.

cheers