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 EC8A1C433FE for ; Thu, 17 Nov 2022 16:27:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 27F7A6B0071; Thu, 17 Nov 2022 11:27:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 22F966B0072; Thu, 17 Nov 2022 11:27:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0D02E8E0001; Thu, 17 Nov 2022 11:27:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id F103B6B0071 for ; Thu, 17 Nov 2022 11:27:14 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id BAE18C09DC for ; Thu, 17 Nov 2022 16:27:14 +0000 (UTC) X-FDA: 80143463988.06.DBD5518 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf16.hostedemail.com (Postfix) with ESMTP id 2C8C9180009 for ; Thu, 17 Nov 2022 16:27:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668702432; 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: in-reply-to:in-reply-to:references:references; bh=ECSnbQYfyTzj4hb5xC2FkZlFvx+IZRrl+au2Rw9T6WM=; b=b1W7SD++7/eHRs04MbtjXhhqyv9dQ9q12TjfYq+lr7e9l7Q7HcbgSiiWHgNLw+nJJ2VM2l XtH6GMu2lGv1byw7XpLMGSNQItP30eOVHLqUr1pu7NDYH77m5Bcnrtb5YVfX8G4vcNDWZ2 RTzfo146vkaDMoMzqAyRAfem5rwiO68= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-120-ttXUkxqJOtSzBO_wkH6ZmQ-1; Thu, 17 Nov 2022 11:27:11 -0500 X-MC-Unique: ttXUkxqJOtSzBO_wkH6ZmQ-1 Received: by mail-qt1-f199.google.com with SMTP id i4-20020ac813c4000000b003a5044a818cso2115432qtj.11 for ; Thu, 17 Nov 2022 08:27:10 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=ECSnbQYfyTzj4hb5xC2FkZlFvx+IZRrl+au2Rw9T6WM=; b=GeN00oW/3kForrHZb2ekr78cKAZi3+x3dB1UVutSbYOnL8MWcSw+GSu7e6D1x3qHQt P8FWGsnFDXn/cK7ujFbcl5YQ8Zut9Y5yK1ONC2UzURekmIV1T2+B9WY13TG6UXC/k5VT 20scmjghoob23mzTy24JRW0yae9MyqtYe8o6z2a+0uZ7qupE7e/SAVQ+2b34r957IRVc N7UpQ5ioPEKSdWC9XecDhMqehCHzDBD7E/rI/W6ajydlrJ0J8JrnsH7nUSfDr33J5cDk 9SP1u/xWNDtlGRAzhKjVdJ5B73+RkSH7py7T7rABwC6OsWjFVnHhV+Ury682ozf3klAJ dkoA== X-Gm-Message-State: ANoB5pnC1E3mSjFCwIU3N4j/iLY3aXG8wm7WLBafi1SL6hWQ/Z0/4kwY sUCEixXzVYLPD2y6OR7FTCtChRXSMb+123BqJks2hzM+5Qrh8UoklHoCvhIXeHkTjbgIQ4NcUQe ynmnbQGJArCc= X-Received: by 2002:ac8:704:0:b0:39c:d509:76a5 with SMTP id g4-20020ac80704000000b0039cd50976a5mr2899881qth.382.1668702430475; Thu, 17 Nov 2022 08:27:10 -0800 (PST) X-Google-Smtp-Source: AA0mqf7EJS/miV6XQ6ptWdO+LFShsaM6eU5na38uW33M+lBwqqItY3BuFgTw/XjdD19YLVqgAVclZg== X-Received: by 2002:ac8:704:0:b0:39c:d509:76a5 with SMTP id g4-20020ac80704000000b0039cd50976a5mr2899857qth.382.1668702430219; Thu, 17 Nov 2022 08:27:10 -0800 (PST) Received: from x1n (bras-base-aurron9127w-grc-46-70-31-27-79.dsl.bell.ca. [70.31.27.79]) by smtp.gmail.com with ESMTPSA id u12-20020a37ab0c000000b006bb29d932e1sm681515qke.105.2022.11.17.08.27.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Nov 2022 08:27:09 -0800 (PST) Date: Thu, 17 Nov 2022 11:27:08 -0500 From: Peter Xu To: James Houghton Cc: Mike Kravetz , Muchun Song , David Hildenbrand , David Rientjes , Axel Rasmussen , Mina Almasry , Zach O'Keefe , Manish Mishra , Naoya Horiguchi , "Dr . David Alan Gilbert" , "Matthew Wilcox (Oracle)" , Vlastimil Babka , Baolin Wang , Miaohe Lin , Yang Shi , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v2 10/47] hugetlb: add hugetlb_pte to track HugeTLB page table entries Message-ID: References: <20221021163703.3218176-1-jthoughton@google.com> <20221021163703.3218176-11-jthoughton@google.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1668702433; a=rsa-sha256; cv=none; b=syBK7o+qaXdypOJk4Q1DWFcvnuEZ8zxVTtqUiEdpKPp+pVFk8x/HsQm2ZBXH5ca+P4uckl 1u5shbTVLMGFkUiHeuXxJl8xxRuJ9v4WSESiCqA4JBIkrfMAARWbJctvpxZ/OM4bnJcVBT ITdKJawK3mlKzwOmL6GB3txbIPJ4ggg= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=b1W7SD++; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf16.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1668702433; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ECSnbQYfyTzj4hb5xC2FkZlFvx+IZRrl+au2Rw9T6WM=; b=o/2c2srYQ8AKunK4xEjAlk92vaMyL12iR39g7T5j8N23syc6IjlO06EM61cjp6eAniZlXx lYuu0cc5ObpFMTHvknakxMPtaklzbVBdMpOgIScaHP8bxi0PCIwfvmV8LbTz82/p5cm3gi 1aRQQuz8VhCoJvl36qGj8rEtkyaeuzQ= X-Rspam-User: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 2C8C9180009 Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=b1W7SD++; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf16.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com X-Stat-Signature: edq1iegrmpnxo3fgnooazqbmzotrndei X-HE-Tag: 1668702433-796378 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 Wed, Nov 16, 2022 at 05:00:08PM -0800, James Houghton wrote: > On Wed, Nov 16, 2022 at 2:18 PM Peter Xu wrote: > > > > On Fri, Oct 21, 2022 at 04:36:26PM +0000, James Houghton wrote: > > > +struct hugetlb_pte { > > > + pte_t *ptep; > > > + unsigned int shift; > > > + enum hugetlb_level level; > > > + spinlock_t *ptl; > > > +}; > > > > Do we need both shift + level? Maybe it's only meaningful for ARM where > > the shift may not be directly calculcated from level? > > > > I'm wondering whether we can just maintain "shift" then we calculate > > "level" realtime. It just reads a bit weird to have these two fields, also > > a burden to most of the call sites where shift and level exactly match.. > > My main concern is interaction with folded levels. For example, if > PUD_SIZE and PMD_SIZE are the same, we want to do something like this: > > pud = pud_offset(p4d, addr) > pmd = pmd_offset(pud, addr) /* this is just pmd = (pmd_t *) pud */ > pte = pte_offset(pmd, addr) > > and I think we should avoid quietly skipping the folded level, which > could happen: > > pud = pud_offset(p4d, addr) > /* Each time, we go back to pte_t *, so if we stored PUD_SHIFT here, > it is impossible to know that `pud` came from `pud_offset` and not > `pmd_offset`. We must assume the deeper level so that we don't get > stuck in a loop. */ > pte = pte_offset(pud, addr) /* pud is cast from (pud_t * -> pte_t * -> > pmd_t *) */ > > Quietly dropping p*d_offset for folded levels is safe; it's just a > cast that we're doing anyway. If you think this is fine, then I can > remove `level`. It might also be that this is a non-issue and that > there will never be a folded level underneath a hugepage level. > > We could also change `ptep` to a union eventually (to clean up > "hugetlb casts everything to pte_t *" messiness), and having an > explicit `level` as a tag for the union would be nice help. In the > same way: I like having `level` explicitly so that we know for sure > where `ptep` came from. Makes sense. > > I can try to reduce the burden at the callsite while keeping `level`: > hpage_size_to_level() is really annoying to have everywhere. Yeah this would be nice. Per what I see most callers are having paired level/shift, so maybe we can make hugetlb_hgm_iter_init() to only take one of them and calculate the other. Then there can also be the __hugetlb_hgm_iter_init() which takes both, only used in the few places where necessary to have explicit level/shift. hugetlb_hgm_iter_init() calls __hugetlb_hgm_iter_init(). Thanks, -- Peter Xu