# Explanation for dropping write-back in mmap 5 messages from 2010-06-07 to 2010-06-07. Participants: Ramkumar Ramachandra, Shawn O. Pearce, Junio C Hamano, David Michael Barr. Thread: https://gitlist.dev/t/24032 ## Ramkumar Ramachandra, 2010-06-07 15:16 Subject: Explanation for dropping write-back in mmap Message-ID: URL: https://gitlist.dev/e/AANLkTilrinkNo9awJ9Wso4tXzQKghWEd2bfDRhPHPOga%40mail.gmail.com ``` 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 ----------------------------------------------------------------------------------------------------------- commit f48000fcbe1009c18f1cc46e56cde2cb632071fa Author: Junio C Hamano 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 ----------------------------------------------------------------------------------------------------------- Thanks. -- Ram ``` ## Shawn O. Pearce, 2010-06-07 15:50 Subject: Re: Explanation for dropping write-back in mmap Message-ID: <20100607155039.GG14847@spearce.org> URL: https://gitlist.dev/e/20100607155039.GG14847%40spearce.org In-Reply-To: ``` Ramkumar Ramachandra wrote: > 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 > > ----------------------------------------------------------------------------------------------------------- > commit f48000fcbe1009c18f1cc46e56cde2cb632071fa > Author: Junio C Hamano > 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 > ----------------------------------------------------------------------------------------------------------- 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. ``` ## Junio C Hamano, 2010-06-07 15:55 Subject: Re: Explanation for dropping write-back in mmap Message-ID: <7vbpbmx2er.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vbpbmx2er.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Ramkumar Ramachandra writes: > In the commit f48000fcbe100, you've forced the caller to use > MAP_PRIVATE in the alternative mmap implementation dropping write-back > support. > ... > commit f48000fcbe1009c18f1cc46e56cde2cb632071fa > Author: Junio C Hamano > 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. 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. ``` ## Ramkumar Ramachandra, 2010-06-07 19:01 Subject: Re: Explanation for dropping write-back in mmap Message-ID: URL: https://gitlist.dev/e/AANLkTinJE5blkIRPp5NDKEWVm2uv-Yelt_niYko6XZa_%40mail.gmail.com In-Reply-To: <7vbpbmx2er.fsf@alter.siamese.dyndns.org> ``` Hi Junio and Shawn, Shawn O. Pearce wrote: > I would strongly suggest finding another way to implement the SVN > exporter, without using MAP_SHARED. 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: > 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. Ah, so you did it for safety/ sanity reasons back then. > 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. Right. Thanks for this- I read the "First cut at git port to Cygwin" thread and learnt the backstory. -- Ram ``` ## David Michael Barr, 2010-06-07 23:13 Subject: Re: Explanation for dropping write-back in mmap Message-ID: URL: https://gitlist.dev/e/AD1AE06F-0986-46C4-BCEF-A496EB5CEB98%40cordelta.com In-Reply-To: ``` Hi Ram, > Shawn O. Pearce wrote: >> I would strongly suggest finding another way to implement the SVN >> exporter, without using MAP_SHARED. > > 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? 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. > Junio C Hamano wrote: >> 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. > > Ah, so you did it for safety/ sanity reasons back then. Having not read the backstory, this was my guess as to the rationale. -- David Barr. ```