Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v4 4/8] hugetlb: create remove_hugetlb_page() to separate
 functionality
To:     Oscar Salvador <osalvador@suse.de>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Roman Gushchin <guro@fb.com>, Michal Hocko <mhocko@suse.com>,
        Shakeel Butt <shakeelb@google.com>,
        David Hildenbrand <david@redhat.com>,
        Muchun Song <songmuchun@bytedance.com>,
        David Rientjes <rientjes@google.com>,
        Miaohe Lin <linmiaohe@huawei.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Matthew Wilcox <willy@infradead.org>,
        HORIGUCHI NAOYA <naoya.horiguchi@nec.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>,
        Waiman Long <longman@redhat.com>, Peter Xu <peterx@redhat.com>,
        Mina Almasry <almasrymina@google.com>,
        Hillf Danton <hdanton@sina.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Barry Song <song.bao.hua@hisilicon.com>,
        Will Deacon <will@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>
References: <20210405230043.182734-1-mike.kravetz@oracle.com>
 <20210405230043.182734-5-mike.kravetz@oracle.com>
 <YGxk85nXJD+NxzYX@localhost.localdomain>
From:   Mike Kravetz <mike.kravetz@oracle.com>
Message-ID: <86020115-11af-e4fc-9b42-4fe809f7b26c@oracle.com>
Date:   Tue, 6 Apr 2021 13:52:57 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.1
In-Reply-To: <YGxk85nXJD+NxzYX@localhost.localdomain>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from [192.168.2.112] (50.38.35.18) by MWHPR03CA0018.namprd03.prod.outlook.com (2603:10b6:300:117::28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4020.16 via Frontend Transport; Tue, 6 Apr 2021 20:52:59 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 1210dc19-57af-4679-8109-08d8f93df6e0
X-MS-TrafficTypeDiagnostic: BYAPR10MB3400:
X-Microsoft-Antispam-PRVS: <BYAPR10MB3400791B66304A59A215196EE2769@BYAPR10MB3400.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:9508;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: +F/0grmDQAND1c2p/iKKHcbVOxK9/XjcOzk8wY+Z43Lz6ACtUKT4gDnfbC19e6di1cZYUljuPfTHvPvthmgCmAcajmeMqR7IZ266StmTvHwkUs9BTqaVjpmI6UHO05CP00oDGXO1/6Aq/P7PevjOv1P6Y8Jx5l9OpOD8d39v9t5ewn9AOjeTsbBfziZOXfVIoUXFzxdn7DdGnzrl1t+/+9jGYRl2wtA628FiaBA5FEt1lfAqRcFnnl1SD7jb5u/yvU2XFgLj+jYaFWMfnXXXbJ/6m/znmFYla2iiK2qGBoF+pcupvvuEhgri2iv3VB5/qrCeIU6xELihNY9JoJRUyqrjf+M3P/yn3XnMsHSUoFvtOnA+rGHoaHFD8XJxTTGf4x4Il0KXEsmw8GWYfSw6UGxm6Z2x3p5Zrn90gkiO/PSWw44wc5N0MAITUdLUXPMuMAKXh2ghvnz+uA5CgYALMIKIaWinGIIsc4BFq3CShSViaTstJm3n8JfgHMHzIcSxcLNi03ZvHItBTOLqjkGF8G/zrDEXpCoWN1P5zu+4d4tuCm0hydUv02LMAgDv5ijeqL30mTOW1Afk0y6WimRMfgfx6CaPev9GtcK4Z127NeD683h63TBM3lw9q3jDL7Z0NScmNnT86ImBNcveCCLnuIIDlQIA3Cy+BnXjpPvcgNFQ9Tk+nkT1RtHQzQatls88d4eZ9cUx/cgiBSgKZtbKDqRfQoz5u6RDIRURcQTE8MkWO/x//Hrih+t1XPmZRTxH
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BY5PR10MB4196.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(366004)(396003)(136003)(376002)(346002)(39860400002)(83380400001)(66946007)(478600001)(6916009)(86362001)(8936002)(54906003)(36756003)(38350700001)(4326008)(2616005)(5660300002)(956004)(44832011)(66556008)(316002)(8676002)(186003)(16526019)(66476007)(31696002)(6486002)(52116002)(2906002)(53546011)(38100700001)(7416002)(26005)(16576012)(31686004)(45980500001)(43740500002);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?ajd2U1FZSVhseWlWa0hwNmFyNUZlbEFYQ1F4Ym1pRTZ1Y0hEMTdwb3BRY1lp?=
 =?utf-8?B?bXRBcVNtTDZidndXbmo5N2g4L29DSTMvdTVsMXJoTFlUZ1h6M3RWZjRwajI3?=
 =?utf-8?B?cjBGMlZReVk0SERna0JXTlRtS1JvUEdNNnRyQ2hCb1lSWXZBd2RsekY1bWxZ?=
 =?utf-8?B?NUJydkRjT1pORFpYenhhUE1BanVRMzRHc3FWRnUya3V4UzRac0xmWHZ0bHk2?=
 =?utf-8?B?b2dUMXh5VVJNdS9HK0U4ZlhXMnJEWWZMZWQ3NUhSUERpVFZybmcwait3Tmht?=
 =?utf-8?B?Y2lRU2R2b1dJdXh4MEQ2Qko0RXRmczNERS94TmZDTzVpUjhJMHJuMXd6ZEdn?=
 =?utf-8?B?d1ErT2VrNFcxd3dIVVVhOThMc0U3bXRNN3pBTTFkR2l2WW8vaHdRQ0xTUnVu?=
 =?utf-8?B?SW1uK3M3TlgxTkFoOWJ3SkhKQUgwRHpuR0IraWhscnBydUJoMThTLzZSUU9P?=
 =?utf-8?B?dzJqRDlRd28rMVFjU3haQkNvd0RjZEQzdDkxWit1Q3NxbE41REJ1MnlHTGd5?=
 =?utf-8?B?MW0xSVRaOEpSKzBveDI5RHY5NWlrUG4vVjdBU1N4N3BUc0cwQUtiRWFTMVBZ?=
 =?utf-8?B?K1lseHFVM3BXM1ZxNGtxYVRiT3Uzajk4QzRKSFVFeHl6T0JSRmhKbVM2Q0E4?=
 =?utf-8?B?WHFtVThNOEpyMFNscUJWRDdXNTBwTW9xNVpvK0xiUzZYMWYyRjIvZUY5d3Fn?=
 =?utf-8?B?aVQ1VkF4R3BtSURiWXpWdTYxL09VSk85L0JaUGZhUDRYUGRhMkUvb2l0M29t?=
 =?utf-8?B?eVlBMHIvaWlCbVBpcFhhTjJheFlnRkMyNTd5UlJWNU4rN0xWMG44aEFEVkFV?=
 =?utf-8?B?blJ3NDVtSnRXeFN0MFlBMURjYlVIS25PWEdrUTVPcVF5ZmxpT1BtY3lkZ1dT?=
 =?utf-8?B?VkZzUmxySmFsNGk4OUdOdzczdnd4SVlORGMxUFNpRnR5SkRLNURYZ2FQUGhQ?=
 =?utf-8?B?OG16aGpBV2U4WldKaWdlYnQwcVVDbklMNFJFaEc0Tlg1bEdRUlVESVhQc3k5?=
 =?utf-8?B?U0FabmpOcUE5cGRiTlYweGc2SGlaNDZKcUdtWDl1MHVJVkR1SW1KT1JwNnVR?=
 =?utf-8?B?V0ViZ2gzR0dOZGovZ3VxUTdmWnVSVzg3MDdsV1JQZDlTV0QzMFRDUmhmZTNH?=
 =?utf-8?B?YTI3UGsxV25kSVVlV1REQnc0N0tkdld0dk5Gc0M4akVXT29CWFBob0wrZVJU?=
 =?utf-8?B?V0QrNzNzY216d3VrdURFdEM4NXBNZWRvU3ZYTzgySUZ3dEZ5aGh4eEdmS3FD?=
 =?utf-8?B?ZXI1MmRPQ2JDbHNKVTIvSVNXUmV5QTZ1VTBCSGJseHF6N0RTMk1VVTJGeXVO?=
 =?utf-8?B?L0JNU0pQeGVnU0FzbmJad0MyOEVzcitYV0hnY3MwK2R0a0Y1MzZQREFqN2hX?=
 =?utf-8?B?WDlxbGlsWTRzeFRSWlY3UWhLanovb0pRcWg5R3NJOG5yS1YwTTdBSmFEenMr?=
 =?utf-8?B?RmtsVG8rRmh4R2pFTzdVbXg0ZG9tRjZxNGFuUWlYcjNlbXBSbVIvZ3lKNGl1?=
 =?utf-8?B?Vzg1TnowSldyQSs0c2lpUUREdUpqa1YrNkJBNkVMa0JZSHNxZE1sKzd1WElp?=
 =?utf-8?B?WlRZVVdCVWNUdytKaEgraDNPL3dkRUhrUDNocXBjbXl3M0ZxV215Zi9Dbjcw?=
 =?utf-8?B?djJQaEswWFNBUVFRVENTUmRjRndEaTBYRFdVTG1teTRWM0JQSFVmdVA3Sncy?=
 =?utf-8?B?Vk5DclZCV1FORUpjc3c5MldXREdVdVI5ak5GOHhqRGNXcGxKajlCNm0yclhR?=
 =?utf-8?Q?rwZbAZm9wfWEnwOMpy5LBzDXxzJCVOnCdSNMQ/o?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1210dc19-57af-4679-8109-08d8f93df6e0
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Apr 2021 20:53:00.8710
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: u4suIbX9Y9Mv5iXFo0JA5DhSD8Zu8kZQhlQWnGHeRz1TN9xzIlxR+MPzRZuJsx1/sLBpfmQy3W/9jmra+gM/JQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3400
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9946 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0
 suspectscore=0 phishscore=0 malwarescore=0 mlxscore=0 spamscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104060000 definitions=main-2104060142
X-Proofpoint-GUID: 9MTuZPCxOul4eTxLCiviRsly4Yb8oYxz
X-Proofpoint-ORIG-GUID: 9MTuZPCxOul4eTxLCiviRsly4Yb8oYxz
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9946 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 mlxlogscore=999
 suspectscore=0 spamscore=0 phishscore=0 clxscore=1015 bulkscore=0
 mlxscore=0 lowpriorityscore=0 adultscore=0 malwarescore=0 impostorscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104060000
 definitions=main-2104060142
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 4/6/21 6:41 AM, Oscar Salvador wrote:
> On Mon, Apr 05, 2021 at 04:00:39PM -0700, Mike Kravetz wrote:
>> +static void remove_hugetlb_page(struct hstate *h, struct page *page,
>> +							bool adjust_surplus)
>> +{
>> +	int nid = page_to_nid(page);
>> +
>> +	if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
>> +		return;
>> +
>> +	list_del(&page->lru);
>> +
>> +	if (HPageFreed(page)) {
>> +		h->free_huge_pages--;
>> +		h->free_huge_pages_node[nid]--;
>> +		ClearHPageFreed(page);
>> +	}
>> +	if (adjust_surplus) {
>> +		h->surplus_huge_pages--;
>> +		h->surplus_huge_pages_node[nid]--;
>> +	}
>> +
>> +	VM_BUG_ON_PAGE(hugetlb_cgroup_from_page(page), page);
>> +	VM_BUG_ON_PAGE(hugetlb_cgroup_from_page_rsvd(page), page);
> 
> These checks feel a bit odd here.
> I would move them above, before we start messing with the counters?
> 

This routine is comprised of code that was previously in update_and_free_page
and __free_huge_page.   In those routines, the VM_BUG_ON_PAGE came after the
counter adjustments.  That is the only reason they are positioned as they are.

I agree that it makes more sense to add them to the beginning of the routine.

>> +
>> +	ClearHPageTemporary(page);
> 
> Why clearing it unconditionally? I guess we do not really care, but
> someone might wonder why when reading the core.
> So I would either do as we used to do and only clear it in case of
> HPageTemporary(), or drop a comment.
> 

Technically, the HPage* flags are meaningless after calling this
routine.  So, there really is no need to modify them at all.  The
flag clearing code is left over from the routines in which they
previously existed.

Any clearing of HPage* flags in this routine is unnecessary and should
be removed to avoid any questions.
-- 
Mike Kravetz