On 1/15/20 1:31 AM, Michal Hocko wrote: > On Tue 14-01-20 21:47:51, Li Xinhai wrote: >> On 2020-01-14 at 17:25 Michal Hocko wrote: >>> On Sat 11-01-20 10:18:05, Li Xinhai wrote: >>>> When check_pte, pfn of normal, hugetlbfs and THP page need be compared. >>>> The current implementation apply comparison as >>>> - normal 4K page: page_pfn <= pfn < page_pfn + 1 >>>> - hugetlbfs page: page_pfn <= pfn < page_pfn + HPAGE_PMD_NR >>>> - THP page: page_pfn <= pfn < page_pfn + HPAGE_PMD_NR >>>> in pfn_in_hpage. For hugetlbfs page, it should be >>>> page_pfn == pfn >>>> >>>> Now, change pfn_in_hpage to pfn_is_match to highlight that comparison >>>> is not only for THP and explicitly compare for these cases. >>> >>> Why is this important to do. I have asked and Mike had the same feeling >>> that the patch is missing any real justification. Why do we need this >>> change? It is great that you have dropped VM_BUG_ON btw. >>> >> I think it is important to make the code clear, as said, comparing hugetlbfs page >> in range page_pfn <= pfn < page_pfn + HPAGE_PMD_NR is confusion. > > I am sorry but I do not really see a big confusion here. It is probably > a matter of taste. If others consider this an improvement then I will > not stand in the way but I consider the justification insuficient for > merging. Perhaps I am confused, but the patch does update the check done for hugetlb pages. IIUC, previously there was no distinction between THP pages and hugetlb pages in the check. It is valid to pass in a sub- page of a THP page, but not that of a hugetlb page. I do not believe it is possible for existing code to pass in a sub-page of a hugetlb page. And, no one has ever seen this as an issue. This is why I was questioning the need for the patch. With all that said, the new code/check is better (more complete) than the original. It may not do anything for the current code base, but it 'could' catch potential errors in future code. Because of this, I do consider the code an improvement. Acked-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx> -- Mike Kravetz
