Subject: Re: Be more careful about updating refs

From: "Shawn O. Pearce" <[email-masked]>
Date: Thu, 17 Jan 2008 22:22:22 -0500

"Shawn O. Pearce" <[email-masked]> wrote:
> "Shawn O. Pearce" <[email-masked]> wrote:
> > "Shawn O. Pearce" <[email-masked]> wrote:
> > > Junio C Hamano <[email-masked]> wrote:
> > > > 
> > > > Especially, I am not sure if the issue only exists at the
> > > > end_packfile() boundary.  Don't we have the same issue reading
> > > > from the packfile being built, and isn't the only reason my hack
> > > > works it around is because access patterns of the testsuite
> > > > happens to not trigger it?
> > > 
> > > Yes, that's my take on it as well (see my other email).  The
> > > testsuite must just be really lucky that its not hitting the
> > > boundary condition.
> > > 
> > > I almost said gfi_unpack_entry() was immune from this bug, but
> > > I went back and read the code again and determined that it does
> > > in fact suffer from this under NO_MMAP, and we're just really
> > > damn lucky nobody has caused it.
> > 
> > I think this solves the problem.  Its based on your first patch, but
> > would replace it.  The trick here is we close the cached windows if
> > we are accessing data from the packfile we are appending into and we
> > have increased the file length.  This way we don't blow away windows
> > during high read/low write periods, like during branch cache reloads.
> 
> Junio pointed out my first attempt at this didn't update the
> memory pressure values, so we could "run out of memory" even
> if we had plenty free.
> 
> Try #2...

OK, that was crap.  Don't even try it.

I'm holding off sending anything more until I get the test suite
to actually run without telling me the bus crashed into the wall.

-- 
Shawn.