git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Brandon Casey <drafnel@gmail.com>
Cc: Brandon Casey <bcasey@nvidia.com>,
	"git\@vger.kernel.org" <git@vger.kernel.org>,
	Jeff King <peff@peff.net>, Shawn Pearce <spearce@spearce.org>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH v2 1/2] sha1_file: introduce close_one_pack() to close packs on fd pressure
Date: Thu, 01 Aug 2013 13:02:26 -0700	[thread overview]
Message-ID: <7vy58lqiwd.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <CA+sFfMer+5bhqxF=_zAQhZ+8sQD6EAYb8HBtYpuhQY_0uj-m9A@mail.gmail.com> (Brandon Casey's message of "Thu, 1 Aug 2013 12:16:57 -0700")

Brandon Casey <drafnel@gmail.com> writes:

> On Thu, Aug 1, 2013 at 11:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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.

OK, so if there were such an OS limit, the unuse_one_window() will
hopefully reduce the number of open windows and as a side effect we
may go below that limit.

What I was worried about was if there was a limit on the number of
files we have windows into (i.e. having one window each in N files,
with fds all closed, somehow costs more than having N window in one
file with the fd closed).  We currently have knobs for total number
of windows and number of open fds consumed for packs, and the latter
indirectly controls the number of active packfiles we have windows
into.  Your proposed change will essentially make the number of
active packfiles unlimited by any of our knobs, and that was where
my uneasiness was coming from.

  parent reply	other threads:[~2013-08-01 20:02 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-30  4:05 [PATCH] sha1_file: introduce close_one_pack() to close packs on fd pressure Brandon Casey
2013-07-30  7:51 ` Eric Sunshine
2013-07-30 15:39 ` Junio C Hamano
2013-07-30 19:52   ` Jeff King
2013-07-30 22:59     ` Brandon Casey
2013-07-31 19:51       ` [PATCH v2 1/2] " Brandon Casey
2013-07-31 19:51         ` [PATCH 2/2] Don't close pack fd when free'ing pack windows Brandon Casey
2013-07-31 21:08           ` Antoine Pelisse
2013-07-31 21:21             ` Fredrik Gustafsson
2013-07-31 21:31               ` Brandon Casey
2013-07-31 21:44                 ` Fredrik Gustafsson
2013-07-31 21:23             ` Brandon Casey
2013-07-31 21:28             ` Thomas Rast
2013-08-01 17:12         ` [PATCH v2 1/2] sha1_file: introduce close_one_pack() to close packs on fd pressure Junio C Hamano
2013-08-01 18:01           ` Brandon Casey
2013-08-01 18:39             ` Junio C Hamano
2013-08-01 19:16               ` Brandon Casey
2013-08-01 19:23                 ` Brandon Casey
2013-08-01 20:02                 ` Junio C Hamano [this message]
2013-08-01 20:37                   ` Brandon Casey
2013-08-02  5:36                     ` [PATCH v3] " Brandon Casey
2013-08-02 16:26                       ` Junio C Hamano
2013-08-02 17:12                         ` Brandon Casey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vy58lqiwd.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=bcasey@nvidia.com \
    --cc=drafnel@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=spearce@spearce.org \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).