Messages in this thread | | | Subject | Re: [PATCH] hugetlbfs: remove meaningless variable avoid_reserve | From | Mike Kravetz <> | Date | Tue, 19 Jan 2021 10:41:58 -0800 |
| |
Please CC Andrew on hugetlb patches as they need to go through his tree.
On 1/16/21 1:26 AM, Miaohe Lin wrote: > The variable avoid_reserve is meaningless because we never changed its > value and just passed it to alloc_huge_page(). So remove it to make code > more clear that in hugetlbfs_fallocate, we never avoid reserve when alloc > hugepage yet.
One might argue that using a named variable makes the call to alloc_huge_page more clear. I do not disagree with the change, However, there are some subtle reasons why alloc_huge_page is called with 'avoid_reserve = 0' from fallocate. Therefore, I would prefer that a comment be added above the call in addition to this change. See below.
> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com> > --- > fs/hugetlbfs/inode.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c > index 88751e35e69d..23ad6ed8b75f 100644 > --- a/fs/hugetlbfs/inode.c > +++ b/fs/hugetlbfs/inode.c > @@ -680,7 +680,6 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset, > */ > struct page *page; > unsigned long addr; > - int avoid_reserve = 0; > > cond_resched(); > > @@ -717,7 +716,7 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset, > } > > /* Allocate page and add to page cache */
Perhaps, change comment to read:
/* * Allocate page without setting the avoid_reserve argument. * There certainly are no reserves associated with the * pseudo_vma. However, there could be shared mappings with * reserves for the file at the inode level. If we fallocate * pages in these areas, we need to consume the reserves * to keep reservation accounting consistent. */
-- Mike Kravetz
> - page = alloc_huge_page(&pseudo_vma, addr, avoid_reserve); > + page = alloc_huge_page(&pseudo_vma, addr, 0); > hugetlb_drop_vma_policy(&pseudo_vma); > if (IS_ERR(page)) { > mutex_unlock(&hugetlb_fault_mutex_table[hash]); >
| |