Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 9 Jan 2023 10:23:46 -0800
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Sidhartha Kumar <sidhartha.kumar@oracle.com>
Cc:     Matthew Wilcox <willy@infradead.org>, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, akpm@linux-foundation.org,
        songmuchun@bytedance.com, tsahu@linux.ibm.com, jhubbard@nvidia.com
Subject: Re: [PATCH mm-unstable 8/8] mm/hugetlb: convert
 demote_free_huge_page to folios
Message-ID: <Y7xbsnYSM687X8pF@monkey>
References: <20230103191340.116536-1-sidhartha.kumar@oracle.com>
 <20230103191340.116536-9-sidhartha.kumar@oracle.com>
 <Y7jGyH9Atv3sPjcZ@monkey>
 <Y7jLVJ9kvBFCUhNd@casper.infradead.org>
 <Y7ncSXYzCmjzmS87@monkey>
 <0b711878-71b0-4a4b-4f0b-af157d6cac42@oracle.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0b711878-71b0-4a4b-4f0b-af157d6cac42@oracle.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?7r9D7q3wVKkO4retObP5iMXb6Pj7qHpX7jb/ptUJqTEl9tD4NWmT4ZAF9Mh/?=
 =?us-ascii?Q?4DOABnAal5vhcM1NmL+yXrLA6YIW6+ZOzZC4jGxmlisXhW0Yuu+yuIcN7iVd?=
 =?us-ascii?Q?Xn3odKodzLiU1PnpBKgUW9UdY5uICB921DHFZ4owSVyYmUW5X5xNX7wTU8y8?=
 =?us-ascii?Q?u9LF1bg1v6zCi7RI4WYyj9OT1Asv5oVJ6MdnRz8NiF6VV9niZQAdrxln/0n+?=
 =?us-ascii?Q?Iad6blIPF3oAg0KY5xfHKJwDnNCfkGhAaKdNBWhZ4ud2WXRr6/ng3XqqaIM7?=
 =?us-ascii?Q?zNT3a6ctyTC9zIh1dCvrz5tqeWAKnmcZT2SoX15wcE3bktOvmVletLfAd0hN?=
 =?us-ascii?Q?rSgYmzK7GqmaEBWPvLm+diMz45t+IknmqZ4cTO/Hal42vD57dKmzuHv9WtBK?=
 =?us-ascii?Q?8ZhbDAovaEImvJNWT4KGubCuJLTlEzvnVsPL4cB+Of+WV3iEmms/2aQ+ssHl?=
 =?us-ascii?Q?s9igugBhqeKgVGkS9E2oTHwtgtwTxb37OFJz2KFBvKLzHBpycmd3Uk5J4KoY?=
 =?us-ascii?Q?f05UxIQZB2yyE9WrtGRq952+hxnlAp4q6HuC07vYbgRQQP+o1UezNzjVY7/O?=
 =?us-ascii?Q?2Y/qUo9ejKvjXNLFaECdNuwk5JWrU97eIdfhiubMKiGUgmKtvw8NHhyS/Q3s?=
 =?us-ascii?Q?xMdjIE0A04Zl43Edb09UUSznKLoueWJHIIU9l65gVOtzJ6RwHwj6FBGhSopz?=
 =?us-ascii?Q?wHCQ7hXZJ5azBZ9t9CN2iGhjDPBBtyRJYnrRFZ5jV2dIPHIVLXaUxcC9zM+v?=
 =?us-ascii?Q?T0hAPHjz3WjZCTg4xG/HQb9urqTmE8E+ELXVaM/aUTA79zIBh/Yx9DWhLWCB?=
 =?us-ascii?Q?Yaon6LBwzK4GcWnSR+nToUl/Dv2JusGau3BOExh57WOTXc7KIqSk2rCwSNhC?=
 =?us-ascii?Q?WxtMquXIfBNZzbunWwsTZg1XCvmQYMZO03QqfqOWUNLoSJJslLFW4kGSRcvs?=
 =?us-ascii?Q?8zWNIzJwqgfYugqW63oHpTwF/SdNEZmoeXpDmf9XHUEyjYUvNoi6Z/rnvJnM?=
 =?us-ascii?Q?aRcZ0efT9UViKcbywSQlQRq0tK0GNcXzhwjAFpMi9fkGtzkWyk5TFFLHue+z?=
 =?us-ascii?Q?/MWps98ulCk+gQX/bD+AB6kn2QjgVocjayDE3y9WVAi7ifjXjM3QV20d5IO8?=
 =?us-ascii?Q?P0GYeAwMHo65TpilZjyjNrevi0IA5tWzEeM12mCx/oW+OZSI9mxG+nZ9tJ83?=
 =?us-ascii?Q?J1hPKXu1JlHbfQZaccZjAuhrWYwtuGIwQTBkqCook4/pTG+Jd9LTjtQH6tSR?=
 =?us-ascii?Q?whcmJ1f1ToKU1E5Z2npbltfFdvBv3fAYBxEKazP7MKeEn9wncV/h8yfVpqIC?=
 =?us-ascii?Q?I8Rfx5lF9pPqEPzbkv5gDOkB5ANivJqxn3nZVA0JAT5BhLwAIx8krEjn/xJf?=
 =?us-ascii?Q?uIsIEyBO3hbYeBxVzteWTDGERbvoS9Ij1rdGLwwfD9aAUVOrqnCpKWBMQeqY?=
 =?us-ascii?Q?0tY+ovYeQvjG8Iy5497LzrR4xseU/8PlWCUlGyaJr0UkSdQq0uOHQI6WfNV3?=
 =?us-ascii?Q?DT5PuKhozXKe3S9xQeeBUjM5Bgyq6YC0iwGTVMquUKxTpnaM/TgGyYxT+JhH?=
 =?us-ascii?Q?EsGJ7PMrAn3yOEPxk74i1N/7oVrig0idAWifyRZF+HWxTbhPYOcGcUFaGeb1?=
 =?us-ascii?Q?Vw=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?OOH02rLFuWYcirgyVLKTMzD5HM2wf9P431Hi7bwsmneo4GOJhHNNExRA6sCb?=
 =?us-ascii?Q?PgixSr6dcg0sfRqNr0U951SbSfOQSBFVyMEBMh3/n4xMdGb05lRwOLXxQy6/?=
 =?us-ascii?Q?IWvkRxVp1jc0pZ/UG9NWPOHcrNeuBEWqKGgydNj7UXUWeM80JXw5pNZ2Np5C?=
 =?us-ascii?Q?zKk9pdPw8zwQWQTiepgv/8yoH9GtvLgT5K7VQVzBjyYeant34V+n7I4RZa7x?=
 =?us-ascii?Q?6xomc8asMnXB0815DsTUWJnJU/YfP6g2odVmC3G9+7fvhkcSTyrYYz6+3xss?=
 =?us-ascii?Q?SLRdGdg1xyyEtUJT2hAUIa8cJ/45CWE3wRqgeeJmPulMaj2tOOFME9rI7Fuu?=
 =?us-ascii?Q?MIeNUcRypshDMG+1ivxNI9vjLAXv3a5HBsyxISzZ+zhqF8iR5T90NEY7jZJR?=
 =?us-ascii?Q?2i92d3ZVgBFlw5AwdxgNr+cdp6M3hjGUCIoga2EVjWv2p6wfoboABPr6jsCK?=
 =?us-ascii?Q?LkpW2aRlqkNyJLm8p3vY0+N1dZwMXuXep1efW2SnGt/8sM09y88ZY8Ghgoyl?=
 =?us-ascii?Q?cKylXCsMSmpRU4EIooDJs1sWge7Hffff7NgkX449XNtH+/C7Kl18CiuAs73L?=
 =?us-ascii?Q?UHDeSRHz9DzSb/H3WhMUXbxRmv7hWZ7ZY2p3dRsWqhsleVQkgdvfBEVGehWd?=
 =?us-ascii?Q?lu/EY4bDc6tZUEku6tmb/K/bI0j5K0cm59QLxu9JW9z7P/Rtrh3FwmWmAwVk?=
 =?us-ascii?Q?kUJPTiFWe9o90oyjYmbDr1d53CqkVcpCV6O/XAxj3Xn7scLk1XQIdF42uyXt?=
 =?us-ascii?Q?1YYFjPI3PZmz0xnwLor9xJ4qXfNQdaNBE50em/W47cmEe6DqxN5PKLViViyj?=
 =?us-ascii?Q?zTUIa8CrxVi5ogdnzZOveUDrE4p86GjnOoFD+IcxZhWg0LGqmcLSRolMRht6?=
 =?us-ascii?Q?lKx48QHQ5O3sEjEZcyXZZ7I5L16a2r06Uz1HlVxVWp7Si2KvQdG7igM5wpAh?=
 =?us-ascii?Q?gIpYbubd2tocUjUC6hi0reVJIf5bXbp4Xr311BK6zpc=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 009c141b-e17b-4068-79f1-08daf26ea73b
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jan 2023 18:23:49.3301
 (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: xl4oOhICLmsM2HoTT7X4nwBsp1DLUx6FQb8HkbpIJWu5xBI56cKpVuFCdxMXXnXVgTPNUpbbvcmK+S4bphS/4w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB4182
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2023-01-09_12,2023-01-09_02,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=999
 suspectscore=0 adultscore=0 phishscore=0 mlxscore=0 malwarescore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2212070000 definitions=main-2301090131
X-Proofpoint-GUID: khyt0lhvYMB5tT2F_9JzGG0YTyTOI-U0
X-Proofpoint-ORIG-GUID: khyt0lhvYMB5tT2F_9JzGG0YTyTOI-U0
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 01/09/23 10:36, Sidhartha Kumar wrote:
> On 1/7/23 2:55 PM, Mike Kravetz wrote:
> > On 01/07/23 01:31, Matthew Wilcox wrote:
> > > On Fri, Jan 06, 2023 at 05:11:36PM -0800, Mike Kravetz wrote:
> > > > On 01/03/23 13:13, Sidhartha Kumar wrote:
> > > > > @@ -3477,15 +3477,15 @@ static int demote_free_huge_page(struct hstate *h, struct page *page)
> > > > >   	mutex_lock(&target_hstate->resize_lock);
> > > > >   	for (i = 0; i < pages_per_huge_page(h);
> > > > >   				i += pages_per_huge_page(target_hstate)) {
> > > > > -		subpage = nth_page(page, i);
> > > > > -		folio = page_folio(subpage);
> > > > > +		subpage = folio_page(folio, i);
> > > > > +		subfolio = page_folio(subpage);
> > > > 
> > > > No problems with the code, but I am not in love with the name subfolio.
> > > > I know it is patterned after 'subpage'.  For better or worse, the term
> > > > subpage is used throughout the kernel.  This would be the first usage of
> > > > the term 'subfolio'.
> > > > 
> > > > Matthew do you have any comments on the naming?  It is local to hugetlb,
> > > > but I would hate to see use of the term subfolio based on its introduction
> > > > here.
> > > 
> > > I'm really not a fan of it either.  I intended to dive into this patch
> > > and understand the function it's modifying, in the hopes of suggesting
> > > a better name and/or method.
> > 
> > At a high level, this routine is splitting a very large folio (1G for
> > example) into multiple large folios of a smaller size (512 2M folios for
> > example).  The loop is iterating through the very large folio at
> > increments of the smaller large folio.  subfolio (previously subpage) is
> > used to point to the smaller large folio within the loop.
> > 
> If folio does not need to be part of the variable name, how about something
> like 'demote_target'? The prep call inside the loop would then look like:
> 
> prep_new_hugetlb_folio(target_hstate, demote_target, nid);
> 
> so it is still clear that demote_target is a folio. A more concise version
> could also be 'demote_dst' but that seems more ambiguous than target.

I am OK with that naming.  Primary concern was the introduction of the
term subfolio.
-- 
Mike Kravetz