From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1759549AbcIMRcU (ORCPT <rfc822;w@1wt.eu>);
        Tue, 13 Sep 2016 13:32:20 -0400
Received: from mga14.intel.com ([192.55.52.115]:64333 "EHLO mga14.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1753968AbcIMRcT (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 13 Sep 2016 13:32:19 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.30,329,1470726000"; 
   d="scan'208";a="878349286"
Subject: Re: [PATCH] memory-hotplug: Fix bad area access on
 dissolve_free_huge_pages()
To: Rui Teng <rui.teng@linux.vnet.ibm.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
References: <1473755948-13215-1-git-send-email-rui.teng@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
        Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
        Michal Hocko <mhocko@suse.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.vnet.ibm.com>,
        Paul Gortmaker <paul.gortmaker@windriver.com>,
        Santhosh G <santhog4@in.ibm.com>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
From: Dave Hansen <dave.hansen@linux.intel.com>
Message-ID: <57D83821.4090804@linux.intel.com>
Date: Tue, 13 Sep 2016 10:32:17 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.8.0
MIME-Version: 1.0
In-Reply-To: <1473755948-13215-1-git-send-email-rui.teng@linux.vnet.ibm.com>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 09/13/2016 01:39 AM, Rui Teng wrote:
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 87e11d8..64b5f81 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1442,7 +1442,7 @@ static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed,
>  static void dissolve_free_huge_page(struct page *page)
>  {
>  	spin_lock(&hugetlb_lock);
> -	if (PageHuge(page) && !page_count(page)) {
> +	if (PageHuge(page) && !page_count(page) && PageHead(page)) {
>  		struct hstate *h = page_hstate(page);
>  		int nid = page_to_nid(page);
>  		list_del(&page->lru);

This is goofy.  What is calling dissolve_free_huge_page() on a tail page?

Hmm:

>         for (pfn = start_pfn; pfn < end_pfn; pfn += 1 << minimum_order)
>                 dissolve_free_huge_page(pfn_to_page(pfn));

So, skip through the area being offlined at the smallest huge page size,
and try to dissolve a huge page in each place one might appear.  But,
after we dissolve a 16GB huge page, we continue looking through the
remaining 15.98GB tail area for huge pages in the area we just
dissolved.  The tail pages are still PageHuge() (how??), and we call
page_hstate() on the tail page whose head was just dissolved.

Note, even with the fix, this taking a (global) spinlock 1023 more times
that it doesn't have to.

This seems inefficient, and fails to fully explain what is going on, and
how tail pages still _look_ like PageHuge(), which seems pretty wrong.

I guess the patch _works_.  But, sheesh, it leaves a lot of room for
improvement.