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 X-Spam-Level: X-Spam-Status: No, score=-7.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B9C65C433DB for ; Wed, 13 Jan 2021 17:50:13 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3A1E1230F9 for ; Wed, 13 Jan 2021 17:50:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3A1E1230F9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=oracle.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 77F4A8D007E; Wed, 13 Jan 2021 12:50:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 72F3B8D006A; Wed, 13 Jan 2021 12:50:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 61E1C8D007E; Wed, 13 Jan 2021 12:50:12 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0175.hostedemail.com [216.40.44.175]) by kanga.kvack.org (Postfix) with ESMTP id 4BE538D006A for ; Wed, 13 Jan 2021 12:50:12 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id D9AD93628 for ; Wed, 13 Jan 2021 17:50:11 +0000 (UTC) X-FDA: 77701490622.08.road63_3604ecc27520 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin08.hostedemail.com (Postfix) with ESMTP id BA7101819E764 for ; Wed, 13 Jan 2021 17:50:11 +0000 (UTC) X-HE-Tag: road63_3604ecc27520 X-Filterd-Recvd-Size: 7194 Received: from aserp2120.oracle.com (aserp2120.oracle.com [141.146.126.78]) by imf41.hostedemail.com (Postfix) with ESMTP for ; Wed, 13 Jan 2021 17:50:10 +0000 (UTC) Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 10DHYPmG054016; Wed, 13 Jan 2021 17:50:04 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=mime-version : message-id : date : from : to : cc : subject : references : in-reply-to : content-type : content-transfer-encoding; s=corp-2020-01-29; bh=wo8LZ7eA6a5WktVrAu4Z2fQye1mJht9qUOWl/ksXaI0=; b=lPm0AyYKPvYM1GVU6r3aTkCvNHisCuFiktsGUswyw8YdLRkKtGMDJzYN5TcY/aI5IWvS bPJnAY2MeWTNtNGtiXPUf6yqrAW9V4T94xRoQzcJm1NuQir9s46Az113b4SN3Quiqtdd JlrkDVCV47ouiF0D+BZDlteiANxYZqnRponNB4jpymsb4Dm2f23PEpXyUWhpAuk7Q0BY NwJYKmQbxAIGaVpVQp3l5ESTCUG4dhI+EPhkPQkce52Ds27hHCdQsQzUc4BRONpPJwTu PUfGHdhTqd/S16nmf5k6dqOMD9ft6lccJWyxqG8hLh8xMufPFrzUbrLeWRzxwpwdmmUu ew== Received: from aserp3020.oracle.com (aserp3020.oracle.com [141.146.126.70]) by aserp2120.oracle.com with ESMTP id 360kcyvqwd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 13 Jan 2021 17:50:04 +0000 Received: from pps.filterd (aserp3020.oracle.com [127.0.0.1]) by aserp3020.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 10DHZfom031149; Wed, 13 Jan 2021 17:50:04 GMT Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by aserp3020.oracle.com with ESMTP id 360ke8rwnx-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 13 Jan 2021 17:50:04 +0000 Received: from abhmp0006.oracle.com (abhmp0006.oracle.com [141.146.116.12]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 10DHnxs6002288; Wed, 13 Jan 2021 17:49:59 GMT Received: from [192.168.2.112] (/50.38.35.18) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 13 Jan 2021 09:49:47 -0800 USER-AGENT: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 Content-Language: en-US MIME-Version: 1.0 Message-ID: <14c4f841-efd2-e5ac-ba6e-567dc2a3461c@oracle.com> Date: Wed, 13 Jan 2021 09:49:46 -0800 (PST) From: Mike Kravetz To: Oscar Salvador Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Michal Hocko , Naoya Horiguchi , Muchun Song , David Hildenbrand , Andrew Morton Subject: Re: [RFC PATCH 1/3] hugetlb: use page.private for hugetlb specific page flags References: <20210111210152.118394-1-mike.kravetz@oracle.com> <20210111210152.118394-2-mike.kravetz@oracle.com> <20210113135439.GA29271@linux> In-Reply-To: <20210113135439.GA29271@linux> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9863 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 suspectscore=0 spamscore=0 mlxlogscore=999 malwarescore=0 bulkscore=0 mlxscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101130106 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9863 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 impostorscore=0 bulkscore=0 adultscore=0 suspectscore=0 malwarescore=0 lowpriorityscore=0 clxscore=1015 mlxlogscore=999 mlxscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101130106 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 1/13/21 5:54 AM, Oscar Salvador wrote: > On Mon, Jan 11, 2021 at 01:01:50PM -0800, Mike Kravetz wrote: >> As hugetlbfs evolved, state information about hugetlb pages was added. >> One 'convenient' way of doing this was to use available fields in tail >> pages. Over time, it has become difficult to know the meaning or contents >> of fields simply be looking at a small bit of code. Sometimes, the >> naming is just confusing. For example: The PagePrivate flag indicates >> a huge page reservation was consumed and needs to be restored if an error >> is encountered and the page is freed before it is instantiated. The >> page.private field contains the pointer to a subpool if the page is >> associated with one. >> >> In an effort to make the code more readable, use page.private to contain >> hugetlb specific flags. These flags will have test, set and clear functions >> similar to those used for 'normal' page flags. More importantly, the >> flags will have names which actually reflect their purpose. >> >> In this patch, >> - Create infrastructure for huge page flag functions >> - Move subpool pointer to page[1].private to make way for flags >> Create routines with meaningful names to modify subpool field >> - Use new HPageRestoreReserve reserve flag instead of PagePrivate >> >> Conversion of other state information will happen in subsequent patches. > > I like this idea, it would make the code much easier to follow, and together > with Muchun's gathering indiscrete index hugetlb code will start looking less > scarier. Thanks for taking a look. > > I do have a question below: > >> +enum htlb_page_flags { >> + HPAGE_RestoreReserve = 0, >> +}; >> + >> +/* >> + * Macros to create function definitions for hpage flags >> + */ >> +#define TESTHPAGEFLAG(flname) \ >> +static inline int HPage##flname(struct page *page) \ >> + { return test_bit(HPAGE_##flname, &(page->private)); } >> + >> +#define SETHPAGEFLAG(flname) \ >> +static inline void SetHPage##flname(struct page *page) \ >> + { set_bit(HPAGE_##flname, &(page->private)); } >> + >> +#define CLEARHPAGEFLAG(flname) \ >> +static inline void ClearHPage##flname(struct page *page) \ >> + { clear_bit(HPAGE_##flname, &(page->private)); } >> + >> +#define HPAGEFLAG(flname) \ >> + TESTHPAGEFLAG(flname) \ >> + SETHPAGEFLAG(flname) \ >> + CLEARHPAGEFLAG(flname) >> + >> +HPAGEFLAG(RestoreReserve) > > I have mixed feelings about this. > Could we have a single function that sets/clears the bit/flag? > e.g: > > static inline void hugetlb_set_flag(struct page *p, page_flag) > { > set_bit(flag, &(page->private)); > } > > etc. > It would look less of an overkill? Sure, we could do this. As noted, I simply patterned this after the page flag routines in page-flags.h. If we go to single functions as you suggest, I would perhaps change the name a bit to indicate the flags were associated with the page. Invoking code comparison would be: SetHPageRestoreReserve(page) -vs- hugetlb_set_pageflag(page, HP_Restore_Reserve) In either case, code would be more readable and you can easily grep for a specific flag. If we do go with single functions as above, then they would certainly be moved to hugetlb.h as some flags need to be accessed outside hugetlb.c. Muchun has already suggested this movement. -- Mike Kravetz