@matheusbsbs / matheus-bagliano / commits / 7c3ecb3254

Don't close pack fd when free'ing pack windows

Now that close_one_pack() has been introduced to handle file descriptor pressure, it is not strictly necessary to close the pack file descriptor in unuse_one_window() when we're under memory pressure. Jeff King provided a justification for leaving the pack file open: If you close packfile descriptors, you can run into racy situations where somebody else is repacking and deleting packs, and they go away while you are trying to access them. If you keep a descriptor open, you're fine; they last to the end of the process. If you don't, then they disappear from under you. For normal object access, this isn't that big a deal; we just rescan the packs and retry. But if you are packing yourself (e.g., because you are a pack-objects started by upload-pack for a clone or fetch), it's much harder to recover (and we print some warnings). Let's do so (or uh, not do so). Signed-off-by: Brandon Casey <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Casey committed Jul 31, 2013 at 12:51 UTC 7c3ecb32541d767518bf78633603f94fefdac8cf

3 files changed +9 -16

builtin/pack-objects.c

+1 -1

@@ -1809,7 +1809,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,

1809 static void try_to_free_from_threads(size_t size)

1810 {

1811 read_lock();

1812 - release_pack_memory(size, -1);

1812 + release_pack_memory(size);

1813 read_unlock();

1814 }

1815

git-compat-util.h

+1 -1

@@ -498,7 +498,7 @@ int inet_pton(int af, const char *src, void *dst);

498 const char *inet_ntop(int af, const void *src, char *dst, size_t size);

499 #endif

500

501 -extern void release_pack_memory(size_t, int);

501 +extern void release_pack_memory(size_t);

502

503 typedef void (*try_to_free_t)(size_t);

504 extern try_to_free_t set_try_to_free_routine(try_to_free_t);

sha1_file.c

+7 -14

@@ -605,7 +605,7 @@ static void scan_windows(struct packed_git *p,

605 }

606 }

607

608 -static int unuse_one_window(struct packed_git *current, int keep_fd)

608 +static int unuse_one_window(struct packed_git *current)

609 {

610 struct packed_git *p, *lru_p = NULL;

611 struct pack_window *lru_w = NULL, *lru_l = NULL;

@@ -619,15 +619,8 @@ static int unuse_one_window(struct packed_git *current, int keep_fd)

619 pack_mapped -= lru_w->len;

620 if (lru_l)

621 lru_l->next = lru_w->next;

622 - else {

622 + else

623 lru_p->windows = lru_w->next;

624 - if (!lru_p->windows && lru_p->pack_fd != -1

625 - && lru_p->pack_fd != keep_fd) {

626 - close(lru_p->pack_fd);

627 - pack_open_fds--;

628 - lru_p->pack_fd = -1;

629 - }

630 - }

624 free(lru_w);

625 pack_open_windows--;

626 return 1;

@@ -635,10 +628,10 @@ static int unuse_one_window(struct packed_git *current, int keep_fd)

628 return 0;

629 }

630

638 -void release_pack_memory(size_t need, int fd)

631 +void release_pack_memory(size_t need)

632 {

633 size_t cur = pack_mapped;

641 - while (need >= (cur - pack_mapped) && unuse_one_window(NULL, fd))

634 + while (need >= (cur - pack_mapped) && unuse_one_window(NULL))

635 ; /* nothing */

636 }

637

@@ -649,7 +642,7 @@ void *xmmap(void *start, size_t length,

642 if (ret == MAP_FAILED) {

643 if (!length)

644 return NULL;

652 - release_pack_memory(length, fd);

645 + release_pack_memory(length);

646 ret = mmap(start, length, prot, flags, fd, offset);

647 if (ret == MAP_FAILED)

648 die_errno("Out of memory? mmap failed");

@@ -961,7 +954,7 @@ unsigned char *use_pack(struct packed_git *p,

954 win->len = (size_t)len;

955 pack_mapped += win->len;

956 while (packed_git_limit < pack_mapped

964 - && unuse_one_window(p, p->pack_fd))

957 + && unuse_one_window(p))

958 ; /* nothing */

959 win->base = xmmap(NULL, win->len,

960 PROT_READ, MAP_PRIVATE,

@@ -1007,7 +1000,7 @@ static struct packed_git *alloc_packed_git(int extra)

1000

1001 static void try_to_free_pack_memory(size_t size)

1002 {

1010 - release_pack_memory(size, -1);

1003 + release_pack_memory(size);

1004 }

1005

1006 struct packed_git *add_packed_git(const char *path, int path_len, int local)