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:   Thu, 7 Jul 2022 14:39:08 -0700
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Mina Almasry <almasrymina@google.com>
Cc:     James Houghton <jthoughton@google.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Peter Xu <peterx@redhat.com>,
        David Hildenbrand <david@redhat.com>,
        David Rientjes <rientjes@google.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Jue Wang <juew@google.com>,
        Manish Mishra <manish.mishra@nutanix.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 10/26] hugetlb: add for_each_hgm_shift
Message-ID: <YsdSfBUAINPlsR9s@monkey>
References: <20220624173656.2033256-1-jthoughton@google.com>
 <20220624173656.2033256-11-jthoughton@google.com>
 <CAHS8izNO+4Nzg=rgr1R027i2Evo-UKiFEK1gQcMHtF8uxcHzeQ@mail.gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHS8izNO+4Nzg=rgr1R027i2Evo-UKiFEK1gQcMHtF8uxcHzeQ@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?RQghSm7PQWE0n2I7rdxjJmnL6FIhnx6bkr0Nye+7xjHCpkDRzHYcHGuVyzD9?=
 =?us-ascii?Q?k+ER+cxelAjHP4oEQpEQ+C34duBvVvv8x8d85MidGsxh9lDAljK2sz7vbTDD?=
 =?us-ascii?Q?msTkZmFgBITeh63RULA2IHRAZg/Cwn2Xv8eM/8stSgrLPDDHgC7sct4POfO2?=
 =?us-ascii?Q?mHFZVUetcGcW6lRWXUW2m/zV9WOIJRnzh0kVRTs6Ezj0LEiahEkKEL0Kz3n5?=
 =?us-ascii?Q?O0VipV9xC+snQaORajL0TitRs8BwpV4R9dS6FSR/wdtY40yj3iGpG+JLnjDk?=
 =?us-ascii?Q?BF6RV+FQbe2jmZIeDsJVgJjpnOfQDA2oFvonJEszEDXDxPYSFbWSxmBYo/W7?=
 =?us-ascii?Q?+vZt128ecGZ9repL4ZOS3OU8YU8ZXZjP/4gSmGFO2JV76LZXcuDpH1UMODbz?=
 =?us-ascii?Q?dK1+FY0zkRwMkatIE29jsESPfj58TQW9lan498OA6LW7LZ/nJvfe3uCGGee8?=
 =?us-ascii?Q?KtvkNmA2frlgPOeMzhV4ebPCK8luAdNHuLHa8PdvefuALk2JwOLwEn5Kn3FI?=
 =?us-ascii?Q?SoSbqWR3937GoKuJcJeofabESFqgaLakfTpuKug2vWc3N7wCYtTlOKYxi0fE?=
 =?us-ascii?Q?6JHmV9kUbcJXyQjJXtx6oYKaSfFFpeZfkryYHPoaDcf/ajLnoFRO9+ialCA8?=
 =?us-ascii?Q?bKBBYJ7ko5HAHvFPIuM+lnydDZu6IYh/jEOD5TwO8N2COv8nXxider+diRp0?=
 =?us-ascii?Q?LnlekXW6Ai7um/n1M3MAODyZoxZp/uucYUPOecaXXWiaCkJaE7eQZTg91Onc?=
 =?us-ascii?Q?qYFtD5bk1b0tbYpeNKrT/OLNTIdC1CeZz/TIuymB92z1kAtvRwfzeeLebuSW?=
 =?us-ascii?Q?H5sNpFzHHdIXzUxnSTXgmSRrHU3TWz1zXfuKpnG/RL9sMvEq+Duo6ZP0eiY1?=
 =?us-ascii?Q?UuatA45WzbBV611qhpkSDq5MnPJUH4raZgLQ50ZWmbJtmtD6pRAkJIBiQqO7?=
 =?us-ascii?Q?z3L4BVgVxsfeSTW7xtePvjXdm8w/loCGvIHrv3xeieeHvds5SsmMFYDUYV4E?=
 =?us-ascii?Q?azdTVxOeDJexVERbPzEZwIiQdMmkS9AIMEQF17ws73mvWcYHJq2kYFxNtGP9?=
 =?us-ascii?Q?Udkcpn1aAMkCx7GUK1zQb8RQQpVrXnWZCZ748lK9T6L4qAWj84g10Am5JeaD?=
 =?us-ascii?Q?Q6P67hYNTDcRsHl1R7lXXP//Dm0FYuhYjUlA20n0YVLNobkLldJ4c8FIAODs?=
 =?us-ascii?Q?tMp9mchufo/zBg9De3Tf+3Y9ZpWKnKZojiiJT35RKzs1/M7awHUGJ8ecA2d7?=
 =?us-ascii?Q?99GSqWv9GlehdxGvUlSdKdjcL9KuC3bdIi2Qq8Wv1R9knA1F3xZk20qXAI65?=
 =?us-ascii?Q?6NtZItRARepu53mbtppu18m5rEI4r6mOumo3bIqnRXBu3BJUnpV6i2DLgh1s?=
 =?us-ascii?Q?KyZisCBzCsyfkPrYe2jJ6ltGM8FhRopOTX+SsTRoxMvlV48SjSJ53XGqnb0a?=
 =?us-ascii?Q?xHHHTOEnlzGnQQCPbrIH+cTXmxvtzQUTLK67njllv5TQqOxs/uHm/47hHjdW?=
 =?us-ascii?Q?S9G2njB9j7L41CPvMhQ9PONKdkhuGm87fgO5w1D4JLkTz5Nx7NGwtozV2URV?=
 =?us-ascii?Q?/ft60DaBHEE62pkO/5adelJO+msH67degicSIjfao3jZMuEYsMohCO8TixWd?=
 =?us-ascii?Q?QQ=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: aac02376-3aff-4566-1236-08da60612168
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Jul 2022 21:39:14.2131
 (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: jOe5nMddaoA0Eiwvg/LcoIDxh34t9jLjDSbropsDnXvKzPG4IVcZMm8pxhJ8nVvPxKvdt2dSrymGfFfgfk8vhA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR10MB4175
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.517,18.0.883
 definitions=2022-07-07_17:2022-06-28,2022-07-07 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0 phishscore=0
 adultscore=0 malwarescore=0 mlxscore=0 spamscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000
 definitions=main-2207070085
X-Proofpoint-ORIG-GUID: 2TwUEiWO8E8fkgTJ-eV1KwmrtB6YzH9e
X-Proofpoint-GUID: 2TwUEiWO8E8fkgTJ-eV1KwmrtB6YzH9e
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,T_SCC_BODY_TEXT_LINE
        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 06/28/22 14:58, Mina Almasry wrote:
> On Fri, Jun 24, 2022 at 10:37 AM James Houghton <jthoughton@google.com> wrote:
> >
> > This is a helper macro to loop through all the usable page sizes for a
> > high-granularity-enabled HugeTLB VMA. Given the VMA's hstate, it will
> > loop, in descending order, through the page sizes that HugeTLB supports
> > for this architecture; it always includes PAGE_SIZE.
> >
> > Signed-off-by: James Houghton <jthoughton@google.com>
> > ---
> >  mm/hugetlb.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 8b10b941458d..557b0afdb503 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -6989,6 +6989,16 @@ bool hugetlb_hgm_enabled(struct vm_area_struct *vma)
> >         /* All shared VMAs have HGM enabled. */
> >         return vma->vm_flags & VM_SHARED;
> >  }
> > +static unsigned int __shift_for_hstate(struct hstate *h)
> > +{
> > +       if (h >= &hstates[hugetlb_max_hstate])
> > +               return PAGE_SHIFT;
> 
> h > &hstates[hugetlb_max_hstate] means that h is out of bounds, no? am
> I missing something here?
> 
> So is this intending to do:
> 
> if (h == hstates[hugetlb_max_hstate]
>     return PAGE_SHIFT;
> 
> ? If so, could we write it as so?
> 
> I'm also wondering why __shift_for_hstate(hstate[hugetlb_max_hstate])
> == PAGE_SHIFT? Isn't the last hstate the smallest hstate which should
> be 2MB on x86? Shouldn't this return PMD_SHIFT in that case?
> 

I too am missing how this is working for similar reasons.
-- 
Mike Kravetz

> > +       return huge_page_shift(h);
> > +}
> > +#define for_each_hgm_shift(hstate, tmp_h, shift) \
> > +       for ((tmp_h) = hstate; (shift) = __shift_for_hstate(tmp_h), \
> > +                              (tmp_h) <= &hstates[hugetlb_max_hstate]; \
> > +                              (tmp_h)++)
> >  #endif /* CONFIG_HUGETLB_HIGH_GRANULARITY_MAPPING */
> >
> >  /*
> > --
> > 2.37.0.rc0.161.g10f37bed90-goog
> >