Explanation for dropping write-back in mmap

Hi Junio,

In the commit f48000fcbe100, you've forced the caller to use MAP_PRIVATE in the alternative mmap implementation dropping write-back support. Could you kindly explain the rationale for this? David's SVN exporter uses mmap with MAP_SHARED, and we figured that the merge immediately breaks this functionality.

Noticed-by: Jonathan Nieder <jrnieder@gmail.com>

-----------------------------------------------------------------------------------------------------------
commit f48000fcbe1009c18f1cc46e56cde2cb632071fa
Author: Junio C Hamano <junkio@cox.net>
Date:   Sat Oct 8 15:54:36 2005 -0700

    Yank writing-back support from gitfakemmap.

    We do not write through our use of mmap(), so make sure callers pass
    MAP_PRIVATE and remove support for writing changes back.

    Signed-off-by: Junio C Hamano <junkio@cox.net>
-----------------------------------------------------------------------------------------------------------

Thanks.

-- Ram

Ramkumar Ramachandra <artagnon@gmail.com> wrote:

It got dropped because we never really used it. In almost every location we were only mapping a file for reading, but were writing through normal IO write functions. The Windows emulation code at that time was using malloc()+read() to emulate mmap(), and thus any modifications made to the buffer would not be flushed back.

The one place where we were doing both was fast-import, but it was playing loose with the mmap consistency rules. I think we have fixed that code since then to ensure the data stays consistent on all platforms.

I would strongly suggest finding another way to implement the SVN exporter, without using MAP_SHARED.

Ramkumar Ramachandra <artagnon@gmail.com> writes:

That's a 

I don't think we _dropped_ a _working_ support that allowed shared mapping. IIRC the implementation emulated only private mapping well enough to support the use of mmap() in our codebase (iow, instead of allocating a buffer and reading into it and possibly mucking with it without affecting outside world, map it to read and then possibly mucking with it), but lacked input validation to make sure that no caller mistakenly thinks the implementation could satisfy non private mapping.

Also I don't think I did this without telling other people---it would be a lot more likely that somebody else noticed it and the issue was discussed on the list and resulted in this commit. I would check the commit date and see the discussion around that time if I were you to learn the backstory.

Hi Junio and Shawn,

Shawn O. Pearce wrote:

David has already started working on using realloc + persist, but I'd like to know the reason for your recommendation- is it simply because mmap with MAP_SHARED is difficult to port, or is there some other reason as well?

Junio C Hamano wrote:

Ah, so you did it for safety/ sanity reasons back then.

Right. Thanks for this- I read the "First cut at git port to Cygwin" thread and learnt the backstory.

-- Ram

Hi Ram,

I had a think about it, this would be a good chance to enforce append-only writes. The general strategy would be to write batches of objects for each commit. All of the data structures in use support this pattern. However, I'll need to refactor string_pool slightly to make it work cleanly.

Having not read the backstory, this was my guess as to the rationale.

-- David Barr.