linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Baoquan He <bhe@redhat.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	david@redhat.com, akpm@linux-foundation.org
Subject: Re: [PATCH v2 4/4] mm/hugetl.c: warn out if expected count of huge pages adjustment is not achieved
Date: Thu, 23 Jul 2020 11:21:52 -0700	[thread overview]
Message-ID: <b94f4dc1-5c53-68ca-2023-0aa4de4df8b7@oracle.com> (raw)
In-Reply-To: <20200723091142.GR32539@MiWiFi-R3L-srv>

On 7/23/20 2:11 AM, Baoquan He wrote:
> On 07/23/20 at 11:46am, Anshuman Khandual wrote:
>>
>>
>> On 07/23/2020 08:52 AM, Baoquan He wrote:
>>> A customer complained that no message is logged wh	en the number of
>>> persistent huge pages is not changed to the exact value written to
>>> the sysfs or proc nr_hugepages file.
>>>
>>> In the current code, a best effort is made to satisfy requests made
>>> via the nr_hugepages file.  However, requests may be only partially
>>> satisfied.
>>>
>>> Log a message if the code was unsuccessful in fully satisfying a
>>> request. This includes both increasing and decreasing the number
>>> of persistent huge pages.
>>
>> But is kernel expected to warn for all such situations where the user
>> requested resources could not be allocated completely ? Otherwise, it
>> does not make sense to add an warning for just one such situation.
> 
> It's not for just one such situation, we have already had one to warn
> out in mm/hugetlb.c, please check hugetlb_hstate_alloc_pages().

Those are a little different in that they are warnings based on kernel
command line parameters.

> As Mike said, in one time of persistent huge page number setting,
> comparing the old value with the new vlaue is good enough for customer
> to get the information. However, if customer want to detect and analyze
> previous setting failure, logging message will be helpful. So I think
> logging the failure or partial success makes sense.

I can understand the argument against adding a new warning for this.
You could even argue that this condition has existed since the time
hugetlb was added to the kernel which was long ago.  And, nobody has
complained enough to add a warning.  I have even heard of a sysadmin
practice of asking for a ridiculously large amount of hugetlb pages
just so that the kernel will allocate as many as possible.  They do
not 'expect' to get the ridiculous amount they asked for.  In such
cases, this will be a new warning in their log.

As mentioned in a previous e-mail, when one sets nr_hugepages by writing
to the sysfs or proc file, one needs to read the file to determine if the
number of requested pages were actually allocated.  Anyone who does not
do this is just asking for trouble.  Yet, I imagine that it may happen.

To be honest, I do not see this log message as something that would be
helpful to end users.  Rather, I could see this as being useful to support
people.  Support always asks for system logs and this could point out a
possible issue with hugetlb usage.

I do not feel strongly one way or another about adding the warning.  Since
it is fairly trivial and could help diagnose issues I am in favor of adding
it.  If people feel strongly that it should not be added, I am open to
those arguments.
-- 
Mike Kravetz


  reply	other threads:[~2020-07-23 18:24 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-23  3:22 [PATCH v2 0/4] mm/hugetlb: Small cleanup and improvement Baoquan He
2020-07-23  3:22 ` [PATCH v2 1/4] mm/hugetlb.c: make is_hugetlb_entry_hwpoisoned return bool Baoquan He
2020-07-23  4:46   ` Anshuman Khandual
2020-07-23  3:22 ` [PATCH v2 2/4] mm/hugetlb.c: Remove the unnecessary non_swap_entry() Baoquan He
2020-07-23  5:06   ` Anshuman Khandual
2020-07-23  6:14     ` Baoquan He
2020-07-23  8:52       ` Anshuman Khandual
2020-07-23 10:46   ` [PATCH v3 " Baoquan He
2020-07-23  3:22 ` [PATCH v2 3/4] doc/vm: fix typo in the hugetlb admin documentation Baoquan He
2020-07-23  5:17   ` Anshuman Khandual
2020-07-23  5:57     ` Baoquan He
2020-07-23  3:22 ` [PATCH v2 4/4] mm/hugetl.c: warn out if expected count of huge pages adjustment is not achieved Baoquan He
2020-07-23  6:16   ` Anshuman Khandual
2020-07-23  9:11     ` Baoquan He
2020-07-23 18:21       ` Mike Kravetz [this message]
2020-07-24 14:59         ` Baoquan He
2020-08-11  2:11         ` Baoquan He
2020-08-11  3:35           ` Mike Kravetz
2020-08-11  7:24             ` Michal Hocko
2020-08-11 23:11               ` Mike Kravetz
2020-08-24 21:57 ` [PATCH v2 0/4] mm/hugetlb: Small cleanup and improvement Mike Kravetz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b94f4dc1-5c53-68ca-2023-0aa4de4df8b7@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=bhe@redhat.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).