From: Oleg Nesterov <oleg@redhat.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Will Deacon <will.deacon@arm.com>,
hpa@zytor.com, luto@amacapital.net, dave.hansen@linux.intel.com,
mingo@elte.hu, minchan@kernel.org, tglx@linutronix.de,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: LTP regressions due to 6dc296e7df4c ("mm: make sure all file VMAs have ->vm_ops set")
Date: Tue, 15 Sep 2015 14:12:01 +0200 [thread overview]
Message-ID: <20150915121201.GA10104@redhat.com> (raw)
In-Reply-To: <20150914182033.GA4165@node.dhcp.inet.fi>
On 09/14, Kirill A. Shutemov wrote:
>
> On Mon, Sep 14, 2015 at 07:05:47PM +0200, Oleg Nesterov wrote:
> > On 09/14, Kirill A. Shutemov wrote:
> > >
> > > Fix is below. I don't really like it, but I cannot find any better
> > > solution.
> >
> > Me too...
> >
> > But this change "documents" the nasty special "vm_file && !vm_ops" case, and
> > I am not sure how we can remove it later...
> >
> > So perhaps we should change vma_is_anonymous() back to check ->fault too,
> >
> > static inline bool vma_is_anonymous(struct vm_area_struct *vma)
> > {
> > - return !vma->vm_ops;
> > + return !vma->vm_ops || !vma->vm_ops->fault;
>
> No. This would give a lot false positives from drives which setup page
> tables upfront and don't use ->fault at all.
And? I mean, I am not sure I understand what exactly do you dislike.
Firstly, I still think that (in the long term) we should change them
to use .faul = no_fault() which just returns VM_FAULT_SIGBUS.
Until then I do not see why the change above can be really bad. The
VM_SHARED case is fine, do_anonymous_page() will return VM_FAULT_SIGBUS.
So afaics the only problem is that after the change above the private
mapping can silently get an anonymous page after (say) MADV_DONTNEED
instead of the nice SIGBUS from do_fault(). I agree, this is not good,
but see above.
Or I missed something else?
Let me repeat, I am not going to really argue, you understand this all
much better than me. But imho we should try to avoid the special case
added by your change as much as possible, in this sense the change above
looks "obviously better" at least as a short-term fix.
Whether we need to keep the vm_ops/fault check in __vma_link_rb() and
mmap_region() is another issue. But if we keep them, then I think we
should at least turn the !vma->vm_ops check in mmap_region into
WARN_ON() as well.
Oleg.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2015-09-15 12:15 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-14 10:53 LTP regressions due to 6dc296e7df4c ("mm: make sure all file VMAs have ->vm_ops set") Will Deacon
2015-09-14 11:57 ` Kirill A. Shutemov
2015-09-14 13:22 ` Will Deacon
2015-09-14 17:05 ` Oleg Nesterov
2015-09-14 17:46 ` Oleg Nesterov
2015-09-14 18:20 ` Kirill A. Shutemov
2015-09-15 12:12 ` Oleg Nesterov [this message]
2015-09-15 13:42 ` Kirill A. Shutemov
2015-09-15 15:13 ` Oleg Nesterov
2015-09-16 21:28 ` Andrew Morton
2015-09-17 15:37 ` Kirill A. Shutemov
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=20150915121201.GA10104@redhat.com \
--to=oleg@redhat.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=kirill@shutemov.name \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@amacapital.net \
--cc=minchan@kernel.org \
--cc=mingo@elte.hu \
--cc=tglx@linutronix.de \
--cc=will.deacon@arm.com \
/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).