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: <f3d8aabe-3d02-8851-b5e9-b0dcdb7c0b6e@bytedance.com>
Date:   Tue, 28 Jun 2022 11:34:29 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.10.0
Subject: Re: [PATCH] mm: hugetlb: kill set_huge_swap_pte_at()
Content-Language: en-US
To:     Matthew Wilcox <willy@infradead.org>
Cc:     mike.kravetz@oracle.com, songmuchun@bytedance.com,
        akpm@linux-foundation.org, catalin.marinas@arm.com,
        will@kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org
References: <20220626145717.53572-1-zhengqi.arch@bytedance.com>
 <YrnBm9FGizF1iLsW@casper.infradead.org>
From:   Qi Zheng <zhengqi.arch@bytedance.com>
In-Reply-To: <YrnBm9FGizF1iLsW@casper.infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2022/6/27 22:41, Matthew Wilcox wrote:
> On Sun, Jun 26, 2022 at 10:57:17PM +0800, Qi Zheng wrote:
>> The commit e5251fd43007 ("mm/hugetlb: introduce set_huge_swap_pte_at()
>> helper") add set_huge_swap_pte_at() to handle swap entries on
>> architectures that support hugepages consisting of contiguous ptes.
>> And currently the set_huge_swap_pte_at() is only overridden by arm64.
> 
> Bleh.  I hate the way we handle these currently.
> 
>> +static inline struct folio *hugetlb_swap_entry_to_folio(swp_entry_t entry)
>> +{
>> +	VM_BUG_ON(!is_migration_entry(entry) && !is_hwpoison_entry(entry));
>> +
>> +	return page_folio(pfn_to_page(swp_offset(entry)));
>> +}
> 
> We haven't needed a pfn_to_folio() yet, but perhaps we should have one?

Hi,

IMO, it would be better to have a pfn_to_folio(), which can save the
redundant page_folio() call in the current case.

But this is not related to the current patch, maybe it can be a
separate optimization patch.

> 
> Related, how should we store migration entries for multi-order folios
> in the page tables?  We can either encode the individual page in
> question, or we can encode the folio.  Do we need to support folios
> being mapped askew (ie unaligned), or will folios always be mapped
> aligned?

Do we currently have a scenario where we need to use skew mapped folios?
Maybe it can be used in pte-mapped THP? Hmm, I have no idea.

> 
>> +	if (!pte_present(pte)) {
>> +		struct folio *folio;
>> +
>> +		folio = hugetlb_swap_entry_to_folio(pte_to_swp_entry(pte));
>> +		ncontig = num_contig_ptes(folio_size(folio), &pgsize);
>> +
>> +		for (i = 0; i < ncontig; i++, ptep++)
>> +			set_pte_at(mm, addr, ptep, pte);
>> +		return;
>> +	}
> 
> It seems like a shame to calculate folio_size() only to turn it into a
> number of pages.  Don't you want to just use:
> 
> 		ncontig = folio_nr_pages(folio);

We can't use folio_nr_pages() here, because for PMD_SIZE we only need
one entry instead of the PTRS_PER_PTE entries returned by
folio_nr_pages().

Thanks,
Qi

> 

-- 
Thanks,
Qi