Messages in this thread | | | Date | Wed, 9 Jun 2021 13:08:16 +0300 | From | "Kirill A. Shutemov" <> | Subject | Re: [PATCH v2] mm, thp: use head page in __migration_entry_wait |
| |
On Tue, Jun 08, 2021 at 09:22:28PM +0800, Yu Xu wrote: > On 6/8/21 8:00 PM, Kirill A. Shutemov wrote: > > On Tue, Jun 08, 2021 at 05:22:39PM +0800, Xu Yu wrote: > > > We notice that hung task happens in a conner but practical scenario when > > > CONFIG_PREEMPT_NONE is enabled, as follows. > > > > > > Process 0 Process 1 Process 2..Inf > > > split_huge_page_to_list > > > unmap_page > > > split_huge_pmd_address > > > __migration_entry_wait(head) > > > __migration_entry_wait(tail) > > > remap_page (roll back) > > > remove_migration_ptes > > > rmap_walk_anon > > > cond_resched > > > > > > Where __migration_entry_wait(tail) is occurred in kernel space, e.g., > > > copy_to_user in fstat, which will immediately fault again without > > > rescheduling, and thus occupy the cpu fully. > > > > > > When there are too many processes performing __migration_entry_wait on > > > tail page, remap_page will never be done after cond_resched. > > > > > > This makes __migration_entry_wait operate on the compound head page, > > > thus waits for remap_page to complete, whether the THP is split > > > successfully or roll back. > > > > > > Note that put_and_wait_on_page_locked helps to drop the page reference > > > acquired with get_page_unless_zero, as soon as the page is on the wait > > > queue, before actually waiting. So splitting the THP is only prevented > > > for a brief interval. > > > > > > Fixes: ba98828088ad ("thp: add option to setup migration entries during PMD split") > > > Suggested-by: Hugh Dickins <hughd@google.com> > > > Signed-off-by: Gang Deng <gavin.dg@linux.alibaba.com> > > > Signed-off-by: Xu Yu <xuyu@linux.alibaba.com> > > > > Looks good to me: > > > > Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > > > But there's one quirk: if split succeed we effectively wait on wrong > > page to be unlocked. And it may take indefinite time if split_huge_page() > > was called on the head page. > > Inspired by you, I look into the codes, and have a new question (nothing > to do with this patch). > > If we split_huge_page_to_list on *tail* page (in fact, I haven't seen > that used yet),
See ksm code for instance.
> mm/huge_memory.c:2666 checks "VM_BUG_ON_PAGE(!PageLocked(head), head);" > in split_huge_page_to_list(), while > > mm/huge_memory.c:2497 does "unlock_page(subpage)", where subpage can > be head in this scenario, in __split_huge_page(). > > My confusion is > 1) how the pin on the @subpage is got outside split_huge_page_to_list()? > can we ever get tail page?
This loop:
for (i = 0; i < nr; i++) { struct page *subpage = head + i; if (subpage == page) continue; unlock_page(subpage);
/* * Subpages may be freed if there wasn't any mapping * like if add_to_swap() is running on a lru page that * had its mapping zapped. And freeing these pages * requires taking the lru_lock so we do the put_page * of the tail pages after the split is complete. */ put_page(subpage); }
We skip unlocking and unpinning the page split_huge_page() got called for.
> > 2) head page is locked outside split_huge_page_to_list(), but unlocked > in __split_huge_page()?
If called on tail page, yes.
-- Kirill A. Shutemov
| |