{"thread":{"id":"57860","subject":"Re: [PATCH] Prevent git from rehashing 4GBi files","startedAt":"2022-05-07T02:15:25Z","lastAt":"2022-05-11T22:24:15Z","messageCount":4,"participants":["Jason Hatton","René Scharfe","Philip Oakley"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"454960","messageId":"CY4PR16MB1655F83010A128D4ED67C7EDAFC49@CY4PR16MB1655.namprd16.prod.outlook.com","threadId":"57860","inReplyTo":null,"subject":"Re: [PATCH] Prevent git from rehashing 4GBi files","fromName":"Jason Hatton","fromEmail":"jhatton@globalfinishing.com","sentAt":"2022-05-07T02:15:00Z","receivedAt":"2022-05-07T02:15:25Z","isPatch":true,"sender":{"key":"jhatton@globalfinishing.com","avatar":"https://avatars.githubusercontent.com/u/36747462?v=4"},"body":">Philip Oakley <philipoakley@iee.email> writes:\n>\n>>> This may treat non-zero multiple of 4GiB as \"not racy\", but has\n>>> anybody double checked the concern Réne brought up earlier that a\n>>> 4GiB file that was added and then got rewritten to 2GiB within the\n>>> same second would suddenly start getting treated as not racy?\n>> This is the pre-existing problem, that ~1in 2^31 size changes might not\n>> get noticed for size change. The 0 byte / 4GiB change is an identical\n>> issue, as is changing from 3 bytes to 4GiB+3 bytes, etc., so that's no\n>> worse than before (well maybe twice as 'unlikely').\n>\n>OK, it added one more case to 2^32-1 existing cases, I guess.\n>\n>>> The patch (the firnal version of it anyway) needs to be accompanied\n>>> by a handful of test additions to tickle corner cases like that.\n>> They'd be protected by the EXPENSIVE prerequisite I would assume.\n> \n> Oh, absolutely.  Thanks for spelling that out.\n\nI have been testing out the patch a bit and have good and (mostly) bad news.\n\nWhat works using a munge value of 1.\n\n$ git add\n$ git status\n\nRacy seems to work.\n\n$ touch .git/index 4GiB # 4GiB is now racy\n$ git status # Git will rehash the racy file\n$ git status # Git cached the file. Second status is fast.\n\nWhat doesn't work.\n\n$ git checkout 4GiB\n$ fatal: packed object is corrupt!\n\nUsing a munge value of 1<<31 causes even more problems. The file hash in the\nindex for 4GiB files (git ls-files -s --debug) are set to the zero file hash.\n\nI looked up and down the code base and couldn't figure out how the munged\nvalue was leaking out of read-cache.c and breaking things. Most of the code\nI found tends to use stat and then convert that to a size_t, not using the\nmunged unsigned int at all.\n\nMaybe someone else will have better luck. This seems over my head :(\n\nThanks\n--\nJason\n\n"},{"id":"454971","messageId":"0b79c442-9b2f-6de7-2424-9f13e9ef9878@web.de","threadId":"57860","inReplyTo":"1DFD3E42-3EF3-4420-8E01-748EF3DBE7A1@iee.email","subject":"Re: [PATCH] Prevent git from rehashing 4GBi files","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2022-05-07T15:22:32Z","receivedAt":"2022-05-07T15:22:49Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.05.22 um 14:33 schrieb Philip Oakley:\n>\n>\n> On 7 May 2022 03:15:00 BST, Jason Hatton <jhatton@globalfinishing.com> wrote:\n>\n>         Philip Oakley <philipoakley@iee.email> writes:\n>\n>                 This may treat non-zero multiple of 4GiB as \"not racy\", but has\n>                 anybody double checked the concern Réne brought up earlier that a\n>                 4GiB file that was added and then got rewritten to 2GiB within the\n>                 same second would suddenly start getting treated as not racy?\n>\n>             This is the pre-existing problem, that ~1in 2^31 size changes might not\n>             get noticed for size change. The 0 byte / 4GiB change is an identical\n>             issue, as is changing from 3 bytes to 4GiB+3 bytes, etc., so that's no\n>             worse than before (well maybe twice as 'unlikely').\n>\n>\n>         OK, it added one more case to 2^32-1 existing cases, I guess.\n>\n>                 The patch (the firnal version of it anyway) needs to be accompanied\n>                 by a handful of test additions to tickle corner cases like that.\n>\n>             They'd be protected by the EXPENSIVE prerequisite I would assume.\n>\n>\n>         Oh, absolutely. Thanks for spelling that out.\n>\n>\n>     I have been testing out the patch a bit and have good and (mostly) bad news.\n>\n>     What works using a munge value of 1.\n>\n>     $ git add\n>     $ git status\n>\n>     Racy seems to work.\n>\n>     $ touch .git/index 4GiB # 4GiB is now racy\n>     $ git status # Git will rehash the racy file\n>     $ git status # Git cached the file. Second status is fast.\n>\n>     What doesn't work.\n>\n>     $ git checkout 4GiB\n>     $ fatal: packed object is corrupt!\n>\n>     Using a munge value of 1<<31 causes even more problems. The file hash in the\n>     index for 4GiB files (git ls-files -s --debug) are set to the zero file hash.\n>\n>     I looked up and down the code base and couldn't figure out how the munged\n>     value was leaking out of read-cache.c and breaking things. Most of the code\n>     I found tends to use stat and then convert that to a size_t, not using the\n>     munged unsigned int at all.\n>\n>     Maybe someone else will have better luck. This seems over my head :(\n>\n>     Thanks\n>     --\n>     Jason\n>\n>\n> Is this on Git for Windows or a 64 bit Linux?\n> There are still some issues on GfW for 2GiB+ files (long Vs long long int).\n\nWhich would explain the zero file hash.  And make the platform unfit for\nhandling big files at all at this time.\n\nFWIW, on MacOS I get this with the patch applied:\n\n   $ git init --quiet /tmp/a\n   $ cd /tmp/a\n   $ : >size-0\n   $ dd if=/dev/zero bs=1 oseek=4294967295 count=1 of=size-4294967296\n   1+0 records in\n   1+0 records out\n   1 bytes transferred in 0.000365 secs (2740 bytes/sec)\n   $ dd if=/dev/zero bs=1 oseek=4294967296 count=1 of=size-4294967297\n   1+0 records in\n   1+0 records out\n   1 bytes transferred in 0.000293 secs (3413 bytes/sec)\n   $ dd if=/dev/zero bs=1 oseek=6442450943 count=1 of=size-6442450944\n   1+0 records in\n   1+0 records out\n   1 bytes transferred in 0.000266 secs (3759 bytes/sec)\n   $ git add size-*\n   $ git commit -m initial\n   [master (root-commit) d9c2a0a] initial\n    4 files changed, 0 insertions(+), 0 deletions(-)\n    create mode 100644 size-0\n    create mode 100644 size-4294967296\n    create mode 100644 size-4294967297\n    create mode 100644 size-6442450944\n\n   $ time git checkout size-*\n   Updated 0 paths from the index\n   git checkout size-*  0.01s user 0.01s system 65% cpu 0.020 total\n\n   $ git ls-files -s --debug | grep size\n   100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0\tsize-0\n     size: 0\tflags: 0\n   100644 451971a31ea5a207a10b391df2d5949910133565 0\tsize-4294967296\n     size: 2147483648\tflags: 0\n   100644 3eb7feb1413c757f0d8181deb28d1dab03d64846 0\tsize-4294967297\n     size: 1\tflags: 0\n   100644 741285bddfa7863072c238f34e27144c2501832d 0\tsize-6442450944\n     size: 2147483648\tflags: 0\n\nSo checkout skips all of the files and their cached sizes have the\nexpected values.\n\nRené\n"},{"id":"455126","messageId":"1a56b96c-2c58-ccaf-11ae-5e8264a323b1@iee.email","threadId":"57860","inReplyTo":"CY4PR16MB1655F83010A128D4ED67C7EDAFC49@CY4PR16MB1655.namprd16.prod.outlook.com","subject":"Re: [PATCH] Prevent git from rehashing 4GBi files","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-10T22:45:14Z","receivedAt":"2022-05-10T22:45:20Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 07/05/2022 03:15, Jason Hatton wrote:\n>> Philip Oakley <philipoakley@iee.email> writes:\n>>\n>>>> This may treat non-zero multiple of 4GiB as \"not racy\", but has\n>>>> anybody double checked the concern Réne brought up earlier that a\n>>>> 4GiB file that was added and then got rewritten to 2GiB within the\n>>>> same second would suddenly start getting treated as not racy?\n>>> This is the pre-existing problem, that ~1in 2^31 size changes might not\n>>> get noticed for size change. The 0 byte / 4GiB change is an identical\n>>> issue, as is changing from 3 bytes to 4GiB+3 bytes, etc., so that's no\n>>> worse than before (well maybe twice as 'unlikely').\n>> OK, it added one more case to 2^32-1 existing cases, I guess.\n>>\n>>>> The patch (the firnal version of it anyway) needs to be accompanied\n>>>> by a handful of test additions to tickle corner cases like that.\n>>> They'd be protected by the EXPENSIVE prerequisite I would assume.\n>> Oh, absolutely.  Thanks for spelling that out.\n> I have been testing out the patch a bit and have good and (mostly) bad news.\n>\n> What works using a munge value of 1.\n>\n> $ git add\n> $ git status\n>\n> Racy seems to work.\n>\n> $ touch .git/index 4GiB # 4GiB is now racy\n> $ git status # Git will rehash the racy file\n> $ git status # Git cached the file. Second status is fast.\n>\n> What doesn't work.\n>\n> $ git checkout 4GiB\n> $ fatal: packed object is corrupt!\n>\n> Using a munge value of 1<<31 causes even more problems. The file hash in the\n> index for 4GiB files (git ls-files -s --debug) are set to the zero file hash.\n>\n> I looked up and down the code base and couldn't figure out how the munged\n> value was leaking out of read-cache.c and breaking things. Most of the code\n> I found tends to use stat and then convert that to a size_t, not using the\n> munged unsigned int at all.\n>\n> Maybe someone else will have better luck. This seems over my head :(\n>\n> Thanks\n> --\n> Jason\n>\nIs there a problem that 1<<31, when on a 32bit long is MAX_NEG, rather \nthan being MAX_POS? And the size would need to be positive to be an \nacceptable file size?\n(The code is a bit of a mish-mash on the Windows LLP64 side, where long \nis only 32 bits).\n\nPhilip\nApologies for the terseness.\n"},{"id":"455181","messageId":"972cb306-04ce-133d-9d09-5da40afd675f@iee.email","threadId":"57860","inReplyTo":"1a56b96c-2c58-ccaf-11ae-5e8264a323b1@iee.email","subject":"Re: [PATCH] Prevent git from rehashing 4GBi files","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-05-11T22:24:07Z","receivedAt":"2022-05-11T22:24:15Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 11/05/2022 18:47, Jason D. Hatton wrote:\n>> Is there a problem that 1<<31, when on a 32bit long is MAX_NEG, \n>> rather than being MAX_POS? And the size would need to be positive to \n>> be an acceptable file size?\n>> (The code is a bit of a mish-mash on the Windows LLP64 side, where \n>> long is only 32 bits).\n>>\n>> Philip\n>> Apologies for the terseness.\n>\n> Philip\n>\n> I made a little test script and tried out several different\n> things.\n>\n> tldr; It didn't make any difference.\n>\n> Files tested:\n> 1, 2 and 4 GiB with and without LFS. Tested with 0, 1, 1<<30,\n> and 1<<31 mung builds. I'm only listing the problems unless\n> stated otherwise. The mung didn't appear to introduce any\n> new issues with my limited tests.\n>\n> git 2.36.0.windows.1 release:    fails on 4GiB w/o LFS - corrupts pack \n> file\n>    git status is very slow.\n>    Sometimes stores zero file instead of corruption.\n>\n> git 2.36.0.windows.1 custom compile w/o patches:\n>    fails on 4GiB w/o LFS - stores zero file\n>    git status is very slow.\n>\n> git 2.36.0.windows.1 with 1U<<31 mung:\n>    fails on 4GiB w/o LFS - stores zero file\n>\n> git 2.36.0.windows.1 with 1U<<30 mung:\n>    fails on 4GiB w/o LFS - stores zero file\n>\n> git 2.36.0.windows.1 with 1 mung:\n>    fails on 4GiB w/o LFS - stores zero file\n>\n> git 2.36.0 Ubuntu\n>    unpatched works, but has the slow status issue.\n>\n> The test script I used is below:\n\nWithout the git-lfs (to grossly shorten the file size in the pack) I \nwasn't expecting much, given the use of 'long' in places in the code \nbase for the file sizes, so 2GiB and 4GiB files would likely fail on the \nWindows LP32 parts.\n\nI was under the impression that the core code for packs had been size_t \nhardened, but there may be some paths either in git-lfs or the actual \nfile checkout that cause that fail.\n\nThere was a previous series by Matt Cooper on:\n  \"Allow clean/smudge filters to handle huge files in the LLP64 data model\"\n(https://lore.kernel.org/git/pull.1068.git.1635320952.gitgitgadget@gmail.com/t/#u)\n  Merge commit f9ba6acaa9348ea7b733bf78adc2f084247a912f\n'mc/clean-smudge-with-llp64'\n\nThat series had some in-code checks, and some test-suite tests, though \nthe latter classed as EXPENSIVE (i.e. not normally run), which may add \nmore insight.\n\n>\n>\n> #!/bin/sh\n>\n> GB1=$((1 * 1024*1024*1024))\n> GB2=$((2 * 1024*1024*1024))\n> GB4=$((4 * 1024*1024*1024))\n>\n> die()\n> {\n>    echo \"$1\"\n>    exit 1\n> }\n>\n> test_file()\n> {\n>    echo \"=== TESTING $2 ===\"\n>    rm -rf .git .gitattributes .gitignore .gitmodules &&\n>        git init &&\n>        git lfs track '*.big' &&\n>        truncate --size \"$1\" \"$2\" &&\n>        git add \"$2\" &&\n>        git commit -m \"$2\" &&\n>        git fsck &&\n>        mv \"$2\" bak &&\n>        git restore \"$2\" &&\n>        cmp \"$2\" bak || die \"$2\"\n>    git status && timeout 5 git status || die \"$2 git status slow\"\n>    rm -rf .git .gitattributes .gitignore .gitmodules \"$2\" bak\n> }\n>\n> test_file \"$GB1\" gb1.big\n> test_file \"$GB2\" gb2.big\n> test_file \"$GB4\" gb4.big\n> test_file \"$GB1\" gb1\n> test_file \"$GB2\" gb2\n> test_file \"$GB4\" gb4\n> echo done \n--\nPhilip\n"}]}