From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 139C4C4332F for ; Fri, 9 Dec 2022 14:28:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A23818E0005; Fri, 9 Dec 2022 09:28:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9D3398E0001; Fri, 9 Dec 2022 09:28:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 89B448E0005; Fri, 9 Dec 2022 09:28:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 7B1188E0001 for ; Fri, 9 Dec 2022 09:28:21 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 3552B160FF3 for ; Fri, 9 Dec 2022 14:28:21 +0000 (UTC) X-FDA: 80222998002.04.7301049 Received: from out-49.mta0.migadu.com (out-49.mta0.migadu.com [91.218.175.49]) by imf11.hostedemail.com (Postfix) with ESMTP id B96E440017 for ; Fri, 9 Dec 2022 14:28:17 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=mnihHuRT; spf=pass (imf11.hostedemail.com: domain of muchun.song@linux.dev designates 91.218.175.49 as permitted sender) smtp.mailfrom=muchun.song@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1670596098; a=rsa-sha256; cv=none; b=t3T42tMg2hV5KKFmJvpz9QkSYvmcfNuNDrGhDWC7xWklT2dz3RWPnnsg4fsSFqqC5zxvV/ UhMwziX+LX+2E3r8I+rdSvGe6RGcMPrk5ZCzzI9Y1E2vdgAOsU6s4L+PtDlo//WBBa8I/b XsH3Oe/xIWwWMXtGU01rz5Nc9sdeqsU= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=mnihHuRT; spf=pass (imf11.hostedemail.com: domain of muchun.song@linux.dev designates 91.218.175.49 as permitted sender) smtp.mailfrom=muchun.song@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1670596098; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=L5PBJXFzAfljLL5GpFuCZtP5a/X5elUo4Ca2Dzwt22Y=; b=bXbB4PWOG9zQVGzhrTtiedvp1Kq12ui6GpzboFDCUHhdouytvGzXMj5KQGrex/pAdDZr7b pmVUpHsu0XOPSVePpdMd2MIToBOLEz85SQDNVcx/u5VcrwZtH+s848Et6invjMGW96MNRY qhrZL5dnPBA2sSD4cVKmYM4qvTuQ18o= Content-Type: text/plain; charset=us-ascii DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1670596095; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=L5PBJXFzAfljLL5GpFuCZtP5a/X5elUo4Ca2Dzwt22Y=; b=mnihHuRTQhvfXW58YLWebiCzuZVttBgCe0CuYAQ4KmPqSErVy/jvpK2M6SeT+oXEFJX2EW mbOTmMwzSNghV5XF4WLLO/vNPbZXBlsMlDHPzCh5Dun9J9GqOQCFbbiP9wRUHhn1yHV19v BegeO8C3lX7mUD+A7xTP1A7YTstX6rI= Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.200.110.1.12\)) Subject: Re: [PATCH mm-unstable] mm: clarify folio_set_compound_order() zero support X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Muchun Song In-Reply-To: <7d72778e-7305-18e9-edf4-ed55a98aa7e2@nvidia.com> Date: Fri, 9 Dec 2022 22:27:38 +0800 Cc: Sidhartha Kumar , Mike Kravetz , Matthew Wilcox , linux-kernel@vger.kernel.org, Linux Memory Management List , Andrew Morton , Muchun Song , tsahu@linux.ibm.com, David Hildenbrand Content-Transfer-Encoding: quoted-printable Message-Id: <00222280-DBDD-49A3-92A5-05112359AE30@linux.dev> References: <20221207223731.32784-1-sidhartha.kumar@oracle.com> <92965844-c430-8b8e-d9f1-705d7578bceb@nvidia.com> <0187f9c2-e80a-9cde-68bc-c9bdbd96b6fe@oracle.com> <2723541a-79aa-c6b5-d82c-53db76b78145@oracle.com> <36ddac45-ecd0-e2d2-e974-8c85ca503053@oracle.com> <20cc2088-b66e-28d1-a529-414e82146336@nvidia.com> <434a111c-7f1a-0018-6bd2-561cb382deea@oracle.com> <7d72778e-7305-18e9-edf4-ed55a98aa7e2@nvidia.com> To: John Hubbard X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: B96E440017 X-Stat-Signature: dbggqiyoeb6upgucma4ys7ewz9zfw1h3 X-HE-Tag: 1670596097-452422 X-HE-Meta: U2FsdGVkX18LnW37oTdZoxJzIVT5uWjdb5ghpNK9gu9y87uwGUbJ9Mk144enjDr6oJ6O2beCC3xNRbtxBuifVJvUbpxVi0FBobyGZTWh9+aVkiw2Y1hocn6b7wePsLTrFLFY7RC0++s29mlMA9oQUsJeu/wHytGwsYmVt4tMOXimeV6ZQxrG8R5jJ7XZTtYIuT5ylDsQDKb5lQ4TUKEa7Q5wv+sztHRNbHIrtXX2FiGqHM/r5Xyo8pgCc46t3UkLKeqrKZ5cef1jq8qBZJOdOZfHxbqofunLnoxNes0saJ6Xjky4x63baDi7JZAL8k/l0DhHHy/L2vQz30BQv2JKPJrfHLseuRN4up4R3ngc0Mbklw3czmxxkqOJSeJ944xzPNzBZTX8YvNBbLsoKOisGeomez1Q/3h3EQhIYldvp1H1KIxLmoBcObyXztDDHVnOApRdGh4E+de5gIm9RYcSxU0czxyr71KA3Nimbya+pBPGF/T4NYJGJp801miqpF+s1j44BPpxOPpreyJdANiThUMQrUVyr6AxAzReX2P6j++M2j58IA1uUldjVABUoGstD5u8mBr8Q9YP9ZmjQpVfhKORcmRs70lWcfrAi75FOXIB295AFb9TOh783o6yuKZ+me4By+qrL9rYiB7/QKYek8hKnx6sr/knzfX1VkkB59Ilt+QVkhX9/dTl54J8zVBSUmOO+4Ac9CwdXVZTBYXsVa6v94JFnekDZ5fI2orlsCOUFtRRti/X1fQYM3U/r+WCK6s7jh4+2nYuwPuxx85pS1hiWLu7a1yhNMXx8NfZ4jHdY76JjEBr9cBDGyVlIoJTEzMbbb7XugBpq/hu7KnQFFi7TqucYRB9Ar16qMNhB61yqcFeVfF61OjPhqIo0v5jzQmpe18FNYA= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: > On Dec 9, 2022, at 06:39, John Hubbard wrote: >=20 > On 12/8/22 14:33, Sidhartha Kumar wrote: >> On 12/8/22 2:14 PM, John Hubbard wrote: >>> On 12/8/22 14:12, Sidhartha Kumar wrote: >>>> On 12/8/22 2:01 PM, John Hubbard wrote: >>>>> On 12/8/22 13:58, Sidhartha Kumar wrote: >>>>>> Thanks John, Mike, Matthew, and Muchun for the feedback. >>>>>>=20 >>>>>> To summarize this discussion and outline the next version of this = patch, the changes I'll make include: >>>>>>=20 >>>>>> 1) change the name of folio_set_compound_order() to = folio_set_order() >>>>>> 2) change the placement of this function from mm.h to = mm/internal.h >>>>>> 3) folio_set_order() will set both _folio_order and = _folio_nr_pages and handle the zero order case correctly. >>>>>> 4) remove the comment about hugetlb's specific use for zero = orders >>>>>> 5) improve the style of folio_set_order() by removing ifdefs from = inside the function to doing >>>>>>=20 >>>>>> #ifdef CONFIG_64BIT >>>>>> static inline void folio_set_order(struct folio *folio, >>>>>> unsigned int order) >>>>>> { >>>>>> VM_BUG_ON_FOLIO(!folio_test_large(folio), folio); >>>>>=20 >>>>> Sounds good, except for this part: why is a function named >>>>> folio_set_order() BUG-ing on a non-large folio? The naming >>>>> is still wrong, perhaps? >>>>>=20 >>>>=20 >>>> This is because the _folio_nr_pages and _folio_order fields are = part of the first tail page in the folio. folio_test_large returns if = the folio is larger than one page which would be required for setting = the fields. >>>=20 >>> OK, but then as I said, the name is wrong. One can either: >>>=20 >>> a) handle the non-large case, or >>>=20 >>> b) rename the function to indicate that it only works on large = folios. >>>=20 >> Discussed here[1], the BUG_ON line seemed more appropriate over >> if (!folio_test_large(folio)) >> return; >> as the misuse would not be silent. I think I would be against = renaming the function as I don't see any large folio specific function = names for other accessors of tail page fields. Would both the BUG_ON and = return on non-large folio be included then? >=20 > Actually, if you want the "misuse to not be silent", today's = guidelines > would point more toward WARN and return, instead of BUG, btw. =46rom you advise, I think we can remove VM_BUG_ON and handle non-zero order page, something like: static inline void folio_set_order(struct folio *folio, unsigned int order) { if (!folio_test_large(folio)) { WARN_ON(order); return; } folio->_folio_order =3D order; #ifdef CONFIG_64BIT folio->_folio_nr_pages =3D order ? 1U << order : 0; #endif } In this case, 1) we can handle both non-zero and zero (folio_order() works as well for this case) order page. 2) it can prevent OOB for non-large folio and warn unexpected users. 3) Do not BUG. 4) No need to rename folio_set_order. What do you think? Thanks. >=20 > I don't think that a survey of existing names is really the final word = on what > to name this. Names should be accurate, even if other names are less = so. How > about something like: >=20 > large_folio_set_order() >=20 > ? >=20 >> [1]: = https://lore.kernel.org/linux-mm/20221129225039.82257-1-sidhartha.kumar@or= acle.com/T/#m98cf80bb21ae533b7385f2e363c602e2c9e2802d >>>=20 >>> thanks, >=20 > thanks, > --=20 > John Hubbard > NVIDIA