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

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Aug 8, 2012, 12:05 UTC
Message-ID: <CACsJy8DZ-+0XD2HxkTFuXKXJFH66MCLLrfHtcMF-1GqBb2AmVg@mail.gmail.com>
In-Reply-To: <1344424681-31469-7-git-send-email-t.gummerer@gmail.com>

uOn Wed, Aug 8, 2012 at 6:17 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:

Show 14 quoted lines

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

Show 6 quoted lines

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

-- 
Duy