| Message ID | 20220116121822.1727633-5-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enabling large folios for 5.17 | expand |
On Sun, Jan 16, 2022 at 12:18:14PM +0000, Matthew Wilcox (Oracle) wrote: > We have to allocate memory in order to split a file-backed folio, so > it's not a good idea to split them in the memory freeing path. Could elaborate on why split a file-backed folio requires memory allocation? > It also > doesn't work for XFS because pages have an extra reference count from > page_has_private() and split_huge_page() expects that reference to have > already been removed. Need to adjust can_split_huge_page()? > Unfortunately, we still have to split shmem THPs > because we can't handle swapping out an entire THP yet. ... especially if the system doesn't have swap :P > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > mm/vmscan.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 700434db5735..45665874082d 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1728,8 +1728,8 @@ static unsigned int shrink_page_list(struct list_head *page_list, > /* Adding to swap updated mapping */ > mapping = page_mapping(page); > } > - } else if (unlikely(PageTransHuge(page))) { > - /* Split file THP */ > + } else if (PageSwapBacked(page) && PageTransHuge(page)) { > + /* Split shmem THP */ > if (split_huge_page_to_list(page, page_list)) > goto keep_locked; > } > -- > 2.34.1 >
On Mon, Jan 17, 2022 at 07:06:25PM +0300, Kirill A. Shutemov wrote: > On Sun, Jan 16, 2022 at 12:18:14PM +0000, Matthew Wilcox (Oracle) wrote: > > We have to allocate memory in order to split a file-backed folio, so > > it's not a good idea to split them in the memory freeing path. > > Could elaborate on why split a file-backed folio requires memory > allocation? In the commit message or explain it to you now? We need to allocate xarray nodes to store all the newly-independent pages. With a folio that's more than 64 entries in size (current implementation), we elide the lowest layer of the radix tree. But with any data structure that tracks folios, we'll need to create space in it to track N folios instead of 1. > > It also > > doesn't work for XFS because pages have an extra reference count from > > page_has_private() and split_huge_page() expects that reference to have > > already been removed. > > Need to adjust can_split_huge_page()? no? > > Unfortunately, we still have to split shmem THPs > > because we can't handle swapping out an entire THP yet. > > ... especially if the system doesn't have swap :P Not sure what correction to the commit message you want here.
On Mon, Jan 17, 2022 at 04:10:46PM +0000, Matthew Wilcox wrote: > On Mon, Jan 17, 2022 at 07:06:25PM +0300, Kirill A. Shutemov wrote: > > On Sun, Jan 16, 2022 at 12:18:14PM +0000, Matthew Wilcox (Oracle) wrote: > > > We have to allocate memory in order to split a file-backed folio, so > > > it's not a good idea to split them in the memory freeing path. > > > > Could elaborate on why split a file-backed folio requires memory > > allocation? > > In the commit message or explain it to you now? > > We need to allocate xarray nodes to store all the newly-independent > pages. With a folio that's more than 64 entries in size (current > implementation), we elide the lowest layer of the radix tree. But > with any data structure that tracks folios, we'll need to create > space in it to track N folios instead of 1. Looks good. > > > It also > > > doesn't work for XFS because pages have an extra reference count from > > > page_has_private() and split_huge_page() expects that reference to have > > > already been removed. > > > > Need to adjust can_split_huge_page()? > > no? I meant we can make can_split_huge_page() expect extra pin if page_has_private() is true. If it is the only thing that stops split_huge_page() from handling XFS pages.
diff --git a/mm/vmscan.c b/mm/vmscan.c index 700434db5735..45665874082d 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1728,8 +1728,8 @@ static unsigned int shrink_page_list(struct list_head *page_list, /* Adding to swap updated mapping */ mapping = page_mapping(page); } - } else if (unlikely(PageTransHuge(page))) { - /* Split file THP */ + } else if (PageSwapBacked(page) && PageTransHuge(page)) { + /* Split shmem THP */ if (split_huge_page_to_list(page, page_list)) goto keep_locked; }
We have to allocate memory in order to split a file-backed folio, so it's not a good idea to split them in the memory freeing path. It also doesn't work for XFS because pages have an extra reference count from page_has_private() and split_huge_page() expects that reference to have already been removed. Unfortunately, we still have to split shmem THPs because we can't handle swapping out an entire THP yet. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- mm/vmscan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)