Hi, Rob Landley wrote on Sat, Sep 07, 2013 : > On 09/03/2013 09:37:58 AM, Dominique Martinet wrote: > > - Add cache=mmap option > > - Try to keep most operations synchronous > > > > Signed-off-by: Dominique Martinet <dominique.martinet@xxxxxx> > > --- > > > > Hi, > > > > I brought up the issue of mmap being read-only with cache=none a bit > > ago, and worked on it since. > > > > Here's what this patch does: > > - Add an option cache=mmap (or just mmap, like other caches), nothing > > changes for default or other caches. > > Interesting. > > Should this become the default behavior instead of "none" for > non-readonly mounts? (What's the point of the "none" mode now that we > have writeable mmap implemented, is there any advantage?) Well, if there is no issue with the principle of flushing the map on close, it might eventually become more widely used and maybe the default. I don't think there should be any inconsistency with what the documentation says mmap should do, but this doesn't mean everything will work as expected and it's not because this passes all the tests I tried that it won't break anything (and in a very subtle way that would silently corrupt data, so definitely not something we want) This definitely needs some consideration/more reviews and, in my opinion, much more testing before pretending to get there. By the way, two more questions from my end, if anyone can help: - the mkwrite function calls "v9fs_fscache_wait_on_page_write(inode, page);". I'm afraid I don't understand why we need to wait that everything has been flushed before we make the page writable, what does it do? Check the the page doesn't need to be invalidated/re-read before the actual edits, so as not to cause corruptions around the edited parts? Does mkwrite require/imply exclusive access to the modified pages? - given that we have the vm_area_struct we are closing, for the flush, would we have any better call than "write_inode_now" on the whole inode since we ought to know which pages are concerned by the vma? (think about a file with many mmaps on different parts of the file, we don't want to flush it all everytime) Would also appreciate comments from maintainers/anyone before I repost a clean patch to push through, might as well fix things as early as possible :) Thanks, -- Dominique -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
