{"thread":{"id":"18509","subject":"fatal: unable to write sha1 file git 1.6.2.1","startedAt":"2009-03-24T18:20:30Z","lastAt":"2009-03-25T10:52:15Z","messageCount":13,"participants":["Peter","Nicolas Pitre","Linus Torvalds","Jeff Layton","Steven French"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"109231","messageId":"49C9246E.3030508@mycircuit.org","threadId":"18509","inReplyTo":null,"subject":"fatal: unable to write sha1 file git 1.6.2.1","fromName":"Peter","fromEmail":"vmail@mycircuit.org","sentAt":"2009-03-24T18:20:30Z","receivedAt":"2009-03-24T18:20:30Z","isPatch":false,"sender":{"key":"vmail@mycircuit.org","avatar":null},"body":"Hi\nI try to add a directory with lots of binary files to a git repository.\nI receive the error message:\n*\nfatal: unable to write sha1 file\n\n*This is git 1.6.2.1.\n\nAre there limits concerning binary files ( like executables , images ) \nfor the inclusion in a git repo ?\n\nThanks a lot for your help\n\nPeter\n"},{"id":"109232","messageId":"alpine.LFD.2.00.0903241527420.26337@xanadu.home","threadId":"18509","inReplyTo":"49C9246E.3030508@mycircuit.org","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-03-24T19:30:47Z","receivedAt":"2009-03-24T19:30:47Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 24 Mar 2009, Peter wrote:\n\n> Hi\n> I try to add a directory with lots of binary files to a git repository.\n> I receive the error message:\n> *\n> fatal: unable to write sha1 file\n> \n> *This is git 1.6.2.1.\n> \n> Are there limits concerning binary files ( like executables , images ) for the\n> inclusion in a git repo ?\n\nCurrently, Git is unable to deal with files that it cannot load entirely \nin memory.\n\nWhat is the size of your largest file?\n\n\nNicolas\n"},{"id":"109233","messageId":"alpine.LFD.2.00.0903241226570.3032@localhost.localdomain","threadId":"18509","inReplyTo":"49C9246E.3030508@mycircuit.org","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-03-24T19:31:36Z","receivedAt":"2009-03-24T19:31:36Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 24 Mar 2009, Peter wrote:\n>\n> I try to add a directory with lots of binary files to a git repository.\n> I receive the error message:\n> *\n> fatal: unable to write sha1 file\n> \n> *This is git 1.6.2.1.\n> \n> Are there limits concerning binary files ( like executables , images ) for the\n> inclusion in a git repo ?\n\nIf that is the only error message you got, then afaik the only way that \ncan happen is if \"close(fd)\" returns an error.\n\nThe only other \"unable to write sha1 file\" message happens if \nwrite_buffer() fails, but if that happens then you should also have gotten \na\n\n\tfile write error ()\n\nmessage in addition to the \"unable to write sha1 file\".\n\nWhat OS? What filesystem? Are you perhaps running out of space?\n\nYou could also try to apply this patch to get more information (Junio, \nmaybe worth doing regardless? We should try to avoid ambiguous error \nmessages that don't give full error information)\n\n\t\tLinus\n---\n sha1_file.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 4563173..54972f9 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2287,7 +2287,7 @@ static void close_sha1_file(int fd)\n \t\tfsync_or_die(fd, \"sha1 file\");\n \tfchmod(fd, 0444);\n \tif (close(fd) != 0)\n-\t\tdie(\"unable to write sha1 file\");\n+\t\tdie(\"error when closing sha1 file (%s)\", strerror(errno));\n }\n \n /* Size of directory component, including the ending '/' */\n"},{"id":"109245","messageId":"49C94B02.5010604@mycircuit.org","threadId":"18509","inReplyTo":"alpine.LFD.2.00.0903241226570.3032@localhost.localdomain","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Peter","fromEmail":"vmail@mycircuit.org","sentAt":"2009-03-24T21:05:06Z","receivedAt":"2009-03-24T21:05:06Z","isPatch":false,"sender":{"key":"vmail@mycircuit.org","avatar":null},"body":"I consolidate from Nicolas and Linus, thanks for your help:\n\n\nCurrently, Git is unable to deal with files that it cannot load entirely \nin memory.\n\nWhat is the size of your largest file?\n\nThe biggest is 51 Mb , followed by one that is 23 MB. I was able to add \nand commit a part of the dir tree where the biggest file was 14 Mb. So \nthere seems to be a threshold between 23 Mb and 14 Mb.\n\n> If that is the only error message you got, then afaik the only way that \n> can happen is if \"close(fd)\" returns an error.\n> \nYes thats the only message I got.\n> The only other \"unable to write sha1 file\" message happens if \n> write_buffer() fails, but if that happens then you should also have gotten \n> a\n>\n> \tfile write error ()\n>\n> message in addition to the \"unable to write sha1 file\".\n>\n> What OS? What filesystem? Are you perhaps running out of space?\n> \nIts Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs ( this \nis a windows 2000 share mounted with samba in a VMware Workstation \nDebian client ( yes, I know ... )). Memory usage, according to htop, is \nconstant = 140/504 MB during the whole process until git fails.\n> You could also try to apply this patch to get more information (Junio, \n> maybe worth doing regardless? We should try to avoid ambiguous error \n> messages that don't give full error information)\n>\n> \t\tLinus\n> ---\n> sha1_file.c | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/sha1_file.c b/sha1_file.c\n> index 4563173..54972f9 100644\n> --- a/sha1_file.c\n> +++ b/sha1_file.c\n> @@ -2287,7 +2287,7 @@ static void close_sha1_file(int fd)\n> \t\tfsync_or_die(fd, \"sha1 file\");\n> \tfchmod(fd, 0444);\n> \tif (close(fd) != 0)\n> -\t\tdie(\"unable to write sha1 file\");\n> +\t\tdie(\"error when closing sha1 file (%s)\", strerror(errno));\n> }\n> \n> /* Size of directory component, including the ending '/' */\n> --\n> \nWill try it, thanks\nPeter\n"},{"id":"109256","messageId":"alpine.LFD.2.00.0903241510010.3032@localhost.localdomain","threadId":"18509","inReplyTo":"49C94B02.5010604@mycircuit.org","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-03-24T22:30:38Z","receivedAt":"2009-03-24T22:30:38Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 24 Mar 2009, Peter wrote:\n> > \n> > What OS? What filesystem? Are you perhaps running out of space?\n> \n> Its Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs ( this is a\n> windows 2000 share mounted with samba in a VMware Workstation Debian client (\n> yes, I know ... )). Memory usage, according to htop, is constant = 140/504 MB\n> during the whole process until git fails.\n\nOk, it sounds very much like a possible CIFS problem. \n\nGetting the exact error code for the \"close()\" will be interesting, \nbecause the only thing that can return an error under Linux in close() is \nif the filesystem \"->flush()\" function fails with an error.\n\nIn cifs, the cifs_flush() thing does a filemap_fdatawrite(), forcing the \ndata out, and that in turn calls do_writepages() which in turn calls \neither generic_writepages() or cifs_writepages() depending on random cifs \ncrap.\n\nI'm not seeing any obvious errors there. But I would _not_ be surprised if \nthe fchmod(fd, 0444) that we did before the close could be causing this: \ncifs gets confused and thinks that it must not write to the file because \nthe file has been turned read-only.\n\nHere's an idea: if this is reproducible for you, does the behavior change \nif you do\n\n\t[core]\n\t\tcore.fsyncobjectfiles = true\n\nin your .git/config file? That causes git to always fsync() the written \ndata _before_ it does that fchmod(), which in turn means that by the time \nthe close() rolls around, there should be no data to write, and thus no \npossibility that anybody gets confused when there is still dirty data on a \nfile that has been marked read-only.\n\nAnyway, I'm cc'ing Steve French and Jeff Layton, as the major CIFS go-to \nguys. It does seem like a CIFS bug is likely.\n\nSteve, Jeff: git does basically\n\n\topen(\".git/objects/xy/tmp_obj_xyzzy\", O_RDWR|O_CREAT|O_EXCL, 0600) = 5\n\twrite(5, \"..\"..., len)\n\tfchmod(5, 0444)\n\tclose(5)\n\tlink(\".git/objects/xy/tmp_obj_xyzzy\", \".git/objects/xy/xyzzy\");\n\tunlink(\".git/objects/xy/tmp_obj_xyzzy\");\n\nto write a new datafile. Under CIFS, that \"close()\" apparently sometimes \nreturns with an error, and fails.\n\nI guess we could change the \"fchmod()\" to happen after the close(), just \nto make it easier for filesystems to get this right. And yes, as outlined \nabove, there's a config option to make git use fdatasync() before it does \nthat fchmod() too. But it does seem like CIFS is just buggy.\n\nIf CIFS has problems with the above sequence (say, if some timeout \nrefreshes the inode data or causes a re-connect with the server or \nwhatever), then maybe cifs should always do an implicit fdatasync() when \ndoing fchmod(), just to make sure that the fchmod won't screw up any \ncached dirty data?\n\n\t\tLinus\n"},{"id":"109258","messageId":"49C961D0.4010704@mycircuit.org","threadId":"18509","inReplyTo":"alpine.LFD.2.00.0903241510010.3032@localhost.localdomain","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Peter","fromEmail":"vmail@mycircuit.org","sentAt":"2009-03-24T22:42:24Z","receivedAt":"2009-03-24T22:42:24Z","isPatch":false,"sender":{"key":"vmail@mycircuit.org","avatar":null},"body":"Linus Torvalds wrote:\n> On Tue, 24 Mar 2009, Peter wrote:\n> \n>>> What OS? What filesystem? Are you perhaps running out of space?\n>>> \n>> \n>> Its Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs ( this is a\n>> windows 2000 share mounted with samba in a VMware Workstation Debian client (\n>> yes, I know ... )). Memory usage, according to htop, is constant = 140/504 MB\n>> during the whole process until git fails.\n>> \n>\n> Ok, it sounds very much like a possible CIFS problem. \n>\n> Getting the exact error code for the \"close()\" will be interesting, \n> because the only thing that can return an error under Linux in close() is \n> if the filesystem \"->flush()\" function fails with an error.\n>\n> In cifs, the cifs_flush() thing does a filemap_fdatawrite(), forcing the \n> data out, and that in turn calls do_writepages() which in turn calls \n> either generic_writepages() or cifs_writepages() depending on random cifs \n> crap.\n>\n> I'm not seeing any obvious errors there. But I would _not_ be surprised if \n> the fchmod(fd, 0444) that we did before the close could be causing this: \n> cifs gets confused and thinks that it must not write to the file because \n> the file has been turned read-only.\n>\n> Here's an idea: if this is reproducible for you, does the behavior change \n> if you do\n>\n> \t[core]\n> \t\tcore.fsyncobjectfiles = true\n>\n> in your .git/config file? That causes git to always fsync() the written \n> data _before_ it does that fchmod(), which in turn means that by the time \n> the close() rolls around, there should be no data to write, and thus no \n> possibility that anybody gets confused when there is still dirty data on a \n> file that has been marked read-only.\n>\n> Anyway, I'm cc'ing Steve French and Jeff Layton, as the major CIFS go-to \n> guys. It does seem like a CIFS bug is likely.\n>\n> Steve, Jeff: git does basically\n>\n> \topen(\".git/objects/xy/tmp_obj_xyzzy\", O_RDWR|O_CREAT|O_EXCL, 0600) = 5\n> \twrite(5, \"..\"..., len)\n> \tfchmod(5, 0444)\n> \tclose(5)\n> \tlink(\".git/objects/xy/tmp_obj_xyzzy\", \".git/objects/xy/xyzzy\");\n> \tunlink(\".git/objects/xy/tmp_obj_xyzzy\");\n>\n> to write a new datafile. Under CIFS, that \"close()\" apparently sometimes \n> returns with an error, and fails.\n>\n> I guess we could change the \"fchmod()\" to happen after the close(), just \n> to make it easier for filesystems to get this right. And yes, as outlined \n> above, there's a config option to make git use fdatasync() before it does \n> that fchmod() too. But it does seem like CIFS is just buggy.\n>\n> If CIFS has problems with the above sequence (say, if some timeout \n> refreshes the inode data or causes a re-connect with the server or \n> whatever), then maybe cifs should always do an implicit fdatasync() when \n> doing fchmod(), just to make sure that the fchmod won't screw up any \n> cached dirty data?\n>\n> \t\tLinus\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \nHi\nThanks a lot , I will check that out tomorrow, in the meantime, this is \nthe result of your patch being applied:\n\n$ git add \n\nfatal: error when closing sha1 file (Bad file descriptor)\n\nPeter\n"},{"id":"109269","messageId":"20090324193506.0962b28e@tupile.poochiereds.net","threadId":"18509","inReplyTo":"alpine.LFD.2.00.0903241510010.3032@localhost.localdomain","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Jeff Layton","fromEmail":"jlayton@redhat.com","sentAt":"2009-03-24T23:35:06Z","receivedAt":"2009-03-24T23:35:06Z","isPatch":false,"sender":{"key":"jlayton@redhat.com","avatar":null},"body":"On Tue, 24 Mar 2009 15:30:38 -0700 (PDT)\nLinus Torvalds wrote:\n\n> \n> \n> On Tue, 24 Mar 2009, Peter wrote:\n> > > \n> > > What OS? What filesystem? Are you perhaps running out of space?\n> > \n> > Its Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs ( this is a\n> > windows 2000 share mounted with samba in a VMware Workstation Debian client (\n> > yes, I know ... )). Memory usage, according to htop, is constant = 140/504 MB\n> > during the whole process until git fails.\n> \n> Ok, it sounds very much like a possible CIFS problem. \n> \n> Getting the exact error code for the \"close()\" will be interesting, \n> because the only thing that can return an error under Linux in close() is \n> if the filesystem \"->flush()\" function fails with an error.\n> \n> In cifs, the cifs_flush() thing does a filemap_fdatawrite(), forcing the \n> data out, and that in turn calls do_writepages() which in turn calls \n> either generic_writepages() or cifs_writepages() depending on random cifs \n> crap.\n> \n> I'm not seeing any obvious errors there. But I would _not_ be surprised if \n> the fchmod(fd, 0444) that we did before the close could be causing this: \n> cifs gets confused and thinks that it must not write to the file because \n> the file has been turned read-only.\n> \n> Here's an idea: if this is reproducible for you, does the behavior change \n> if you do\n> \n> \t[core]\n> \t\tcore.fsyncobjectfiles = true\n> \n> in your .git/config file? That causes git to always fsync() the written \n> data _before_ it does that fchmod(), which in turn means that by the time \n> the close() rolls around, there should be no data to write, and thus no \n> possibility that anybody gets confused when there is still dirty data on a \n> file that has been marked read-only.\n> \n> Anyway, I'm cc'ing Steve French and Jeff Layton, as the major CIFS go-to \n> guys. It does seem like a CIFS bug is likely.\n> \n> Steve, Jeff: git does basically\n> \n> \topen(\".git/objects/xy/tmp_obj_xyzzy\", O_RDWR|O_CREAT|O_EXCL, 0600) = 5\n> \twrite(5, \"..\"..., len)\n> \tfchmod(5, 0444)\n> \tclose(5)\n> \tlink(\".git/objects/xy/tmp_obj_xyzzy\", \".git/objects/xy/xyzzy\");\n> \tunlink(\".git/objects/xy/tmp_obj_xyzzy\");\n> \n> to write a new datafile. Under CIFS, that \"close()\" apparently sometimes \n> returns with an error, and fails.\n> \n> I guess we could change the \"fchmod()\" to happen after the close(), just \n> to make it easier for filesystems to get this right. And yes, as outlined \n> above, there's a config option to make git use fdatasync() before it does \n> that fchmod() too. But it does seem like CIFS is just buggy.\n> \n> If CIFS has problems with the above sequence (say, if some timeout \n> refreshes the inode data or causes a re-connect with the server or \n> whatever), then maybe cifs should always do an implicit fdatasync() when \n> doing fchmod(), just to make sure that the fchmod won't screw up any \n> cached dirty data?\n> \n\nYes. That's probably the right thing to do here. This is looks like a\nregression that I introduced some time ago in\ncea218054ad277d6c126890213afde07b4eb1602. Before that delta we always\nflushed all the data before doing any setattr. After that delta, we\njust did it on size changes. In a later commit, Steve fixed it so that\nit got done on ATTR_MTIME too.\n\nWe can easily change the cifs_setattr variants to flush all the dirty\ndata on ATTR_MODE changes, and maybe even change it to only flush on\ncertain ATTR_MODE changes, like when we're clearing all the write bits.\n\nI wonder though whether that's sufficient. If we're changing ownership\nfor instance, will we hit the same issue? Maybe the safest approach is\njust to go back to flushing on any setattr call.\n\nSteve, thoughts?\n\n-- \nJeff Layton \n"},{"id":"109273","messageId":"alpine.LFD.2.00.0903241655210.3032@localhost.localdomain","threadId":"18509","inReplyTo":"49C961D0.4010704@mycircuit.org","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-03-25T00:03:22Z","receivedAt":"2009-03-25T00:03:22Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 24 Mar 2009, Peter wrote:\n>\n> Thanks a lot , I will check that out tomorrow, in the meantime, this is the\n> result of your patch being applied:\n> \n> $ git add \n> \n> fatal: error when closing sha1 file (Bad file descriptor)\n\nOk, that's probably cifs_writepages() doing\n\n open_file = find_writable_file(CIFS_I(mapping->host));\n if (!open_file) {\n cERROR(1, (\"No writable handles for inode\"));\n rc = -EBADF;\n\t\t\t} else {\n\t\t\t\t..\n\nso yeah, looks like it's the fchmod() that triggers it.\n\nI suspect this would be a safer - if slightly slower - way to make sure \nthe file is read-only. It's slower, because it is going to look up the \nfilename once more, but I bet it is going to avoid this particular CIFS \nbug.\n\n\t\t\tLinus\n---\n http-push.c | 2 +-\n sha1_file.c | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 48e5f38..ba4fa4d 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -748,8 +748,8 @@ static void finish_request(struct transfer_request *request)\n \t\t\taborted = 1;\n \t\t}\n \t} else if (request->state == RUN_FETCH_LOOSE) {\n-\t\tfchmod(request->local_fileno, 0444);\n \t\tclose(request->local_fileno); request->local_fileno = -1;\n+\t\tchmod(request->tmpfile, 0444);\n \n \t\tif (request->curl_result != CURLE_OK &&\n \t\t request->http_code != 416) {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 4563173..8268da7 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2285,9 +2285,8 @@ static void close_sha1_file(int fd)\n {\n \tif (fsync_object_files)\n \t\tfsync_or_die(fd, \"sha1 file\");\n-\tfchmod(fd, 0444);\n \tif (close(fd) != 0)\n-\t\tdie(\"unable to write sha1 file\");\n+\t\tdie(\"error when closing sha1 file (%s)\", strerror(errno));\n }\n \n /* Size of directory component, including the ending '/' */\n@@ -2384,6 +2383,7 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,\n \tclose_sha1_file(fd);\n \tfree(compressed);\n \n+\tchmod(tmpfile, 0444);\n \tif (mtime) {\n \t\tstruct utimbuf utb;\n \t\tutb.actime = mtime;\n"},{"id":"109274","messageId":"alpine.LFD.2.00.0903241703290.3032@localhost.localdomain","threadId":"18509","inReplyTo":"20090324193506.0962b28e@tupile.poochiereds.net","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-03-25T00:11:40Z","receivedAt":"2009-03-25T00:11:40Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 24 Mar 2009, Jeff Layton wrote:\n> \n> Yes. That's probably the right thing to do here. This is looks like a\n> regression that I introduced some time ago in\n> cea218054ad277d6c126890213afde07b4eb1602. Before that delta we always\n> flushed all the data before doing any setattr. After that delta, we\n> just did it on size changes. In a later commit, Steve fixed it so that\n> it got done on ATTR_MTIME too.\n\nAhh. Yes. You do need to flush on at least ATTR_MODE and ATTR_UID/GID \nchanges too.\n\n> I wonder though whether that's sufficient. If we're changing ownership\n> for instance, will we hit the same issue? Maybe the safest approach is\n> just to go back to flushing on any setattr call.\n> \n> Steve, thoughts?\n\nIs there anything relevant left to be sufficient reason to make it \nconditional? Once you're doing it on MODE/SIZE/UID/GID/MTIME changes, that \npretty much will cover all set_attr calls. \n\n\t\t\tLinus\n"},{"id":"109275","messageId":"OFBC676D76.DF7BF27E-ON87257584.0000C3CB-86257584.0001BE68@us.ibm.com","threadId":"18509","inReplyTo":"20090324193506.0962b28e@tupile.poochiereds.net","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Steven French","fromEmail":"sfrench@us.ibm.com","sentAt":"2009-03-25T00:17:34Z","receivedAt":"2009-03-25T00:17:34Z","isPatch":false,"sender":{"key":"sfrench@us.ibm.com","avatar":null},"body":"Jeff Layton wrote on 03/24/2009 06:35:06 PM:\n\n> On Tue, 24 Mar 2009 15:30:38 -0700 (PDT)\n> Linus Torvalds wrote:\n> \n> > \n> > \n> > On Tue, 24 Mar 2009, Peter wrote:\n> > > > \n> > > > What OS? What filesystem? Are you perhaps running out of space?\n> > > \n> > > Its Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs \n> ( this is a\n> > > windows 2000 share mounted with samba in a VMware Workstation \n> Debian client (\n> > > yes, I know ... )). Memory usage, according to htop, is constant\n> = 140/504 MB\n> > > during the whole process until git fails.\n> > \n> > Ok, it sounds very much like a possible CIFS problem. \n> > \n> > Getting the exact error code for the \"close()\" will be interesting, \n> > because the only thing that can return an error under Linux in close() \nis \n> > if the filesystem \"->flush()\" function fails with an error.\n> > \n> > In cifs, the cifs_flush() thing does a filemap_fdatawrite(), forcing \nthe \n> > data out, and that in turn calls do_writepages() which in turn calls \n> > either generic_writepages() or cifs_writepages() depending on random \ncifs \n> > crap.\n> > \n> > I'm not seeing any obvious errors there. But I would _not_ be \nsurprised if \n> > the fchmod(fd, 0444) that we did before the close could be causing \nthis: \n> > cifs gets confused and thinks that it must not write to the file \nbecause \n> > the file has been turned read-only.\n> > \n> > Here's an idea: if this is reproducible for you, does the behavior \nchange \n> > if you do\n> > \n> > [core]\n> > core.fsyncobjectfiles = true\n> > \n> > in your .git/config file? That causes git to always fsync() the \nwritten \n> > data _before_ it does that fchmod(), which in turn means that by the \ntime \n> > the close() rolls around, there should be no data to write, and thus \nno \n> > possibility that anybody gets confused when there is still dirty data \non a \n> > file that has been marked read-only.\n> > \n> > Anyway, I'm cc'ing Steve French and Jeff Layton, as the major CIFS \ngo-to \n> > guys. It does seem like a CIFS bug is likely.\n> > \n> > Steve, Jeff: git does basically\n> > \n> > open(\".git/objects/xy/tmp_obj_xyzzy\", O_RDWR|O_CREAT|O_EXCL, 0600) \n= 5\n> > write(5, \"..\"..., len)\n> > fchmod(5, 0444)\n> > close(5)\n> > link(\".git/objects/xy/tmp_obj_xyzzy\", \".git/objects/xy/xyzzy\");\n> > unlink(\".git/objects/xy/tmp_obj_xyzzy\");\n> > \n> > to write a new datafile. Under CIFS, that \"close()\" apparently \nsometimes \n> > returns with an error, and fails.\n> > \n> > I guess we could change the \"fchmod()\" to happen after the close(), \njust \n> > to make it easier for filesystems to get this right. And yes, as \noutlined \n> > above, there's a config option to make git use fdatasync() before it \ndoes \n> > that fchmod() too. But it does seem like CIFS is just buggy.\n> > \n> > If CIFS has problems with the above sequence (say, if some timeout \n> > refreshes the inode data or causes a re-connect with the server or \n> > whatever), then maybe cifs should always do an implicit fdatasync() \nwhen \n> > doing fchmod(), just to make sure that the fchmod won't screw up any \n> > cached dirty data?\n> > \n> \n> Yes. That's probably the right thing to do here. This is looks like a\n> regression that I introduced some time ago in\n> cea218054ad277d6c126890213afde07b4eb1602. Before that delta we always\n> flushed all the data before doing any setattr. After that delta, we\n> just did it on size changes. In a later commit, Steve fixed it so that\n> it got done on ATTR_MTIME too.\n> \n> We can easily change the cifs_setattr variants to flush all the dirty\n> data on ATTR_MODE changes, and maybe even change it to only flush on\n> certain ATTR_MODE changes, like when we're clearing all the write bits.\n> \n> I wonder though whether that's sufficient. If we're changing ownership\n> for instance, will we hit the same issue? Maybe the safest approach is\n> just to go back to flushing on any setattr call.\n> \n> Steve, thoughts?\n\nFlushing on all setattr calls seems excessive, but the case of\nfchmod(0444) followed by close should not require it, and should\nwork over cifs unless the tcp socket drops between the chmod\nand the close/flush (requiring reconnection and reopening\nthe file handle). For the normal case, the chmod should not\nmatter, since for cifs, unlike NFSv3, there is a network\nopen operation and access checks are done on the open call, and \nyou already have the file open for write). If /proc/fs/cifs/Stats\nshows the number of reconnects as 0, then we don't have the case \nof dropping the network between chmod and flush/close. Since certain\nwritebehind errors (in writepages) can not be returned to the application\nuntil flush/close, it is just as likely that what you are seeing is\nan error from a previous failed write. The logic for handling\nerrors in cifs_writepages is much better in recent kernels.\n\nWhat is the kernel version?\n\nSteve French\nSenior Software Engineer\nLinux Technology Center - IBM Austin\nphone: 512-838-2294\nemail: sfrench at-sign us dot ibm dot com\n"},{"id":"109278","messageId":"20090324202429.130f33c3@tleilax.poochiereds.net","threadId":"18509","inReplyTo":"alpine.LFD.2.00.0903241655210.3032@localhost.localdomain","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Jeff Layton","fromEmail":"jlayton@redhat.com","sentAt":"2009-03-25T00:24:29Z","receivedAt":"2009-03-25T00:24:29Z","isPatch":false,"sender":{"key":"jlayton@redhat.com","avatar":null},"body":"On Tue, 24 Mar 2009 17:03:22 -0700 (PDT)\nLinus Torvalds wrote:\n\n> \n> \n> On Tue, 24 Mar 2009, Peter wrote:\n> >\n> > Thanks a lot , I will check that out tomorrow, in the meantime, this is the\n> > result of your patch being applied:\n> > \n> > $ git add \n> > \n> > fatal: error when closing sha1 file (Bad file descriptor)\n> \n> Ok, that's probably cifs_writepages() doing\n> \n> open_file = find_writable_file(CIFS_I(mapping->host));\n> if (!open_file) {\n> cERROR(1, (\"No writable handles for inode\"));\n> rc = -EBADF;\n> \t\t\t} else {\n> \t\t\t\t..\n> \n> so yeah, looks like it's the fchmod() that triggers it.\n> \n> I suspect this would be a safer - if slightly slower - way to make sure \n> the file is read-only. It's slower, because it is going to look up the \n> filename once more, but I bet it is going to avoid this particular CIFS \n> bug.\n>\n\nYeah, that should work around it. This CIFS patch should also fix it.\nIt's untested but it's reasonably simple.\n\nJust out of curiosity, what sort of server are you mounting here?\n\n--------------[snip]------------------\n\n>From ea8dc9927fb9542bb1c73b1982cc44bf3d4a9798 Mon Sep 17 00:00:00 2001\nFrom: Jeff Layton \nDate: Tue, 24 Mar 2009 19:50:17 -0400\nSubject: [PATCH] cifs: flush data on any setattr\n\nWe already flush all the dirty pages for an inode before doing\nATTR_SIZE and ATTR_MTIME changes. There's another problem though -- if\nwe change the mode so that the file becomes read-only then we may not\nbe able to write data to it afterward.\n\nFix this by just going back to flushing all the dirty data on any\nsetattr call. There are probably some cases that can be optimized out,\nbut we need to take care that we don't cause a regression by doing that.\n\nSigned-off-by: Jeff Layton \n---\n fs/cifs/inode.c | 58 ++++++++++++++++++++++++++++--------------------------\n 1 files changed, 30 insertions(+), 28 deletions(-)\n\ndiff --git a/fs/cifs/inode.c b/fs/cifs/inode.c\nindex a8797cc..f4e880d 100644\n--- a/fs/cifs/inode.c\n+++ b/fs/cifs/inode.c\n@@ -1792,20 +1792,21 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)\n \t\tgoto out;\n \t}\n \n-\tif ((attrs->ia_valid & ATTR_MTIME) || (attrs->ia_valid & ATTR_SIZE)) {\n-\t\t/*\n-\t\t Flush data before changing file size or changing the last\n-\t\t write time of the file on the server. If the\n-\t\t flush returns error, store it to report later and continue.\n-\t\t BB: This should be smarter. Why bother flushing pages that\n-\t\t will be truncated anyway? Also, should we error out here if\n-\t\t the flush returns error?\n-\t\t */\n-\t\trc = filemap_write_and_wait(inode->i_mapping);\n-\t\tif (rc != 0) {\n-\t\t\tcifsInode->write_behind_rc = rc;\n-\t\t\trc = 0;\n-\t\t}\n+\t/*\n+\t * Attempt to flush data before changing attributes. We need to do\n+\t * this for ATTR_SIZE and ATTR_MTIME for sure, and if we change the\n+\t * ownership or mode then we may also need to do this. Here, we take\n+\t * the safe way out and just do the flush on all setattr requests. If\n+\t * the flush returns error, store it to report later and continue.\n+\t *\n+\t * BB: This should be smarter. Why bother flushing pages that\n+\t * will be truncated anyway? Also, should we error out here if\n+\t * the flush returns error?\n+\t */\n+\trc = filemap_write_and_wait(inode->i_mapping);\n+\tif (rc != 0) {\n+\t\tcifsInode->write_behind_rc = rc;\n+\t\trc = 0;\n \t}\n \n \tif (attrs->ia_valid & ATTR_SIZE) {\n@@ -1903,20 +1904,21 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)\n \t\treturn -ENOMEM;\n \t}\n \n-\tif ((attrs->ia_valid & ATTR_MTIME) || (attrs->ia_valid & ATTR_SIZE)) {\n-\t\t/*\n-\t\t Flush data before changing file size or changing the last\n-\t\t write time of the file on the server. If the\n-\t\t flush returns error, store it to report later and continue.\n-\t\t BB: This should be smarter. Why bother flushing pages that\n-\t\t will be truncated anyway? Also, should we error out here if\n-\t\t the flush returns error?\n-\t\t */\n-\t\trc = filemap_write_and_wait(inode->i_mapping);\n-\t\tif (rc != 0) {\n-\t\t\tcifsInode->write_behind_rc = rc;\n-\t\t\trc = 0;\n-\t\t}\n+\t/*\n+\t * Attempt to flush data before changing attributes. We need to do\n+\t * this for ATTR_SIZE and ATTR_MTIME for sure, and if we change the\n+\t * ownership or mode then we may also need to do this. Here, we take\n+\t * the safe way out and just do the flush on all setattr requests. If\n+\t * the flush returns error, store it to report later and continue.\n+\t *\n+\t * BB: This should be smarter. Why bother flushing pages that\n+\t * will be truncated anyway? Also, should we error out here if\n+\t * the flush returns error?\n+\t */\n+\trc = filemap_write_and_wait(inode->i_mapping);\n+\tif (rc != 0) {\n+\t\tcifsInode->write_behind_rc = rc;\n+\t\trc = 0;\n \t}\n \n \tif (attrs->ia_valid & ATTR_SIZE) {\n-- \n1.6.0.6\n"},{"id":"109284","messageId":"20090324204925.7037435f@tleilax.poochiereds.net","threadId":"18509","inReplyTo":"OFBC676D76.DF7BF27E-ON87257584.0000C3CB-86257584.0001BE68@us.ibm.com","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Jeff Layton","fromEmail":"jlayton@redhat.com","sentAt":"2009-03-25T00:49:25Z","receivedAt":"2009-03-25T00:49:25Z","isPatch":false,"sender":{"key":"jlayton@redhat.com","avatar":null},"body":"On Tue, 24 Mar 2009 19:17:34 -0500\nSteven French wrote:\n\n> Jeff Layton wrote on 03/24/2009 06:35:06 PM:\n> \n> > On Tue, 24 Mar 2009 15:30:38 -0700 (PDT)\n> > Linus Torvalds wrote:\n> > \n> > > \n> > > \n> > > On Tue, 24 Mar 2009, Peter wrote:\n> > > > > \n> > > > > What OS? What filesystem? Are you perhaps running out of space?\n> > > > \n> > > > Its Lenny Debian 5.0.0, Diskspace is ample . Filesystem is cifs \n> > ( this is a\n> > > > windows 2000 share mounted with samba in a VMware Workstation \n> > Debian client (\n> > > > yes, I know ... )). Memory usage, according to htop, is constant\n> > = 140/504 MB\n> > > > during the whole process until git fails.\n> > > \n> > > Ok, it sounds very much like a possible CIFS problem. \n> > > \n> > > Getting the exact error code for the \"close()\" will be interesting, \n> > > because the only thing that can return an error under Linux in close() \n> is \n> > > if the filesystem \"->flush()\" function fails with an error.\n> > > \n> > > In cifs, the cifs_flush() thing does a filemap_fdatawrite(), forcing \n> the \n> > > data out, and that in turn calls do_writepages() which in turn calls \n> > > either generic_writepages() or cifs_writepages() depending on random \n> cifs \n> > > crap.\n> > > \n> > > I'm not seeing any obvious errors there. But I would _not_ be \n> surprised if \n> > > the fchmod(fd, 0444) that we did before the close could be causing \n> this: \n> > > cifs gets confused and thinks that it must not write to the file \n> because \n> > > the file has been turned read-only.\n> > > \n> > > Here's an idea: if this is reproducible for you, does the behavior \n> change \n> > > if you do\n> > > \n> > > [core]\n> > > core.fsyncobjectfiles = true\n> > > \n> > > in your .git/config file? That causes git to always fsync() the \n> written \n> > > data _before_ it does that fchmod(), which in turn means that by the \n> time \n> > > the close() rolls around, there should be no data to write, and thus \n> no \n> > > possibility that anybody gets confused when there is still dirty data \n> on a \n> > > file that has been marked read-only.\n> > > \n> > > Anyway, I'm cc'ing Steve French and Jeff Layton, as the major CIFS \n> go-to \n> > > guys. It does seem like a CIFS bug is likely.\n> > > \n> > > Steve, Jeff: git does basically\n> > > \n> > > open(\".git/objects/xy/tmp_obj_xyzzy\", O_RDWR|O_CREAT|O_EXCL, 0600) \n> = 5\n> > > write(5, \"..\"..., len)\n> > > fchmod(5, 0444)\n> > > close(5)\n> > > link(\".git/objects/xy/tmp_obj_xyzzy\", \".git/objects/xy/xyzzy\");\n> > > unlink(\".git/objects/xy/tmp_obj_xyzzy\");\n> > > \n> > > to write a new datafile. Under CIFS, that \"close()\" apparently \n> sometimes \n> > > returns with an error, and fails.\n> > > \n> > > I guess we could change the \"fchmod()\" to happen after the close(), \n> just \n> > > to make it easier for filesystems to get this right. And yes, as \n> outlined \n> > > above, there's a config option to make git use fdatasync() before it \n> does \n> > > that fchmod() too. But it does seem like CIFS is just buggy.\n> > > \n> > > If CIFS has problems with the above sequence (say, if some timeout \n> > > refreshes the inode data or causes a re-connect with the server or \n> > > whatever), then maybe cifs should always do an implicit fdatasync() \n> when \n> > > doing fchmod(), just to make sure that the fchmod won't screw up any \n> > > cached dirty data?\n> > > \n> > \n> > Yes. That's probably the right thing to do here. This is looks like a\n> > regression that I introduced some time ago in\n> > cea218054ad277d6c126890213afde07b4eb1602. Before that delta we always\n> > flushed all the data before doing any setattr. After that delta, we\n> > just did it on size changes. In a later commit, Steve fixed it so that\n> > it got done on ATTR_MTIME too.\n> > \n> > We can easily change the cifs_setattr variants to flush all the dirty\n> > data on ATTR_MODE changes, and maybe even change it to only flush on\n> > certain ATTR_MODE changes, like when we're clearing all the write bits.\n> > \n> > I wonder though whether that's sufficient. If we're changing ownership\n> > for instance, will we hit the same issue? Maybe the safest approach is\n> > just to go back to flushing on any setattr call.\n> > \n> > Steve, thoughts?\n> \n> Flushing on all setattr calls seems excessive, but the case of\n> fchmod(0444) followed by close should not require it, and should\n> work over cifs unless the tcp socket drops between the chmod\n> and the close/flush (requiring reconnection and reopening\n> the file handle). For the normal case, the chmod should not\n> matter, since for cifs, unlike NFSv3, there is a network\n> open operation and access checks are done on the open call, and \n> you already have the file open for write). If /proc/fs/cifs/Stats\n> shows the number of reconnects as 0, then we don't have the case \n> of dropping the network between chmod and flush/close. Since certain\n> writebehind errors (in writepages) can not be returned to the application\n> until flush/close, it is just as likely that what you are seeing is\n> an error from a previous failed write. The logic for handling\n> errors in cifs_writepages is much better in recent kernels.\n> \n> What is the kernel version?\n> \n\nGood point -- I stand corrected. I just gave this series of calls a quick\ntest against a win2k8 server:\n\nunlink(\"/mnt/win2k8/testfile\") = 0\nopen(\"/mnt/win2k8/testfile\", O_RDWR|O_CREAT, 0644) = 3\nwrite(3, \"\\1\\0\\0\\0\\0\\0\\0\\0\\35\\244@\\211:\\0\\0\\0\\7\\0\\0\\0\\0\\0\\0\\0X\\r\\25-\\377\\177\\0\\0`\"..., 1024) = 1024\nfchmod(3, 0444) = 0\nclose(3) = 0\n\nHere's what it looks like on the wire (forgive the IPv6 addresses, it's\njust what I had handy)...\n\nCreate the file:\n 3.842955 feed::2 -> feed::8 SMB NT Create AndX Request, Path: \\testfile\n 3.843641 feed::8 -> feed::2 SMB NT Create AndX Response, FID: 0x4007\n\nSet the ATTR_READONLY bit:\n 3.844255 feed::2 -> feed::8 SMB Trans2 Request, SET_FILE_INFO, FID: 0x4007\n 3.845070 feed::8 -> feed::2 SMB Trans2 Response, FID: 0x4007, SET_FILE_INFO\n\nWrite the data:\n 3.845766 feed::2 -> feed::8 SMB Write AndX Request, FID: 0x4007, 1024 bytes at offset 0\n 3.846627 feed::8 -> feed::2 SMB Write AndX Response, FID: 0x4007, 1024 bytes\n\nClose the file:\n 3.847043 feed::2 -> feed::8 SMB Close Request, FID: 0x4007\n 3.847940 feed::8 -> feed::2 SMB Close Response, FID: 0x4007\n\n...none of that errored out, even though the writes clearly hit the\nserver after the mode change. Either there's something else going on\nhere, or this server is misbehaving. It might be interesting to see a\ncapture.\n\nSome info about what sort of server this is might also be helpful.\n\n-- \nJeff Layton \n"},{"id":"109334","messageId":"49CA0CDF.7060203@mycircuit.org","threadId":"18509","inReplyTo":"alpine.LFD.2.00.0903241510010.3032@localhost.localdomain","subject":"Re: fatal: unable to write sha1 file git 1.6.2.1","fromName":"Peter","fromEmail":"vmail@mycircuit.org","sentAt":"2009-03-25T10:52:15Z","receivedAt":"2009-03-25T10:52:15Z","isPatch":false,"sender":{"key":"vmail@mycircuit.org","avatar":null},"body":"\n>\n> Here's an idea: if this is reproducible for you, does the behavior change \n> if you do\n>\n> \t[core]\n> \t\tcore.fsyncobjectfiles = true\n>\n> \nYes, that works !\nThanks a lot !\n\nPeter\n"}]}