RE: [PATCH v3 1/1] mm: introduce put_user_page*(), placeholder versions

[Weiny, Ira]

> Subject: Re: [PATCH v3 1/1] mm: introduce put_user_page*(), placeholder
> versions
> 
> On 3/7/19 6:58 PM, Christopher Lameter wrote:
> > On Wed, 6 Mar 2019, john.hubbard@xxxxxxxxx wrote:
> >
> >> Dave Chinner's description of this is very clear:
> >>
> >>     "The fundamental issue is that ->page_mkwrite must be called on every
> >>     write access to a clean file backed page, not just the first one.
> >>     How long the GUP reference lasts is irrelevant, if the page is clean
> >>     and you need to dirty it, you must call ->page_mkwrite before it is
> >>     marked writeable and dirtied. Every. Time."
> >>
> >> This is just one symptom of the larger design problem: filesystems do
> >> not actually support get_user_pages() being called on their pages,
> >> and letting hardware write directly to those pages--even though that
> >> patter has been going on since about 2005 or so.
> >
> > Can we distinguish between real filesystems that actually write to a
> > backing device and the special filesystems (like hugetlbfs, shm and
> > friends) that are like anonymous memory and do not require
> > ->page_mkwrite() in the same way as regular filesystems?
> 
> Yes. I'll change the wording in the commit message to say "real filesystems
> that actually write to a backing device", instead of "filesystems". That does
> help, thanks.
> 
> >
> > The use that I have seen in my section of the world has been
> > restricted to RDMA and get_user_pages being limited to anonymous
> > memory and those special filesystems. And if the RDMA memory is of
> > such type then the use in the past and present is safe.
> 
> Agreed.
> 
> >
> > So a logical other approach would be to simply not allow the use of
> > long term get_user_page() on real filesystem pages. I hope this patch
> > supports that?
> 
> This patch neither prevents nor provides that. What this patch does is
> provide a prerequisite to clear identification of pages that have had
> get_user_pages() called on them.
> 
> 
> >
> > It is customary after all that a file read or write operation involve
> > one single file(!) and that what is written either comes from or goes
> > to memory (anonymous or special memory filesystem).
> >
> > If you have an mmapped memory segment with a regular device backed
> > file then you already have one file associated with a memory segment
> > and a filesystem that does take care of synchronizing the contents of
> > the memory segment to a backing device.
> >
> > If you now perform RDMA or device I/O on such a memory segment then
> > you will have *two* different devices interacting with that memory
> > segment. I think that ought not to happen and not be supported out of
> > the box. It will be difficult to handle and the semantics will be hard
> > for users to understand.
> >
> > What could happen is that the filesystem could agree on request to
> > allow third party I/O to go to such a memory segment. But that needs
> > to be well defined and clearly and explicitly handled by some
> > mechanism in user space that has well defined semantics for data
> > integrity for the filesystem as well as the RDMA or device I/O.
> >
> 
> Those discussions are underway. Dave Chinner and others have been talking
> about filesystem leases, for example. The key point here is that we'll still
> need, in any of these approaches, to be able to identify the gup-pinned pages.
> And there are lots (100+) of call sites to change. So I figure we'd better get
> that started.
>

+ 1

I'm exploring patch sets like this.  Having this interface available will, IMO, allow for better review of those patches rather than saying "go over to Johns tree to get the pre-requisite patches".  :-D

Also I think it will be easier for users to get things right by calling [get|put]_user_pages() rather than get_user_pages() followed by put_page().

Ira

> thanks,
> --
> John Hubbard
> NVIDIA