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;
Message-ID: <54e46486-8886-f5d4-4900-8a250bb5e805@nutanix.com>
Date:   Wed, 29 Jun 2022 20:03:16 +0530
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.10.0
Subject: Re: [RFC PATCH 12/26] hugetlb: add HugeTLB splitting functionality
To:     James Houghton <jthoughton@google.com>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Peter Xu <peterx@redhat.com>
Cc:     David Hildenbrand <david@redhat.com>,
        David Rientjes <rientjes@google.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Mina Almasry <almasrymina@google.com>,
        Jue Wang <juew@google.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
References: <20220624173656.2033256-1-jthoughton@google.com>
 <20220624173656.2033256-13-jthoughton@google.com>
From:   "manish.mishra" <manish.mishra@nutanix.com>
In-Reply-To: <20220624173656.2033256-13-jthoughton@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?U004TEdoVHJNdzBrMlVsTlUwNzJscG5PNnhEbkVoellzN0lHQ0JMWTYxQ1hz?=
 =?utf-8?B?eVBTcFNlcHFPSmtNSU41RGpQUis1K0JNNDF5Z2lpNU9EdlJyUTIyU24ySUxS?=
 =?utf-8?B?QzdIY1l4bUVpVHcrMjJYdXN3anc1U2RQaXozNnU5TDc1RTJxc2NudmwxckhW?=
 =?utf-8?B?V2ZzZW5DUkVwZjQ1TDg5RlBNNkZzRS90cEZ6Y0xNSHdHNVFLU2R5Skc4akt1?=
 =?utf-8?B?VWZZUDE2TEhWZ3pVb29wbzh0TnArbXJqbDR5bzR5WEJla0ZqYjV1TG5lNThq?=
 =?utf-8?B?UEwraXU3Z3had3YxbmVQWG5tRXljNGxBWXJUUEhCaXVjSlBsSUVMcWxnSFB0?=
 =?utf-8?B?RG9MRENzZWh4STUwM1BDSDZJbkVoeTYrRG5Mb1hTRlhYaW83ZkZjSC9Nd3Js?=
 =?utf-8?B?aE8rbGVESVRIWUJ1WWdDY0tOaWRuamNGYXpFbkxoK3ZNVkRyc2ZIL21yQnVI?=
 =?utf-8?B?WXZoRzJpSVNXZjBlVjJQS090VlFCdmRobUZLM01IYU9JMVJadGhZdTRuUXdP?=
 =?utf-8?B?UE4yMC9pL2FJaUFoVVpDcjB5RklqdkVSK3JnZmJJayt3RFpVcWpsUXpzeXNY?=
 =?utf-8?B?NkhmSGxvcFNJTmNUczVzc1pzMEpZR1libXJNWXFTc0ozVHJVcXB3dlFINUNn?=
 =?utf-8?B?c0lkWlF2TTNYRXJld0RjWk5BQlJwT3dvZXVhQWlyUmR3UGl0c1BuNVhjU1Yv?=
 =?utf-8?B?UER6Y0xXU3NBdVRpbHJuR0dUczNCMWJvZktsRHI2Q24vZ3lhNGNTNDFpQUNz?=
 =?utf-8?B?NHhZUkpqZlYxTUt6elVTdXNWc0ZMamFRZ0U1WGY0cGJTbHpmdXBKWkd4aGpV?=
 =?utf-8?B?enMzamovVUdZSUFhUnJ5Q1B4ankxTHJBNnRnT2QrNk1oSlBTN0tRcS9Tcm9L?=
 =?utf-8?B?UEx4RjA3VzJMSHlsbEg5b3gydGtDdlhlTE02Sm9hM1d4N21XeHd5R0xLdUNJ?=
 =?utf-8?B?bGp1bytmY29EeUpxVzVQbGtPYjJhUHg4Y0RKdFdnL0loSDJLL2l6TXF6aSt5?=
 =?utf-8?B?VU5pVG52dzlnbDkrVm5qTy8ycUtHN3BxeVBNWFh4d1dFVndDb2lEaElYVmNp?=
 =?utf-8?B?dmNQbzdjYzV2cEN2V1dqZ0dzb1lsZ2FiK2dWaVpieU9BTTFIWGx2eWRlM3dm?=
 =?utf-8?B?djhQQ0ZMMEFMQW5zRUxSdEtMOWJPRGYzckxXV21MMDFOSGU2cXUvbXY2OUVS?=
 =?utf-8?B?a1J1MjErNk1namhvMlNTVjY1eTh6S2lKVEhyWmM1T2tTMER1eWo2V0t6dFpQ?=
 =?utf-8?B?bi9Bd0NUbGpkY3hhUlNrR283VC9JYUlNZWlyRDA4Y2xQN3htdlZ1cVltbGpJ?=
 =?utf-8?B?eThYYkJxb3cyTG04SXhScmZvbEhZNFA5T09aTnhvZ3EyMzVFMVllM1AvUkto?=
 =?utf-8?B?MGkzSlg5Z3Q0MGVEU0JwYm04TUZEaUMyWFNkWlRtL2p0aEdqTysxdjdiaVRs?=
 =?utf-8?B?T2NZOFJBdzRadlQ0ZlpJVStWUHMvT0V5Z2Z4L0dMdTlpK0JMdE4xRldOYzZ2?=
 =?utf-8?B?bkJ5NExFYXVaWFZhdGVGcHRTL2NrSlpaNFUxUFJSR0Z0YUJjUk9vVi9Ha28v?=
 =?utf-8?B?YnhrM2FUaXovM2NlU2JqMGFqK0dydy9sZXZFQUV6UUppSnBmOGhvTXFuMk5R?=
 =?utf-8?B?N0xyNFNNQzBtT3ZDTWtLUU14dVpjZW5MUUg0Ti9CVStiSXBLekxUekNQR0pv?=
 =?utf-8?B?cFQwdFNhV1h0VGx0aElpa21HYjBQb0lrRVFUbnFzaTZDdTQyRGt1Y3lHclVJ?=
 =?utf-8?B?dkR3RFcxbGUyYzBpVURSY1ZoNCthSVd2NnVuYVh4a3ZXK1NQaHdjUitWSzRB?=
 =?utf-8?B?Ny9ESyswa3hyQVNCRFJGeWF2OWZrbFFyMGNlVXdDNGtFTFp0TytJc1JKYU1W?=
 =?utf-8?B?Wjc4clZGSm9ML2MrTWlILzlvMG5venNGU0ZqeFN0OW1LUGZyajRyTDdXS1U5?=
 =?utf-8?B?YjNVTUVVb2ZzZEJNbTFuSGplZzVSMVJJaHNWMUFqbTJyRDhiZU9uQ3Z3Y2U1?=
 =?utf-8?B?d1FrR05GUnpGM3Zjc1V2SGNaSk1JTnVod1YvMXhCUWtNb29TVXVSN0V3V1JO?=
 =?utf-8?B?bm53RnNMdXVsT0QzelpaRHJRRkgzV0NGbE5QekkrdVh2VDhpQVZ1M3h5VUZj?=
 =?utf-8?B?aDNsdGlSRWlwazZUOFZxQjlJdXQrVy81S2hNcmdzdFM1Z2duODhiaGlPMVY4?=
 =?utf-8?B?TkE9PQ==?=
