Re: Explanation for dropping write-back in mmap

From: Shawn O. Pearce <spearce@spearce.org>
Date: Jun 7, 2010, 15:50 UTC
Message-ID: <20100607155039.GG14847@spearce.org>
In-Reply-To: <AANLkTilrinkNo9awJ9Wso4tXzQKghWEd2bfDRhPHPOga@mail.gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 20 quoted lines

> 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>
> -----------------------------------------------------------------------------------------------------------

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.

-- 
Shawn.