{"thread":{"id":"690","subject":"running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste","startedAt":"2005-05-22T12:28:49Z","lastAt":"2005-05-22T22:07:34Z","messageCount":8,"participants":["Thomas Glanzmann","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"3748","messageId":"20050522122849.GJ15178@cip.informatik.uni-erlangen.de","threadId":"690","inReplyTo":null,"subject":"running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste","fromName":"Thomas Glanzmann","fromEmail":"sithglan@stud.uni-erlangen.de","sentAt":"2005-05-22T12:28:49Z","receivedAt":"2005-05-22T12:28:49Z","isPatch":false,"sender":{"key":"sithglan@stud.uni-erlangen.de","avatar":null},"body":"Hello,\nI wonder why 'git-update-cache --refresh' running in the same directory\nshared via NFS ends up in reindexing the whole files when running on\ndifferent machines on a NFS share.\n\nIs there a reason for this or can it easily be fixes. I also wonder if\nthe locking which is used to lock the cache is 'nfs safe'.\n\n\tThomas\n"},{"id":"3775","messageId":"Pine.LNX.4.58.0505221205580.2307@ppc970.osdl.org","threadId":"690","inReplyTo":"20050522122849.GJ15178@cip.informatik.uni-erlangen.de","subject":"Re: running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-22T19:09:41Z","receivedAt":"2005-05-22T19:09:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 22 May 2005, Thomas Glanzmann wrote:\n>\n> I wonder why 'git-update-cache --refresh' running in the same directory\n> shared via NFS ends up in reindexing the whole files when running on\n> different machines on a NFS share.\n\nIt does?\n\nCan you check what \n\n\tls -li --time=atime\n\nshows on the different clients? Also, try \"ctime\".\n\n> Is there a reason for this or can it easily be fixes. I also wonder if\n> the locking which is used to lock the cache is 'nfs safe'.\n\nIt _should_ be safe. It does the old lockfile thing, with a \"link()\" that\nshould guarantee atomicity. No fcntl locking or similar that can have\nproblems with networked filesystems and different UNIXes.\n\n\t\tLinus\n"},{"id":"3779","messageId":"20050522192734.GB23388@cip.informatik.uni-erlangen.de","threadId":"690","inReplyTo":"Pine.LNX.4.58.0505221205580.2307@ppc970.osdl.org","subject":"Re: running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste","fromName":"Thomas Glanzmann","fromEmail":"sithglan@stud.uni-erlangen.de","sentAt":"2005-05-22T19:27:34Z","receivedAt":"2005-05-22T19:27:34Z","isPatch":false,"sender":{"key":"sithglan@stud.uni-erlangen.de","avatar":null},"body":"Hello,\n\n> It does?\n\nNot for me at the moment:\n\nfaui03  -> NFS Server (Solaris 2.9)\nfaui04a -> NFS Client (Solaris 2.9)\nfaui01  -> NFS Client (Linux 2.4.30)\n\n(faui03) [~/work/blastwave] date; time git-update-cache --refresh\nSun May 22 21:09:33 CEST 2005\n\nreal    1m6.362s\nuser    0m12.550s\nsys     0m9.200s\n\n(faui04a) [~/work/blastwave] date; time git-update-cache --refresh\nSun May 22 21:10:56 CEST 2005\n\nreal    1m20.097s\nuser    0m12.270s\nsys     0m8.930s\n\n(faui01) [~/work/blastwave] date; time git-update-cache --refresh;\nSun May 22 21:17:22 CEST 2005\n\nreal    0m30.617s\nuser    0m2.340s\nsys     0m7.970s\n\n> Can you check what \n\n> \tls -li --time=atime\n\n> shows on the different clients? Also, try \"ctime\".\n\natime is different of course different.\n\n(faui01) [~/work/blastwave] (ls -Rli --time=atime; ls -lRi --time=ctime) > ~/faui01\n(faui03) [~/work/blastwave] (ls -Rli --time=atime; ls -lRi --time=ctime) > ~/faui03\n(faui04a) [~/work/blastwave] (ls -Rli --time=atime; ls -lRi --time=ctime) > ~/faui04a\n\n(faui01) [~/work/blastwave] md5sum ~/faui0{1,3,4a}\na2c2cdb38537a54fb74613d1cf6537f0  /home/cip/adm/sithglan/faui01\n67aee985bfb7514900a0a1d2c629cec9  /home/cip/adm/sithglan/faui03\n67aee985bfb7514900a0a1d2c629cec9  /home/cip/adm/sithglan/faui04a\n(faui01) [~/work/blastwave] diff -b -u ~/faui01 ~/faui03\n--- /home/cip/adm/sithglan/faui01       2005-05-22 21:24:02.000000000 +0200\n+++ /home/cip/adm/sithglan/faui03       2005-05-22 21:23:54.000000000 +0200\n@@ -1,11 +1,11 @@\n .:\n total 15\n 5483033 -rw-r--r--  1 sithglan icipguru  391 May 22 21:14 Makefile\n-1842682 drwxr-xr-x  2 sithglan icipguru  512 May 22 21:23 packages/\n-5541351 drwxr-xr-x  2 sithglan icipguru  512 May 22 21:23 public_html/\n-5541339 drwxr-xr-x  2 sithglan icipguru  512 May 22 21:23 scripts/\n-5482949 drwxr-xr-x  2 sithglan icipguru 8704 May 22 21:23 sources/\n-5482985 drwxr-xr-x  2 sithglan icipguru 2048 May 22 21:23 specs/\n+1842682 drwxr-xr-x    2 sithglan icipguru      512 May 22 21:19 packages/\n+5541351 drwxr-xr-x    2 sithglan icipguru      512 May 22 21:19 public_html/\n+5541339 drwxr-xr-x    2 sithglan icipguru      512 May 22 21:19 scripts/\n+5482949 drwxr-xr-x    2 sithglan icipguru     8704 May 22 21:19 sources/\n+5482985 drwxr-xr-x    2 sithglan icipguru     2048 May 22 21:19 specs/\n\n ./packages:\n total 0\n\nIf you need the files:\n\nhttp://wwwcip.informatik.uni-erlangen.de/~sithglan/faui01  (58k)\nhttp://wwwcip.informatik.uni-erlangen.de/~sithglan/faui03  (61k)\nhttp://wwwcip.informatik.uni-erlangen.de/~sithglan/faui04a (61k)\n\n> It _should_ be safe. It does the old lockfile thing, with a \"link()\" that\n> should guarantee atomicity. No fcntl locking or similar that can have\n> problems with networked filesystems and different UNIXes.\n\nIs link() NFS safe? I thought only mkdir() for nfs?\n\n\tThomas\n"},{"id":"3782","messageId":"Pine.LNX.4.58.0505221332590.2307@ppc970.osdl.org","threadId":"690","inReplyTo":"20050522192734.GB23388@cip.informatik.uni-erlangen.de","subject":"Re: running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-22T20:43:55Z","receivedAt":"2005-05-22T20:43:55Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 22 May 2005, Thomas Glanzmann wrote:\n> \n> Is link() NFS safe? I thought only mkdir() for nfs?\n\nSorry, I meant \"rename\", not \"link\", and yes, it should be NFS-safe. It's \nhow all the mailers do things too, afaik.\n\nAs to your update-cache problem, it seems to be just due to NFS stat\ncaching. You generally should _not_ work on two machines at the same time,\nbut it probably does the right thing in the end.\n\nIn general, I would suggest using separate GIT repositories over sharing\nthem over NFS. As far as I'm concerned, I think NFS should work in the\nsense that you can work from different clients at _different_times_, and\nI'm certainly not going to guarantee that two different clients that work\nat the same time against the same repository will get sane results.\n\nFor example, if you do a \"git-checkout-cache -f -a\" at the same time, I \nwon't guarantee that things won't race on the working files. Don't do it.\n\n\t\tLinus\n"},{"id":"3783","messageId":"20050522212312.GC23388@cip.informatik.uni-erlangen.de","threadId":"690","inReplyTo":"Pine.LNX.4.58.0505221332590.2307@ppc970.osdl.org","subject":"[PATCH] Don't include devicenumber into INODE_CHANGED test [WAS: Re: running git-update-cache --refresh on different machines on a NFS share always ends up in a lot of io/cpu/time waste]","fromName":"Thomas Glanzmann","fromEmail":"sithglan@stud.uni-erlangen.de","sentAt":"2005-05-22T21:23:12Z","receivedAt":"2005-05-22T21:23:12Z","isPatch":true,"sender":{"key":"sithglan@stud.uni-erlangen.de","avatar":null},"body":"Hello,\n\n> Sorry, I meant \"rename\", not \"link\", and yes, it should be NFS-safe. It's \n> how all the mailers do things too, afaik.\n\nokay. I will doublecheck that and come back.\n\n> As to your update-cache problem, it seems to be just due to NFS stat\n> caching. You generally should _not_ work on two machines at the same time,\n> but it probably does the right thing in the end.\n\nI added some debugging output (see attached patch) and saw that the\nreason for the invalid thing is that the inode has changed:\n\n...\nname: pull.h 0x00000010\nname: read-cache.c 0x00000010\n...\n\n#define INODE_CHANGED   0x0010\n\nSame problem tla had. It looked at the device number. And of course the\ndevice number for NFS shares isn't the same on all machines. So I\nattached a little patch which fixes the issue for me (and others).\n\n> In general, I would suggest using separate GIT repositories over sharing\n> them over NFS. As far as I'm concerned, I think NFS should work in the\n> sense that you can work from different clients at _different_times_, and\n> I'm certainly not going to guarantee that two different clients that work\n> at the same time against the same repository will get sane results.\n\nIt is more like that I don't remember on which machine I worked last and\nworking accidently on my next free window in screen (and I have a lot of\nwindows). And getting 370 Mbyte over NFS hits my nerves. ;-)\n\n> For example, if you do a \"git-checkout-cache -f -a\" at the same time, I \n> won't guarantee that things won't race on the working files. Don't do it.\n\nI will not do that. And I will add locking for such operations in my frontend\nanyway.\n\n\tThomas\n\nCRAP CRAP CRAP: This is just the patch which showed me the debugging\noutput:\n\ndiff --git a/update-cache.c b/update-cache.c\n--- a/update-cache.c\n+++ b/update-cache.c\n@@ -174,6 +174,8 @@ static struct cache_entry *refresh_entry\n \tif (!changed)\n \t\treturn ce;\n \n+\tfprintf(stderr, \"name: %s 0x%08x\\n\", ce->name, changed);\n+\n \t/*\n \t * If the mode or type has changed, there's no point in trying\n \t * to refresh the entry - it's not going to match\n\nHere is the real patch:\n\n[PATCH] Don't include devicenumber into INODE_CHANGED test\n\nThis fixes the problem that git-update-cache --refresh rebuilds the\ncache stat information everytime it is started on a different host while\nworking in the same NFS shared repository.\n\nSigned-off-by: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>\n\ndiff --git a/read-cache.c b/read-cache.c\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -65,8 +65,7 @@ int ce_match_stat(struct cache_entry *ce\n \tif (ce->ce_uid != htonl(st->st_uid) ||\n \t    ce->ce_gid != htonl(st->st_gid))\n \t\tchanged |= OWNER_CHANGED;\n-\tif (ce->ce_dev != htonl(st->st_dev) ||\n-\t    ce->ce_ino != htonl(st->st_ino))\n+\tif (ce->ce_ino != htonl(st->st_ino))\n \t\tchanged |= INODE_CHANGED;\n \tif (ce->ce_size != htonl(st->st_size))\n \t\tchanged |= DATA_CHANGED;\n"},{"id":"3784","messageId":"20050522214115.GD23388@cip.informatik.uni-erlangen.de","threadId":"690","inReplyTo":"20050522212312.GC23388@cip.informatik.uni-erlangen.de","subject":"Alternate Patch: [PATCH] Don't include device number in cache invalidation when running on NFS","fromName":"Thomas Glanzmann","fromEmail":"sithglan@stud.uni-erlangen.de","sentAt":"2005-05-22T21:41:15Z","receivedAt":"2005-05-22T21:41:15Z","isPatch":true,"sender":{"key":"sithglan@stud.uni-erlangen.de","avatar":null},"body":"Hello,\n\n* Thomas Glanzmann <sithglan@stud.uni-erlangen.de> [050522 23:24]:\n> Hello,\n\n> > Sorry, I meant \"rename\", not \"link\", and yes, it should be NFS-safe. It's \n> > how all the mailers do things too, afaik.\n\n> okay. I will doublecheck that and come back.\n\nyes, you're right.\n\nWhile reading liblockfile I saw the following:\n\n/*\n *      See if the directory where is certain file is in\n *      is located on an NFS mounted volume.\n */\nstatic int is_nfs(const char *file)\n{\n        char dir[1024];\n        char *s;\n        struct stat st;\n\n        strncpy(dir, file, sizeof(dir));\n        if ((s = strrchr(dir, '/')) != NULL)\n                *s = 0;\n        else\n                strcpy(dir, \".\");\n\n        if (stat(dir, &st) < 0)\n                return 0;\n\n        return ((st.st_dev & 0xFF00) == 0);\n}\n\nSo here comes an alternate patch if you like to verify the st_dev for non\nNFS stuff. Also tested.\n\n[PATCH] Don't include device number in cache invalidation when running on NFS\n\nThis patches includes the device number only in the cache invalidation\nprocess when not running on a NFS volume.\n\nSigned-off-by: Thomas Glanzmann <sithglan@stud.uni-erlangen.de>\n\ndiff --git a/read-cache.c b/read-cache.c\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -65,8 +65,11 @@ int ce_match_stat(struct cache_entry *ce\n \tif (ce->ce_uid != htonl(st->st_uid) ||\n \t    ce->ce_gid != htonl(st->st_gid))\n \t\tchanged |= OWNER_CHANGED;\n-\tif (ce->ce_dev != htonl(st->st_dev) ||\n-\t    ce->ce_ino != htonl(st->st_ino))\n+\t/* Only include device number if not running on NFS */\n+\tif (ce->ce_dev != htonl(st->st_dev) &&\n+\t    ((st->st_dev & 0xFF00) == 0))\n+\t\tchanged |= INODE_CHANGED;\n+\tif (ce->ce_ino != htonl(st->st_ino))\n \t\tchanged |= INODE_CHANGED;\n \tif (ce->ce_size != htonl(st->st_size))\n \t\tchanged |= DATA_CHANGED;\n"},{"id":"3785","messageId":"Pine.LNX.4.58.0505221451590.2307@ppc970.osdl.org","threadId":"690","inReplyTo":"20050522214115.GD23388@cip.informatik.uni-erlangen.de","subject":"Re: Alternate Patch: [PATCH] Don't include device number in cache invalidation when running on NFS","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-22T21:58:39Z","receivedAt":"2005-05-22T21:58:39Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 22 May 2005, Thomas Glanzmann wrote:\n> \n> While reading liblockfile I saw the following:\n\nThis is _really_ Linux-specific afaik. Which is ok for git, but at the\nsame time it really makes me go \"Ewww\". It's testing that the major number \nis 0, and it would be a lot more cleaner to use \n\n\tif (!major(st.st_dev))\n\nbut even that is very Linux-specific.\n\n> [PATCH] Don't include device number in cache invalidation when running on NFS\n\nI'll have to think about it. Maybe I should just remove the st_dev check. \nI guess inode/size/mtime/ctime should be plenty safe enough in practice.\n\n\t\tLinus\n"},{"id":"3786","messageId":"20050522220734.GF23388@cip.informatik.uni-erlangen.de","threadId":"690","inReplyTo":"Pine.LNX.4.58.0505221451590.2307@ppc970.osdl.org","subject":"Re: Alternate Patch: [PATCH] Don't include device number in cache invalidation when running on NFS","fromName":"Thomas Glanzmann","fromEmail":"sithglan@stud.uni-erlangen.de","sentAt":"2005-05-22T22:07:34Z","receivedAt":"2005-05-22T22:07:34Z","isPatch":true,"sender":{"key":"sithglan@stud.uni-erlangen.de","avatar":null},"body":"Hello,\n\n> This is _really_ Linux-specific afaik. Which is ok for git, but at the\n> same time it really makes me go \"Ewww\". It's testing that the major number \n> is 0, and it would be a lot more cleaner to use \n\n> \tif (!major(st.st_dev))\n\n> but even that is very Linux-specific.\n\nI see.\n\n> I'll have to think about it. Maybe I should just remove the st_dev check. \n> I guess inode/size/mtime/ctime should be plenty safe enough in practice.\n\nI think so. At least I kick this one out because it is just getting on\nmy nerves. :-)\n\n\tThomas\n"}]}