Re: [PATCH] mm-move-mremap-from-file_operations-to-vm_operations_struct-fix

[Benjamin LaHaise <bcrl@kvack.org>]

On Fri, Jul 17, 2015 at 01:52:28AM +0200, Oleg Nesterov wrote:
> On 07/16, Andrew Morton wrote:
> >
> > On Fri, 17 Jul 2015 01:14:05 +0200 Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > > fs/aio.c can't be compiled if CONFIG_MMU=n, filemap_page_mkwrite()
> > > is not defined in this case. Add yet another "must not be called"
> > > helper into nommu.c to make the linker happy.
> > >
> > > I still think this is pointless, afaics sys_io_setup() simply can't
> > > succeed if CONFIG_MMU=n. Instead we should make CONFIG_AIO depend
> > > on CONFIG_MMU.
> > >
> > > ..
> > >
> > > --- a/mm/nommu.c
> > > +++ b/mm/nommu.c
> > > @@ -2008,6 +2008,12 @@ void filemap_map_pages(struct vm_area_struct *vma, struct vm_fault *vmf)
> > >  }
> > >  EXPORT_SYMBOL(filemap_map_pages);
> > >
> > > +int filemap_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
> > > +{
> > > +	BUG();
> > > +	return 0;
> > > +}
> > > +
> > >  static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> > >  		unsigned long addr, void *buf, int len, int write)
> > >  {
> >
> > So if anyone starts testing aio on NOMMU, this patch will make the
> > whole thing immediately go BUG.  This isn't helpful :(
> 
> Well, I'm afraid I could miss something, but _afaics_ this can not
> happen. filemap_page_mkwrite() can't be called if NOMMU.
> 
> In particular, simply because sys_io_setup() is the only user (if
> NOMMU) and it can't succeed. But even if I missed something and it
> can succeed, ->page_mkwrite() must not be called anyway. But this,
> again, unless I missed something ;)
> 
> > Yes, making AIO depend on MMU sounds better.
> 
> Perhaps Benjamin can change his mind or correct me.

Either try to fix it correctly, or disable the config.  Making it just 
compile but be knowingly broken is worse than either of those 2 options.  
My point was that it is valid for someone to want to use the functionality 
on a nommu system, and given that it should have worked before the page 
migration code was added, It Would Be Nice(tm) to return it to that state.  
Adding a BUG() like that to the code is just plain broken.

		-ben

> > Because if it wasn't
> > busted before, it sure is now!
> 
> I hope this change can't make any difference.
> 
> Oleg.

-- 
"Thought is the essence of where you are now."