Hi: On 2021/1/20 2:41, Mike Kravetz wrote: > 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@xxxxxxxxxx> >> --- >> 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. > */ > Many thanks for detailed and excellent comment. Will do it in v2. Thanks again.
