On Thu, Jan 14, 2021 at 7:27 AM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote: > > On 1/13/21 1:20 AM, Oscar Salvador wrote: > > On Tue, Jan 12, 2021 at 07:33:33PM +0800, Muchun Song wrote: > >>> It seems a bit odd to only pass "start" for the BUG_ON. > >>> Also, I kind of dislike the "addr += PAGE_SIZE" in vmemmap_pte_range. > >>> > >>> I wonder if adding a ".remap_start_addr" would make more sense. > >>> And adding it here with the vmemmap_remap_walk init. > >> > >> How about introducing a new function which aims to get the reuse > >> page? In this case, we can drop the BUG_ON() and "addr += PAGE_SIZE" > >> which is in vmemmap_pte_range. The vmemmap_remap_range only > >> does the remapping. > > > > How would that look? > > It might be good, dunno, but the point is, we should try to make the rules as > > simple as possible, dropping weird assumptions. > > > > Callers of vmemmap_remap_free should know three things: > > > > - Range to be remapped > > - Addr to remap to > > - Current implemantion needs addr to be remap to to be part of the complete > > range > > > > right? > > And, current implementation needs must have remap addr be the first in the > complete range. This is just because of the way the page tables are walked > for remapping. The remap/reuse page must be found first so that the following > pages can be remapped to it. You are right. > > That implementation seems to be the 'most efficient' for hugetlb pages where > we want vmemmap pages n+3 and beyond mapped to n+2. > > In a more general purpose vmemmap_remap_free implementation, the reuse/remap > address would not necessarily need to be related to the range. However, this > would require a separate page table walk/validation for the reuse address > independent of the range. This may be what Muchun was proposing for 'a new > function which aims to get the reuse page'. Agree. > > IMO, the decision on how to implement depends on the intended use case. > - If this is going to be hugetlb only (or perhaps generic huge page only) > functionality, then I am OK with an efficient implementation that has > some restrictions. > - If we see this being used for more general purpose remapping, then we > should go with a more general purpose implementation. I think this approach may be only suitable for generic huge page only. So we can implement it only for huge page. Hi Oscar, What's your opinion about this? > > Again, just my opinion. > -- > Mike Kravetz
