From: Alexander Duyck <alexander.duyck@gmail.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-mm <linux-mm@kvack.org>,
"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
"Aneesh Kumar K . V" <aneesh.kumar@linux.vnet.ibm.com>
Subject: Re: [PATCH 3/6] mm: Remove CONFIG_TRANSPARENT_HUGE_PAGECACHE
Date: Tue, 3 Mar 2020 14:54:55 -0800 [thread overview]
Message-ID: <CAKgT0UfgTu1E4UbprPHWkqrE7GVQAY+zncriTUAiWJtBNKn46w@mail.gmail.com> (raw)
In-Reply-To: <20200303223401.GX29971@bombadil.infradead.org>
On Tue, Mar 3, 2020 at 2:34 PM Matthew Wilcox <willy@infradead.org> 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@infradead.org> 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.
next prev parent reply other threads:[~2020-03-03 22:55 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-03 4:11 [PATCH 0/6] Misc MM patches Matthew Wilcox
2020-03-03 4:11 ` [PATCH 1/6] mm: Use vm_fault error code directly Matthew Wilcox
2020-03-03 4:11 ` [PATCH 2/6] mm: Optimise find_subpage for !THP Matthew Wilcox
2020-03-03 21:28 ` Alexander Duyck
2020-03-03 21:47 ` Matthew Wilcox
2020-03-05 9:54 ` William Kucharski
2020-03-03 4:11 ` [PATCH 3/6] mm: Remove CONFIG_TRANSPARENT_HUGE_PAGECACHE Matthew Wilcox
2020-03-03 21:52 ` Alexander Duyck
2020-03-03 22:34 ` Matthew Wilcox
2020-03-03 22:54 ` Alexander Duyck [this message]
2020-03-04 2:06 ` Matthew Wilcox
2020-03-03 4:11 ` [PATCH 4/6] mm: Use VM_BUG_ON_PAGE in clear_page_dirty_for_io Matthew Wilcox
2020-03-03 4:11 ` [PATCH 5/6] mm: Unexport find_get_entry Matthew Wilcox
2020-03-03 4:11 ` [PATCH 6/6] mm: Rewrite pagecache_get_page documentation Matthew Wilcox
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKgT0UfgTu1E4UbprPHWkqrE7GVQAY+zncriTUAiWJtBNKn46w@mail.gmail.com \
--to=alexander.duyck@gmail.com \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-mm@kvack.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).