On Tue, Mar 3, 2020 at 2:34 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> 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.
It looks like you are only doing a partial revert though. The original
changes in the function were:
@@ -1681,8 +1683,6 @@ static unsigned int
khugepaged_scan_mm_slot(unsigned int pages,
if (khugepaged_scan.address < hstart)
khugepaged_scan.address = hstart;
VM_BUG_ON(khugepaged_scan.address & ~HPAGE_PMD_MASK);
- if (shmem_file(vma->vm_file) && !shmem_huge_enabled(vma))
- goto skip;
while (khugepaged_scan.address < hend) {
int ret;
@@ -1694,9 +1694,12 @@ static unsigned int
khugepaged_scan_mm_slot(unsigned int pages,
khugepaged_scan.address + HPAGE_PMD_SIZE >
hend);
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);
ret = 1;
khugepaged_scan_shmem(mm, file->f_mapping,
You reverted the second piece, but I didn't notice you reverting the
first. WIth the first piece being reverted it would make more sense as
you would just be skipping the block that much sooner.
