From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751579AbdJWSU1 (ORCPT ); Mon, 23 Oct 2017 14:20:27 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:48158 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932109AbdJWSUZ (ORCPT ); Mon, 23 Oct 2017 14:20:25 -0400 Subject: Re: [PATCH 1/1] mm:hugetlbfs: Fix hwpoison reserve accounting To: Naoya Horiguchi Cc: "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , Michal Hocko , Aneesh Kumar , Anshuman Khandual , Andrew Morton , "stable@vger.kernel.org" References: <20171019230007.17043-1-mike.kravetz@oracle.com> <20171019230007.17043-2-mike.kravetz@oracle.com> <20171020023019.GA9318@hori1.linux.bs1.fc.nec.co.jp> <5016e528-8ea9-7597-3420-086ae57f3d9d@oracle.com> <20171023073258.GA5115@hori1.linux.bs1.fc.nec.co.jp> From: Mike Kravetz Message-ID: <26945734-ac7e-f71e-dbfa-0b0f0fdaff32@oracle.com> Date: Mon, 23 Oct 2017 11:20:02 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20171023073258.GA5115@hori1.linux.bs1.fc.nec.co.jp> Content-Type: text/plain; charset=iso-2022-jp Content-Language: en-US Content-Transfer-Encoding: 7bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/23/2017 12:32 AM, Naoya Horiguchi wrote: > On Fri, Oct 20, 2017 at 10:49:46AM -0700, Mike Kravetz wrote: >> On 10/19/2017 07:30 PM, Naoya Horiguchi wrote: >>> On Thu, Oct 19, 2017 at 04:00:07PM -0700, Mike Kravetz wrote: >>> >>> Thank you for addressing this. The patch itself looks good to me, but >>> the reported issue (negative reserve count) doesn't reproduce in my trial >>> with v4.14-rc5, so could you share the exact procedure for this issue? >> >> Sure, but first one question on your test scenario below. >> >>> >>> When error handler runs over a huge page, the reserve count is incremented >>> so I'm not sure why the reserve count goes negative. >> >> I'm not sure I follow. What specific code is incrementing the reserve >> count? > > The call path is like below: > > hugetlbfs_error_remove_page > hugetlb_fix_reserve_counts > hugepage_subpool_get_pages(spool, 1) > hugetlb_acct_memory(h, 1); > gather_surplus_pages > h->resv_huge_pages += delta; > Ah OK. This is a result of call to hugetlb_fix_reserve_counts which I believe is incorrect in most instances, and is unlikely to happen with my patch. >> >> Remove the file (rm /var/opt/oracle/hugepool/foo) >> ------------------------------------------------- >> HugePages_Total: 1 >> HugePages_Free: 0 >> HugePages_Rsvd: 18446744073709551615 >> HugePages_Surp: 0 >> Hugepagesize: 2048 kB >> >> I am still confused about how your test maintains a reserve count after >> poisoning. It may be a good idea for you to test my patch with your >> test scenario as I can not recreate here. > > Interestingly, I found that this reproduces if all hugetlb pages are > reserved when poisoning. > Your testing meets the condition, and mine doesn't. > > In gather_surplus_pages() we determine whether we extend hugetlb pool > with surplus pages like below: > > needed = (h->resv_huge_pages + delta) - h->free_huge_pages; > if (needed <= 0) { > h->resv_huge_pages += delta; > return 0; > } > ... > > needed is 1 if h->resv_huge_pages == h->free_huge_pages, and then > the reserve count gets inconsistent. > I confirmed that your patch fixes the issue, so I'm OK with it. Thanks. That now makes sense to me. hugetlb_fix_reserve_counts (which results in gather_surplus_pages being called), is only designed to be called in the extremely rare cases when we have free'ed a huge page but are unable to free the reservation entry. Just curious, when the hugetlb_fix_reserve_counts call was added to hugetlbfs_error_remove_page, was the intention to preserve the original reservation? I remember thinking hard about that for the hole punch case and came to the conclusion that it was easier and less error prone to remove the reservation as well. That will also happen in the error case with the patch I provided. -- Mike Kravetz > > Acked-by: Naoya Horiguchi > > Thanks, > Naoya Horiguchi >