| Message ID | 20191017164223.2762148-5-songliubraving@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for THP in page cache | expand |
On Thu, Oct 17, 2019 at 9:42 AM Song Liu <songliubraving@fb.com> wrote: > > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > Once a THP is added to the page cache, it cannot be dropped via > /proc/sys/vm/drop_caches. Fix this issue with proper handling in > invalidate_mapping_pages(). > > Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS") > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Tested-by: Song Liu <songliubraving@fb.com> > Signed-off-by: Song Liu <songliubraving@fb.com> > --- > mm/truncate.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/mm/truncate.c b/mm/truncate.c > index 8563339041f6..dd9ebc1da356 100644 > --- a/mm/truncate.c > +++ b/mm/truncate.c > @@ -592,6 +592,16 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping, > unlock_page(page); > continue; > } > + > + /* Take a pin outside pagevec */ > + get_page(page); > + > + /* > + * Drop extra pins before trying to invalidate > + * the huge page. > + */ > + pagevec_remove_exceptionals(&pvec); > + pagevec_release(&pvec); Shall we skip the outer pagevec_remove_exceptions() if it has been done here? > } > > ret = invalidate_inode_page(page); > @@ -602,6 +612,8 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping, > */ > if (!ret) > deactivate_file_page(page); > + if (PageTransHuge(page)) > + put_page(page); > count += ret; > } > pagevec_remove_exceptionals(&pvec); > -- > 2.17.1 > >
On Thu, Oct 17, 2019 at 02:46:38PM -0700, Yang Shi wrote: > On Thu, Oct 17, 2019 at 9:42 AM Song Liu <songliubraving@fb.com> wrote: > > > > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > > > Once a THP is added to the page cache, it cannot be dropped via > > /proc/sys/vm/drop_caches. Fix this issue with proper handling in > > invalidate_mapping_pages(). > > > > Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS") > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > Tested-by: Song Liu <songliubraving@fb.com> > > Signed-off-by: Song Liu <songliubraving@fb.com> > > --- > > mm/truncate.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/mm/truncate.c b/mm/truncate.c > > index 8563339041f6..dd9ebc1da356 100644 > > --- a/mm/truncate.c > > +++ b/mm/truncate.c > > @@ -592,6 +592,16 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping, > > unlock_page(page); > > continue; > > } > > + > > + /* Take a pin outside pagevec */ > > + get_page(page); > > + > > + /* > > + * Drop extra pins before trying to invalidate > > + * the huge page. > > + */ > > + pagevec_remove_exceptionals(&pvec); > > + pagevec_release(&pvec); > > Shall we skip the outer pagevec_remove_exceptions() if it has been done here? It will be NOP and skipping would complicate the code.
On Fri, Oct 18, 2019 at 6:32 AM Kirill A. Shutemov <kirill@shutemov.name> wrote: > > On Thu, Oct 17, 2019 at 02:46:38PM -0700, Yang Shi wrote: > > On Thu, Oct 17, 2019 at 9:42 AM Song Liu <songliubraving@fb.com> wrote: > > > > > > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > > > > > > Once a THP is added to the page cache, it cannot be dropped via > > > /proc/sys/vm/drop_caches. Fix this issue with proper handling in > > > invalidate_mapping_pages(). > > > > > > Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS") > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > > Tested-by: Song Liu <songliubraving@fb.com> > > > Signed-off-by: Song Liu <songliubraving@fb.com> > > > --- > > > mm/truncate.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/mm/truncate.c b/mm/truncate.c > > > index 8563339041f6..dd9ebc1da356 100644 > > > --- a/mm/truncate.c > > > +++ b/mm/truncate.c > > > @@ -592,6 +592,16 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping, > > > unlock_page(page); > > > continue; > > > } > > > + > > > + /* Take a pin outside pagevec */ > > > + get_page(page); > > > + > > > + /* > > > + * Drop extra pins before trying to invalidate > > > + * the huge page. > > > + */ > > > + pagevec_remove_exceptionals(&pvec); > > > + pagevec_release(&pvec); > > > > Shall we skip the outer pagevec_remove_exceptions() if it has been done here? > > It will be NOP and skipping would complicate the code. Yes, it would be. Anyway, it looks ok too. Acked-by: Yang Shi <yang.shi@linux.alibaba.com> > > -- > Kirill A. Shutemov
diff --git a/mm/truncate.c b/mm/truncate.c index 8563339041f6..dd9ebc1da356 100644 --- a/mm/truncate.c +++ b/mm/truncate.c @@ -592,6 +592,16 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping, unlock_page(page); continue; } + + /* Take a pin outside pagevec */ + get_page(page); + + /* + * Drop extra pins before trying to invalidate + * the huge page. + */ + pagevec_remove_exceptionals(&pvec); + pagevec_release(&pvec); } ret = invalidate_inode_page(page); @@ -602,6 +612,8 @@ unsigned long invalidate_mapping_pages(struct address_space *mapping, */ if (!ret) deactivate_file_page(page); + if (PageTransHuge(page)) + put_page(page); count += ret; } pagevec_remove_exceptionals(&pvec);