Re: [PATCH/RFC v3 06/13] Read index-v5

> uOn Wed, Aug 8, 2012 at 6:17 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> > +static struct cache_entry *read_entry(struct directory_entry *de,
> > +                       unsigned long *entry_offset,
> > +                       void **mmap,
> > +                       unsigned long mmap_size,
> > +                       unsigned int *foffsetblock,
> > +                       int fd)
> > +{
> > ....
> > +               if (crc_wrong) {
> > +                       /* wait for 10 milliseconds */
> > +                       usleep(10*1000);
> > +                       munmap(*mmap, mmap_size);
> > +                       *mmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> > +               }
> 
> Do we really need to munmap and mmap again? I don't see mmap man page
> mention anything about "refreshing" the mmap'd memory with file
> changes, not sure how it works. msync() seems for writing only.
> 
> If remapping is necessary, how about mremap? What I want to see is
> whether we could avoid passing "fd" down to here.
> 
> > +struct index_ops v5_ops = {
> > +       match_stat_basic,
> > +       verify_hdr,
> > +       read_index_v5,
> > +       NULL
> > +};
> 
> If you do it right, putting write_index_v2 here should work because
> in-core structure is not changed (except that write_index_v2 is static
> function, well..). Maybe putting write_index to this struct is a wrong
> decision. We should be able to read_index_v5+write_index_v2 and
> read_index_v2+write_index_v5.