On 08/08, Nguyen Thai Ngoc Duy wrote:
> uOn Wed, Aug 8, 2012 at 6:17 PM, Thomas Gummerer <t.gumme...@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.

Hrm I don't think it should be a problem putting write_index_v2 here,
but the way I currently handle it is by replacing the whole index_ops.
I think it's a good thing that the index_ops version always matches
the index version, to avoid any problems there.  The only place where
this can happen in the current code is in update-index.c when we change
the index version.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to