Skip to content

Commit

Permalink
Don't close pack fd when free'ing pack windows
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Brandon Casey authored and Junio C Hamano committed Aug 2, 2013
1 parent 88d0db5 commit 7c3ecb3
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 16 deletions.
2 changes: 1 addition & 1 deletion builtin/pack-objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -1809,7 +1809,7 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
static void try_to_free_from_threads(size_t size)
{
read_lock();
release_pack_memory(size, -1);
release_pack_memory(size);
read_unlock();
}

Expand Down
2 changes: 1 addition & 1 deletion git-compat-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ int inet_pton(int af, const char *src, void *dst);
const char *inet_ntop(int af, const void *src, char *dst, size_t size);
#endif

extern void release_pack_memory(size_t, int);
extern void release_pack_memory(size_t);

typedef void (*try_to_free_t)(size_t);
extern try_to_free_t set_try_to_free_routine(try_to_free_t);
Expand Down
21 changes: 7 additions & 14 deletions sha1_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ static void scan_windows(struct packed_git *p,
}
}

static int unuse_one_window(struct packed_git *current, int keep_fd)
static int unuse_one_window(struct packed_git *current)
{
struct packed_git *p, *lru_p = NULL;
struct pack_window *lru_w = NULL, *lru_l = NULL;
Expand All @@ -619,26 +619,19 @@ static int unuse_one_window(struct packed_git *current, int keep_fd)
pack_mapped -= lru_w->len;
if (lru_l)
lru_l->next = lru_w->next;
else {
else
lru_p->windows = lru_w->next;
if (!lru_p->windows && lru_p->pack_fd != -1
&& lru_p->pack_fd != keep_fd) {
close(lru_p->pack_fd);
pack_open_fds--;
lru_p->pack_fd = -1;
}
}
free(lru_w);
pack_open_windows--;
return 1;
}
return 0;
}

void release_pack_memory(size_t need, int fd)
void release_pack_memory(size_t need)
{
size_t cur = pack_mapped;
while (need >= (cur - pack_mapped) && unuse_one_window(NULL, fd))
while (need >= (cur - pack_mapped) && unuse_one_window(NULL))
; /* nothing */
}

Expand All @@ -649,7 +642,7 @@ void *xmmap(void *start, size_t length,
if (ret == MAP_FAILED) {
if (!length)
return NULL;
release_pack_memory(length, fd);
release_pack_memory(length);
ret = mmap(start, length, prot, flags, fd, offset);
if (ret == MAP_FAILED)
die_errno("Out of memory? mmap failed");
Expand Down Expand Up @@ -961,7 +954,7 @@ unsigned char *use_pack(struct packed_git *p,
win->len = (size_t)len;
pack_mapped += win->len;
while (packed_git_limit < pack_mapped
&& unuse_one_window(p, p->pack_fd))
&& unuse_one_window(p))
; /* nothing */
win->base = xmmap(NULL, win->len,
PROT_READ, MAP_PRIVATE,
Expand Down Expand Up @@ -1007,7 +1000,7 @@ static struct packed_git *alloc_packed_git(int extra)

static void try_to_free_pack_memory(size_t size)
{
release_pack_memory(size, -1);
release_pack_memory(size);
}

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

0 comments on commit 7c3ecb3

Please sign in to comment.