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@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>