On Thu, May 18, 2017 at 03:28:18PM +0200, Jan Kara wrote:
> Hi Kirill,
>
> in commit fc127da085c26 "truncate: handle file thp" you've added the
> following to invalidate_mapping_pages():
>
> /* Middle of THP: skip */
> if (PageTransTail(page)) {
> unlock_page(page);
> continue;
> } else if (PageTransHuge(page)) {
> index += HPAGE_PMD_NR - 1;
> i += HPAGE_PMD_NR - 1;
> /* 'end' is in the middle of THP */
> if (index == round_down(end, HPAGE_PMD_NR))
> continue;
> }
>
> Now how can ever condition "if (index == round_down(end,
> HPAGE_PMD_NR))" be true? We have just added HPAGE_PMD_NR - 1 to 'index'
> so it will not be a multiple of HPAGE_PMD_NR. Presumably you wanted to
> check whether the current THP is the one containing 'end' here which would
> be something like 'round_down(index, HPAGE_PMD_NR) == round_down(end,
> HPAGE_PMD_NR)'.
You're right, it's a bug. 'page->index' instead of 'index' should do the
trick.
Would you like to prepare the patch? (I'm deep in 5-level paging at the
moment.)
> but then I still miss why you'd like to avoid invalidating the partial
> THP at the end of file... Can you please enlighten me? Thanks!
My logic was that the data in the non-invalidated part of the page can be
still useful and it's better to leave it in page cache.
I don't have performance numbers to validate my intuition.
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
