On Tue, Jan 5, 2021 at 6:40 AM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote: > > On 1/3/21 10:58 PM, Muchun Song wrote: > > Because we only can isolate a active page via isolate_huge_page() > > and hugetlbfs_fallocate() forget to mark it as active, we cannot > > isolate and migrate those pages. > > > > Fixes: 70c3547e36f5 (hugetlbfs: add hugetlbfs_fallocate()) > > Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx> > > --- > > fs/hugetlbfs/inode.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > Good catch. This is indeed an issue. > > > > > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c > > index b5c109703daa..2aceb085d202 100644 > > --- a/fs/hugetlbfs/inode.c > > +++ b/fs/hugetlbfs/inode.c > > @@ -737,10 +737,11 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset, > > > > /* > > * unlock_page because locked by add_to_page_cache() > > - * page_put due to reference from alloc_huge_page() > > + * put_page() (which is in the putback_active_hugepage()) > > + * due to reference from alloc_huge_page() > > Thanks for fixing the comment. > > > */ > > unlock_page(page); > > - put_page(page); > > + putback_active_hugepage(page); > > I'm curious why you used putback_active_hugepage() here instead of simply > calling set_page_huge_active() before the put_page()? > > When the page was allocated, it was placed on the active list (alloc_huge_page). > Therefore, the hugetlb_lock locking and list movement should not be necessary. I agree with you. Because set_page_huge_active is not exported (static function). Only exporting set_page_huge_active seems strange (leaving clear_page_huge_active not export). This is just my opinion. What's yours, Mike? Thanks. > > -- > Mike Kravetz > > > } > > > > if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + len > inode->i_size) > >
