On Sat, Sep 06, 2014 at 09:50:14AM +0200, Michael Haggerty wrote: > Sorry for the long delay since v3. This version mostly cleans up a > couple more places where the lockfile object was left in an > ill-defined state. Thanks to Johannes Sixt and Torsten Bögershausen > for their review of v3. > > I believe that this series addresses all of the comments from v1 [1], > v2 [2], and v3 [3]. This looks pretty good to me overall. I did coincidentally have an interesting experience with our lockfile code earlier today, which I'd like to relate. I was running pack-refs on a repository with a very large number of loose refs (about 1.8 million). Needless to say, this ran very slowly and thrashed the disk, as that's almost 7G using 4K inodes. But it did eventually generate a packed-refs file, at which point it tried to prune the loose refs. To do so, we have to lock each ref before removing it (to protect against a simultaneous update). Each call to lock_ref_sha1_basic allocates a "struct lock_file", which then gets added to the global lock_file list. Each one contains a fixed PATH_MAX buffer (4K on this machine). After we're done updating the ref, we leak the lock_file struct, since there's no way to remove it from the list. As a result, git tried to allocate 7G of RAM and got OOM-killed (the machine had only 8G). In addition to thrashing the disk even harder, since there was no room left for disk cache while we touched millions of loose refs. :) Your change in this series to use a strbuf would make this a lot better. But I still wonder how hard it would be to just remove lock_file structs from the global list when they are committed or rolled back. That would presumably also make the "avoid transitory valid states" patch from your series a bit easier, too (you could prepare the lockfile in peace, and then link it in fully formed, and do the opposite when removing it). I think with your strbuf patch, this leak at least becomes reasonable. So maybe it's not worth going further. But I'd be interested to hear your thoughts since you've been touching the area recently. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html