On Mon, Jul 30, 2018 at 6:01 AM Kirill A. Shutemov <kirill@xxxxxxxxxxxxx> wrote:
>
> I think I missed vma_set_anonymous() somewhere, but I fail to see where.
Honestly, by now we just need to revert that commit.
It's not even clear that it was a good idea to begin with. The rest of
the commits were cleanups, this one was driven by a incorrect
VM_BUG_ON() that triggered, and that checked "vma_is_anonymous(vma)"
without any explanations of wht it should matter.
I think the biggest problem with vma_is_anonymous() may be its name,
not what it does.
What the code historically *did* (and what vma_is_anonymous() checks)
is not "is this anonymous", but rather "does this have any special
operations associated with it".
The two are similar. But people have grown opinions about exactly what
"anonymous" means. If we had named it just "no_vm_ops()", we wouldn't
have random crazy checks for "vma_is_anonymous()" in places where it
makes no sense.
So what I think we want a real explanation for is why people who use
"vma_is_anonymous()" care. Instead of trying to change its very
historical meaning, we should look at the users, and perhaps change
its name.
In this case, for example, I think the *real* problem was described by
commit 684283988f70 ("huge pagecache: mmap_sem is unlocked when
truncation splits pmd"), and the problem is that an existing check
that required that mmap_sem was held was changed to say "only for
anonymous mappings".
But the fact is, you can truncate mappings that don't have any ops just *fine*.
So maybe that original BUG() was entirely bogus to begin with, and it
shouldn't exist at all?
Or maybe the code should test "do I have a vm_file" instead of testing
"do I have vm_ops"?
What's the problem with just doing split_huge_pmd() there when it's a
pmd_trans_huge or pmd_devmap pmd? Why is that VM_BUG_ON_VMA() there in
the first place? Why are allegedly "anonymous" mappings so special
here for locking?
Adding a few more people to the cc, they were involved the last that
time VM_BUG_ON_VMA() was modified.
New people: see commit bfd40eaff5ab ("mm: fix vma_is_anonymous()
false-positives") for details. Right now I think it's getting
reverted, but the oops explanation in the commit is about that
kernel BUG at mm/memory.c:1422!
which was/is debatable and seems to make no sense (and definitely is
still triggerable despite that commit 684283988f70 ("huge pagecache:
mmap_sem is unlocked when truncation splits pmd") that limited it a
bit - but I think it didn't limit it enough.
Linus
