Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2] powerpc/mm: move a KERN_WARNING message to pr_debug()
To:     David Gibson <david@gibson.dropbear.id.au>
Cc:     linux-kernel@vger.kernel.org,
        Michael Ellerman <mpe@ellerman.id.au>,
        linuxppc-dev@lists.ozlabs.org,
        Christophe Leroy <christophe.leroy@c-s.fr>
References: <20190205202133.5048-1-lvivier@redhat.com>
 <20190207030305.GA518@umbus.fritz.box>
From:   Laurent Vivier <lvivier@redhat.com>
Message-ID: <632657bb-acd7-54bd-ac02-0d6f9d87d26b@redhat.com>
Date:   Thu, 7 Feb 2019 10:25:09 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.8.0
MIME-Version: 1.0
In-Reply-To: <20190207030305.GA518@umbus.fritz.box>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 07/02/2019 04:03, David Gibson wrote:
> On Tue, Feb 05, 2019 at 09:21:33PM +0100, Laurent Vivier wrote:
>> resize_hpt_for_hotplug() reports a warning when it cannot
>> increase the hash page table ("Unable to resize hash page
>> table to target order") but this is not blocking and
>> can make user thinks something has not worked properly.
>> As we move the message to the debug area, report again the
>> ENODEV error.
>>
>> If the operation cannot be done the real error message
>> will be reported by arch_add_memory() if create_section_mapping()
>> fails.
>>
>> Fixes: 7339390d772dd
>>        powerpc/pseries: Don't give a warning when HPT resizing isn't available
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> Sorry, I'm pretty dubious about this.  It's true that in the case for
> which this bug was filed this is a harmless situation which deserves a
> pr_debug() at most.
> 
> But that's not necessarily true in all paths leading to this message.
> It will also trip if we fail to reshrink the HPT after genuinely
> hotunplugging a bunch of memory, in which case failing to release
> expected resources does deserve a warning.

But if there is a real problem this function should return an error and
this error should be managed by the caller.

Moreover, the function that can fail (pseries_lpar_resize_hpt()) has
already a warning for each error case:

ETIMEDOUT: "Unexpected error %d cancelling timed out HPT resize\n"
EIO:       "Unexpected error %d from H_RESIZE_HPT_PREPARE\n"
           "Unexpected error %d from H_RESIZE_HPT_COMMIT\n
ENOSPC:    "Hash collision while resizing HPT\n"

ENODEV has no error message but is already silently ignored.
EINVAL and EPERM have no message but this happens if hcall is not used
correctly and deserve only a pr_debug() I think.

Thanks,
Laurent