git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Frank Ch. Eigler" <fche@redhat.com>
To: Linus Torvalds <torvalds@linuxfoundation.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: reducing prune sync()s
Date: Fri, 30 May 2008 11:25:27 -0400	[thread overview]
Message-ID: <20080530152527.GF4032@redhat.com> (raw)
In-Reply-To: <alpine.LFD.1.10.0805291656260.3141@woody.linux-foundation.org>

Hi -

On Thu, May 29, 2008 at 05:27:35PM -0700, Linus Torvalds wrote:
> [...]
> >	  Or perhaps having the blanket sync be replaced a
> > list of fsync()s for only the relevant git repository files?
> [...]
> Soemthing like this *may* work. THIS IS TOTALLY UNTESTED. And when I say 
> "TOTALLY UNTESTED", I mean it. Zero testing. None. Nada. Zilch. Testing is 
> for people who are actually interested in the feature (hint, hint).

The patch does add an fsync or two into the mix, a "git gc" or 
"git repack -a" still goes through the "git-repack" shell script, which
still did its "sync".  How about this patch, which adds a "git-fsync"
builtin for the shell scripts?  Added to yours, it replaces all the
syncs with fsync's, and tests fine in the same environment originally
reported.  (Lots of dirty data for another filesystem does not block
the fsyncs.)

diff --git a/.gitignore b/.gitignore
index 4ff2fec..708c5ac 100644
--- a/.gitignore
+++ b/.gitignore
@@ -47,6 +47,7 @@ git-for-each-ref
 git-format-patch
 git-fsck
 git-fsck-objects
+git-fsync
 git-gc
 git-get-tar-commit-id
 git-grep
diff --git a/Makefile b/Makefile
index 865e2bf..2148196 100644
--- a/Makefile
+++ b/Makefile
@@ -500,6 +500,7 @@ BUILTIN_OBJS += builtin-fetch.o
 BUILTIN_OBJS += builtin-fmt-merge-msg.o
 BUILTIN_OBJS += builtin-for-each-ref.o
 BUILTIN_OBJS += builtin-fsck.o
+BUILTIN_OBJS += builtin-fsync.o
 BUILTIN_OBJS += builtin-gc.o
 BUILTIN_OBJS += builtin-grep.o
 BUILTIN_OBJS += builtin-init-db.o
diff --git a/builtin-fsync.c b/builtin-fsync.c
new file mode 100644
index 0000000..8c4c7f7
--- /dev/null
+++ b/builtin-fsync.c
@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2008 Frank Ch. Eigler
+ */
+#include "cache.h"
+#include "commit.h"
+#include "tar.h"
+#include "builtin.h"
+#include "quote.h"
+
+static const char fsync_usage[] =
+"git-fsync <filepattern>...\n";
+
+int cmd_fsync(int argc, const char **argv, const char *prefix)
+{
+	int i;
+	for (i=1; i<argc; i++) {
+		int fd = open (argv[i], O_RDONLY);
+		if (fd < 0) {
+			error("unable to open for fsync %s", argv[i]);
+			return 1;
+		} else {
+			int rc = fsync (fd);
+			if (rc < 0) {
+				error("unable to fsync %s", argv[i]);
+				return 1;
+			}
+			close (fd);
+		}
+	}
+	return 0;
+}
diff --git a/builtin-prune.c b/builtin-prune.c
index 25f9304..d79a6ff 100644
--- a/builtin-prune.c
+++ b/builtin-prune.c
@@ -155,8 +155,6 @@ int cmd_prune(int argc, const char **argv, const char *prefix)
 	}
 	mark_reachable_objects(&revs, 1);
 	prune_object_dir(get_object_directory());
-
-	sync();
 	prune_packed_objects(show_only);
 	remove_temporary_files();
 	return 0;
diff --git a/builtin.h b/builtin.h
index 95126fd..4cedc5d 100644
--- a/builtin.h
+++ b/builtin.h
@@ -41,6 +41,7 @@ extern int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix);
 extern int cmd_for_each_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_format_patch(int argc, const char **argv, const char *prefix);
 extern int cmd_fsck(int argc, const char **argv, const char *prefix);
+extern int cmd_fsync(int argc, const char **argv, const char *prefix);
 extern int cmd_gc(int argc, const char **argv, const char *prefix);
 extern int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);
 extern int cmd_grep(int argc, const char **argv, const char *prefix);
diff --git a/git-repack.sh b/git-repack.sh
index 501519a..650372c 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -125,12 +125,11 @@ then
 	# We know $existing are all redundant.
 	if [ -n "$existing" ]
 	then
-		sync
 		( cd "$PACKDIR" &&
 		  for e in $existing
 		  do
 			case " $fullbases " in
-			*" $e "*) ;;
+			*" $e "*) git-fsync "$e.pack" "$e.idx" ;;
 			*)	rm -f "$e.pack" "$e.idx" "$e.keep" ;;
 			esac
 		  done
diff --git a/git.c b/git.c
index 89b431f..21f3b7d 100644
--- a/git.c
+++ b/git.c
@@ -305,6 +305,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "format-patch", cmd_format_patch, RUN_SETUP },
 		{ "fsck", cmd_fsck, RUN_SETUP },
 		{ "fsck-objects", cmd_fsck, RUN_SETUP },
+		{ "fsync", cmd_fsync },
 		{ "gc", cmd_gc, RUN_SETUP },
 		{ "get-tar-commit-id", cmd_get_tar_commit_id },
 		{ "grep", cmd_grep, RUN_SETUP | USE_PAGER },

  parent reply	other threads:[~2008-05-30 15:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-29 20:57 reducing prune sync()s Frank Ch. Eigler
2008-05-30  0:27 ` Linus Torvalds
2008-05-30  0:32   ` Linus Torvalds
2008-05-30  1:50     ` Frank Ch. Eigler
2008-05-30 20:07     ` Florian Weimer
2008-05-30  1:51   ` David Dillow
2008-05-30  2:17     ` Linus Torvalds
2008-05-30  2:30       ` Linus Torvalds
2008-05-30 15:25   ` Frank Ch. Eigler [this message]
2008-05-30 15:57     ` Linus Torvalds
2008-05-30 16:08       ` [PATCH 1/2] Make pack creation always fsync() the result Linus Torvalds
2008-05-30 16:11         ` [PATCH 2/2] Remove now unnecessary 'sync()' calls Linus Torvalds
2008-05-30 20:27         ` [PATCH 1/2] Make pack creation always fsync() the result Nicolas Pitre
2008-05-31 14:19         ` Frank Ch. Eigler
2008-06-02 22:23           ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080530152527.GF4032@redhat.com \
    --to=fche@redhat.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=torvalds@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).