X-OriginatorOrg: nutanix.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3906e10a-98ba-415e-e019-08da59dc5633
X-MS-Exchange-CrossTenant-AuthSource: PH0PR02MB7384.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jun 2022 14:33:30.2153
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: bb047546-786f-4de1-bd75-24e5b6f79043
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: QQy+ifkFx8qzEJQ0/dzunRJg06u3VAyPvVDjsR3Sq53gxLPvaJsE2WF/aG/TAHQ2+L+VxmAqnNlrFD2wxDTbqlZLBl+U9HgAhgRy4iymyxA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR02MB7963
X-Proofpoint-GUID: V_sg5qgX2uGKR9Zr1oD6V3msqafxhy1J
X-Proofpoint-ORIG-GUID: V_sg5qgX2uGKR9Zr1oD6V3msqafxhy1J
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-06-29_17,2022-06-28_01,2022-06-22_01
X-Proofpoint-Spam-Reason: safe
X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        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 24/06/22 11:06 pm, James Houghton wrote:
> The new function, hugetlb_split_to_shift, will optimally split the page
> table to map a particular address at a particular granularity.
>
> This is useful for punching a hole in the mapping and for mapping small
> sections of a HugeTLB page (via UFFDIO_CONTINUE, for example).
>
> Signed-off-by: James Houghton <jthoughton@google.com>
> ---
>   mm/hugetlb.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 122 insertions(+)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 3ec2a921ee6f..eaffe7b4f67c 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -102,6 +102,18 @@ struct mutex *hugetlb_fault_mutex_table ____cacheline_aligned_in_smp;
>   /* Forward declaration */
>   static int hugetlb_acct_memory(struct hstate *h, long delta);
>   
> +/*
> + * Find the subpage that corresponds to `addr` in `hpage`.
> + */
> +static struct page *hugetlb_find_subpage(struct hstate *h, struct page *hpage,
> +				 unsigned long addr)
> +{
> +	size_t idx = (addr & ~huge_page_mask(h))/PAGE_SIZE;
> +
> +	BUG_ON(idx >= pages_per_huge_page(h));
> +	return &hpage[idx];
> +}
> +
>   static inline bool subpool_is_free(struct hugepage_subpool *spool)
>   {
>   	if (spool->count)
> @@ -7044,6 +7056,116 @@ static unsigned int __shift_for_hstate(struct hstate *h)
>   	for ((tmp_h) = hstate; (shift) = __shift_for_hstate(tmp_h), \
>   			       (tmp_h) <= &hstates[hugetlb_max_hstate]; \
>   			       (tmp_h)++)
> +
> +/*
> + * Given a particular address, split the HugeTLB PTE that currently maps it
> + * so that, for the given address, the PTE that maps it is `desired_shift`.
> + * This function will always split the HugeTLB PTE optimally.
> + *
> + * For example, given a HugeTLB 1G page that is mapped from VA 0 to 1G. If we
> + * call this function with addr=0 and desired_shift=PAGE_SHIFT, will result in
> + * these changes to the page table:
> + * 1. The PUD will be split into 2M PMDs.
> + * 2. The first PMD will be split again into 4K PTEs.
> + */
> +static int hugetlb_split_to_shift(struct mm_struct *mm, struct vm_area_struct *vma,
> +			   const struct hugetlb_pte *hpte,
> +			   unsigned long addr, unsigned long desired_shift)
> +{
> +	unsigned long start, end, curr;
> +	unsigned long desired_sz = 1UL << desired_shift;
> +	struct hstate *h = hstate_vma(vma);
> +	int ret;
> +	struct hugetlb_pte new_hpte;
> +	struct mmu_notifier_range range;
> +	struct page *hpage = NULL;
> +	struct page *subpage;
> +	pte_t old_entry;
> +	struct mmu_gather tlb;
> +
> +	BUG_ON(!hpte->ptep);
> +	BUG_ON(hugetlb_pte_size(hpte) == desired_sz);
> +
> +	start = addr & hugetlb_pte_mask(hpte);
> +	end = start + hugetlb_pte_size(hpte);
> +
> +	i_mmap_assert_write_locked(vma->vm_file->f_mapping);
> +
> +	BUG_ON(!hpte->ptep);
> +	/* This function only works if we are looking at a leaf-level PTE. */
> +	BUG_ON(!hugetlb_pte_none(hpte) && !hugetlb_pte_present_leaf(hpte));
> +
> +	/*
> +	 * Clear the PTE so that we will allocate the PT structures when
> +	 * walking the page table.
> +	 */
> +	old_entry = huge_ptep_get_and_clear(mm, start, hpte->ptep);

Sorry missed it last time, what if hgm mapping present here and current hpte is

at higher level. Where we will clear and free child page-table pages.

I see it does not happen in huge_ptep_get_and_clear.

> +
> +	if (!huge_pte_none(old_entry))
> +		hpage = pte_page(old_entry);
> +
> +	BUG_ON(!IS_ALIGNED(start, desired_sz));
> +	BUG_ON(!IS_ALIGNED(end, desired_sz));
> +
> +	for (curr = start; curr < end;) {
> +		struct hstate *tmp_h;
> +		unsigned int shift;
> +
> +		for_each_hgm_shift(h, tmp_h, shift) {
> +			unsigned long sz = 1UL << shift;
> +
> +			if (!IS_ALIGNED(curr, sz) || curr + sz > end)
> +				continue;
> +			/*
> +			 * If we are including `addr`, we need to make sure
> +			 * splitting down to the correct size. Go to a smaller
> +			 * size if we are not.
> +			 */
> +			if (curr <= addr && curr + sz > addr &&
> +					shift > desired_shift)
> +				continue;
> +
> +			/*
> +			 * Continue the page table walk to the level we want,
> +			 * allocate PT structures as we go.
> +			 */
> +			hugetlb_pte_copy(&new_hpte, hpte);
> +			ret = hugetlb_walk_to(mm, &new_hpte, curr, sz,
> +					      /*stop_at_none=*/false);
> +			if (ret)
> +				goto err;
> +			BUG_ON(hugetlb_pte_size(&new_hpte) != sz);
> +			if (hpage) {
> +				pte_t new_entry;
> +
> +				subpage = hugetlb_find_subpage(h, hpage, curr);
> +				new_entry = make_huge_pte_with_shift(vma, subpage,
> +								     huge_pte_write(old_entry),
> +								     shift);
> +				set_huge_pte_at(mm, curr, new_hpte.ptep, new_entry);
> +			}
> +			curr += sz;
> +			goto next;
> +		}
> +		/* We couldn't find a size that worked. */
> +		BUG();
> +next:
> +		continue;
> +	}
> +
> +	mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma, vma->vm_mm,
> +				start, end);
> +	mmu_notifier_invalidate_range_start(&range);
> +	return 0;
> +err:
> +	tlb_gather_mmu(&tlb, mm);
> +	/* Free any newly allocated page table entries. */
> +	hugetlb_free_range(&tlb, hpte, start, curr);
> +	/* Restore the old entry. */
> +	set_huge_pte_at(mm, start, hpte->ptep, old_entry);
> +	tlb_finish_mmu(&tlb);
> +	return ret;
> +}
>   #endif /* CONFIG_HUGETLB_HIGH_GRANULARITY_MAPPING */
>   
>   /*