Re: [PATCH v2 1/2] sha1_file: introduce close_one_pack() to close packs on fd pressure

On Thu, Aug 1, 2013 at 11:39 AM, Junio C Hamano [off-list ref] wrote:

Brandon Casey [off-list ref] writes:

quoted

I've been looking closer at uses of p->windows everywhere, and it
seems that we always open_packed_git() before we try to create new
windows.  There doesn't seem to be any reason that we can't continue
to use the existing open windows even after closing the pack file.
...
If we don't need to close_pack_windows(), find_lru_pack() doesn't
strictly need to reject packs that have windows in use.

That makes me feel somewhat uneasy.  Yes, you can open/mmap/close
and hold onto the contents of a file still mapped in-core, and it
may not count as "open filedescriptor", but do OSes allow infinite
such mmapped regions to us?  We do keep track of number of open
windows, but is there a way for us to learn how close we are to the
limit?

Not that I know of, but xmmap() does already try to unmap existing
windows when mmap() fails, and then retries the mmap.  It calls
release_pack_memory() which calls unuse_one_window().  mmap returns
ENOMEM when either there is no available memory or if the limit of
mmap mappings has been exceeded.

So, I think we'll be ok.  It's the same situation we'd be in if there
were many large packs (but fewer than pack_max_fds) and a small
packedGitWindowSize, requiring many mmap windows.  We'd try to map an
additional segment, fail, release some unused segments, and retry.

The memory usage of all mmap segments would still be bounded by
packedGitLimit.  It's just that now, when we're only under file
descriptor pressure, we won't close the mmap windows unnecessarily
when they may be needed again.

-Brandon