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 S1752593AbbFKJuB (ORCPT <rfc822;w@1wt.eu>);
	Thu, 11 Jun 2015 05:50:01 -0400
Received: from cantor2.suse.de ([195.135.220.15]:60729 "EHLO mx2.suse.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751101AbbFKJtw (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Thu, 11 Jun 2015 05:49:52 -0400
Message-ID: <557959BC.5000303@suse.cz>
Date: Thu, 11 Jun 2015 11:49:48 +0200
From: Vlastimil Babka <vbabka@suse.cz>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0
MIME-Version: 1.0
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Hugh Dickins <hughd@google.com>
CC: Dave Hansen <dave.hansen@intel.com>, Mel Gorman <mgorman@suse.de>,
        Rik van Riel <riel@redhat.com>, Christoph Lameter <cl@gentwo.org>,
        Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
        Steve Capper <steve.capper@linaro.org>,
        "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
        Johannes Weiner <hannes@cmpxchg.org>, Michal Hocko <mhocko@suse.cz>,
        Jerome Marchand <jmarchan@redhat.com>,
        Sasha Levin <sasha.levin@oracle.com>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org
Subject: Re: [PATCHv6 29/36] thp: implement split_huge_pmd()
References: <1433351167-125878-1-git-send-email-kirill.shutemov@linux.intel.com> <1433351167-125878-30-git-send-email-kirill.shutemov@linux.intel.com>
In-Reply-To: <1433351167-125878-30-git-send-email-kirill.shutemov@linux.intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
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 06/03/2015 07:06 PM, Kirill A. Shutemov wrote:
> Original split_huge_page() combined two operations: splitting PMDs into
> tables of PTEs and splitting underlying compound page. This patch
> implements split_huge_pmd() which split given PMD without splitting
> other PMDs this page mapped with or underlying compound page.
>
> Without tail page refcounting, implementation of split_huge_pmd() is
> pretty straight-forward.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Tested-by: Sasha Levin <sasha.levin@oracle.com>

[...]

> +
> +	if (atomic_add_negative(-1, compound_mapcount_ptr(page))) {
> +		/* Last compound_mapcount is gone. */
> +		__dec_zone_page_state(page, NR_ANON_TRANSPARENT_HUGEPAGES);
> +		if (PageDoubleMap(page)) {
> +			/* No need in mapcount reference anymore */
> +			ClearPageDoubleMap(page);
> +			for (i = 0; i < HPAGE_PMD_NR; i++)
> +				atomic_dec(&page[i]._mapcount);
> +		}
> +	} else if (!TestSetPageDoubleMap(page)) {
> +		/*
> +		 * The first PMD split for the compound page and we still
> +		 * have other PMD mapping of the page: bump _mapcount in
> +		 * every small page.
> +		 * This reference will go away with last compound_mapcount.
> +		 */
> +		for (i = 0; i < HPAGE_PMD_NR; i++)
> +			atomic_inc(&page[i]._mapcount);

The order of actions here means that between TestSetPageDoubleMap() and 
the atomic incs, anyone calling page_mapcount() on one of the pages not 
processed by the for loop yet, will see a value lower by 1 from what he 
should see. I wonder if that can cause any trouble somewhere, especially 
if there's only one other compound mapping and page_mapcount() will 
return 0 instead of 1?

Conversely, when clearing PageDoubleMap() above (or in one of those rmap 
functions IIRC), one could see mapcount inflated by one. But I guess 
that's less dangerous.