git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "R. Tyler Ballance" <tyler@slide.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Nicolas Pitre" <nico@cam.org>, "Jan Krüger" <jk@jk.gs>,
	"Git ML" <git@vger.kernel.org>
Subject: Re: [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file
Date: Wed, 07 Jan 2009 16:49:40 -0800	[thread overview]
Message-ID: <1231375780.8870.629.camel@starfruit> (raw)
In-Reply-To: <alpine.LFD.2.00.0901071621340.3283@localhost.localdomain>

[-- Attachment #1: Type: text/plain, Size: 2976 bytes --]

On Wed, 2009-01-07 at 16:37 -0800, Linus Torvalds wrote:
> 
> On Wed, 7 Jan 2009, Linus Torvalds wrote:
> >
> > >         > limit ~1.5GB -> corrupt file
> > >         > limit ~3GB -> magically no longer corrupt.
> > 
> > That is interesting, although I also worry that there might be other 
> > issues going on (ie since you've reported thigns magically fixing 
> > themselves, maybe the ulimit tests just _happened_ to show that, even if 
> > it wasn't the core reason).
> > 
> > BUT! This is definitely worth looking at.
> > 
> > For example, we do have some cases where we try to do "mmap()", and if it 
> > fails, we try to free some memory and try again. In particular, in 
> > xmmap(), if an mmap() fails - which may be due to running out of virtual 
> > address space - we'll actually try to release some pack-file memory and 
> > try again. Maybe there's a bug there - and it would be one that seldom 
> > triggers for others.
> 
> Ho humm. We really do have some interesting things there. 

Always enjoyable when these mail threads get this deep ;)

> 
> Is this a 64-bit machine? I didn't think OS X did that, but if there is 
> some limited 64-bit support there, maybe "sizeof(void *)" is 8, then we 
> default the default git pack-window to a pretty healthy 1GB.

I was only mentioning OS X with regards to the Samba/NFS red herring,
the rest of our operations are on 64-bit Linux machines.

The machine I reproduced this on ("Public repo case!") is the following:
        tyler@grapefruit:~> uname -a
        Linux grapefruit.corp.slide.com 2.6.27.7-9-default #1 SMP
        2008-12-04 18:10:04 +0100 x86_64 x86_64 x86_64 GNU/Linux
        tyler@grapefruit:~> cat /etc/issue
        Welcome to openSUSE 11.1   - Kernel \r (\l).
        
The machines we're experiencing this issue on "in the wild" are:
        xdev3 (master)% uname -a 
        Linux xdev3 2.6.24-22-server #1 SMP Mon Nov 24 20:06:28 UTC 2008
        x86_64 GNU/Linux
        xdev3 (master)% cat /etc/issue
        Ubuntu 8.04.1 \n \l
> 
> I could easily see that if you have a virtual memory size limit of 1.5GB, 
> and the pack window size is 1GB, we might have trouble. Because we could 
> only keep one such pack window in memory at a time.

The DEFAULT_PACKED_GIT_WINDOW_SIZE in our local builds is 256M, FWIW

> 
> I have _not_ looked at the code, though. I'd have expected a SIGSEGV if we 
> really had issues with the window handling.
> 
> Anyway, _if_ your system has 64-bit pointers, then _maybe_ something the 
> default 1GB pack window causes problem.
> 
> If so, then adding a
> 
> 	[core]
> 		packedgitwindowsize = 64M
> 
> might make a difference. It would certainly be very interesting to hear if 
> there's any impact.

I can try this still if you'd like, but it doesn't seem like that'd be
the issue since we're already lowering the window size system-wide



Cheers
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

  reply	other threads:[~2009-01-08  0:51 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-09  8:36 [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file Jan Krüger
2008-12-09  9:02 ` R. Tyler Ballance
2008-12-09 16:24 ` Shawn O. Pearce
2009-01-06 22:52 ` R. Tyler Ballance
2009-01-07  1:25   ` Nicolas Pitre
2009-01-07  1:39     ` R. Tyler Ballance
2009-01-07  2:09       ` Nicolas Pitre
2009-01-07  2:47         ` R. Tyler Ballance
2009-01-07  3:21           ` Nicolas Pitre
2009-01-07  4:54       ` Linus Torvalds
2009-01-07  7:41         ` R. Tyler Ballance
2009-01-07  8:16           ` Junio C Hamano
2009-01-07  8:32             ` R. Tyler Ballance
2009-01-07  9:42               ` Junio C Hamano
2009-01-07  9:05           ` R. Tyler Ballance
2009-01-07 15:31           ` Nicolas Pitre
2009-01-07 16:07           ` Linus Torvalds
2009-01-07 16:08             ` Linus Torvalds
2009-01-07 22:55             ` R. Tyler Ballance
2009-01-07 23:29               ` Linus Torvalds
2009-01-08  0:28                 ` Public repro case! " R. Tyler Ballance
2009-01-08  0:48                   ` Linus Torvalds
2009-01-08  0:57                     ` R. Tyler Ballance
2009-01-08  1:08                       ` Linus Torvalds
2009-01-08  1:29                         ` Linus Torvalds
2009-01-08  1:46                           ` Shawn O. Pearce
2009-01-08  2:21                     ` James Pickens
2009-01-08  2:43                       ` Shawn O. Pearce
2009-01-08  5:40                         ` Junio C Hamano
2009-01-08  6:04                           ` Shawn O. Pearce
2009-01-08  2:52                       ` Boyd Stephen Smith Jr.
2009-01-08  2:52                   ` Linus Torvalds
2009-01-08  3:01                     ` Shawn O. Pearce
2009-01-08  3:06                       ` Linus Torvalds
2009-01-08  3:13                         ` Shawn O. Pearce
2009-01-08  3:16                           ` [PATCH] Wrap inflateInit to retry allocation after releasing pack memory Shawn O. Pearce
2009-01-08  3:54                             ` Linus Torvalds
2009-01-08  5:23                               ` Junio C Hamano
2009-01-08 15:35                                 ` Linus Torvalds
2009-01-08 15:34                               ` Shawn O. Pearce
2009-01-08 16:14                                 ` Linus Torvalds
2009-01-08 18:15                               ` R. Tyler Ballance
2009-01-08 20:22                                 ` Linus Torvalds
2009-01-08 20:37                                   ` R. Tyler Ballance
2009-01-09  1:43                                   ` Junio C Hamano
2009-01-08  0:37                 ` [PATCH/RFC] Allow writing loose objects that are corrupted in a pack file Linus Torvalds
2009-01-08  0:49                   ` R. Tyler Ballance [this message]
2009-01-08  1:01                     ` Linus Torvalds
2009-01-08  1:06                       ` R. Tyler Ballance

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

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

  git send-email \
    --in-reply-to=1231375780.8870.629.camel@starfruit \
    --to=tyler@slide.com \
    --cc=git@vger.kernel.org \
    --cc=jk@jk.gs \
    --cc=nico@cam.org \
    --cc=torvalds@linux-foundation.org \
    /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.
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 NNTP newsgroup(s).