On Tue, Mar 03, 2020 at 01:52:10PM -0800, Alexander Duyck wrote:
> On Mon, Mar 2, 2020 at 8:11 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> > @@ -1986,14 +1984,10 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages,
> > khugepaged_scan.address + HPAGE_PMD_SIZE >
> > hend);
> > if (IS_ENABLED(CONFIG_SHMEM) && vma->vm_file) {
> > - struct file *file;
> > + struct file *file = get_file(vma->vm_file);
> > pgoff_t pgoff = linear_page_index(vma,
> > khugepaged_scan.address);
> >
> > - if (shmem_file(vma->vm_file)
> > - && !shmem_huge_enabled(vma))
> > - goto skip;
> > - file = get_file(vma->vm_file);
>
> In the code above you didn't eliminate shmem_huge_enabled, it still
> exists and has multiple paths that can return false. Are we guaranteed
> that it will return true or is it that it can be ignored here?
It probably helps to read this in conjunction with e496cf3d7821 --
that patch did this:
if (shmem_file(vma->vm_file)) {
- struct file *file = get_file(vma->vm_file);
+ struct file *file;
pgoff_t pgoff = linear_page_index(vma,
khugepaged_scan.address);
+ if (!shmem_huge_enabled(vma))
+ goto skip;
+ file = get_file(vma->vm_file);
up_read(&mm->mmap_sem);
so I'm just undoing that.
