unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Linux-only fdatasync() in 3c13bc
@ 2014-05-06 19:59 Xīcò
  2014-05-07 11:20 ` Kushal Kumaran
  0 siblings, 1 reply; 8+ messages in thread
From: Xīcò @ 2014-05-06 19:59 UTC (permalink / raw)
  To: notmuch

[-- Attachment #1: Type: text/plain, Size: 149 bytes --]

Also, commit 3c13bc introduced a call to fdatasync() which is not
available on FreeBSD, and probably not either on MacOS at least.

Best,

--
Xīcò

[-- Attachment #2: 0002-Compatibility-for-Linux-s-fdatasync.patch --]
[-- Type: text/x-diff, Size: 2826 bytes --]

From e2b9520e69e52b35348d07eb53a6a88d1397fa3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?X=C4=ABc=C3=B2?= <xico@atelo.org>
Date: Tue, 6 May 2014 12:56:03 -0700
Subject: [PATCH 1/1] =?UTF-8?q?Compatibility=20for=20Linux=E2=80=99s=20fda?=
 =?UTF-8?q?tasync().?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 configure      | 7 +++++++
 notmuch-dump.c | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/configure b/configure
index 7204812..7bc602d 100755
--- a/configure
+++ b/configure
@@ -271,6 +271,7 @@ EOF
 errors=0
 
 libdir_in_ldconfig=0
+have_fdatasync=0
 
 printf "Checking which platform we are on... "
 uname=`uname`
@@ -294,6 +295,7 @@ elif [ $uname = "Linux" ] || [ $uname = "GNU" ] ; then
     printf "$uname\n"
     platform="$uname"
     linker_resolves_library_dependencies=1
+    have_fdatasync=1
 
     printf "Checking for $libdir_expanded in ldconfig... "
     ldconfig_paths=$(/sbin/ldconfig -N -X -v 2>/dev/null | sed -n -e 's,^\(/.*\):\( (.*)\)\?$,\1,p')
@@ -859,6 +861,9 @@ HAVE_D_TYPE = ${have_d_type}
 # Whether the Xapian version in use supports compaction
 HAVE_XAPIAN_COMPACT = ${have_xapian_compact}
 
+# Optimized fsync() on Linux
+HAVE_FDATASYNC = ${have_fdatasync}
+
 # Whether the getpwuid_r function is standards-compliant
 # (if not, then notmuch will #define _POSIX_PTHREAD_SEMANTICS
 # to enable the standards-compliant version -- needed for Solaris)
@@ -926,6 +931,7 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)      \\
 		   -DSTD_GETPWUID=\$(STD_GETPWUID)                       \\
 		   -DSTD_ASCTIME=\$(STD_ASCTIME)                         \\
 		   -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)	 \\
+		   -DHAVE_FDATASYNC=\$(HAVE_FDATASYNC)	                 \\
 		   -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
 
 CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
@@ -939,6 +945,7 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS)    \\
 		     -DSTD_GETPWUID=\$(STD_GETPWUID)                     \\
 		     -DSTD_ASCTIME=\$(STD_ASCTIME)                       \\
 		     -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT)       \\
+		     -DHAVE_FDATASYNC=\$(HAVE_FDATASYNC)	         \\
 		     -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER)
 
 CONFIGURE_LDFLAGS =  \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(ZLIB_LDFLAGS) \$(XAPIAN_LDFLAGS)
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 2849eab..86b275a 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -169,7 +169,11 @@ notmuch_database_dump (notmuch_database_t *notmuch,
     }
 
     if (output_file_name) {
+#if HAVE_FDATASYNC
 	ret = fdatasync (outfd);
+#else
+	ret = fsync (outfd);
+#endif
 	if (ret) {
 	    fprintf (stderr, "Error syncing %s to disk: %s\n",
 		     name_for_error, strerror (errno));
-- 
1.9.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: Linux-only fdatasync() in 3c13bc
  2014-05-06 19:59 Linux-only fdatasync() in 3c13bc Xīcò
@ 2014-05-07 11:20 ` Kushal Kumaran
  2014-05-07 12:46   ` Tomi Ollila
  0 siblings, 1 reply; 8+ messages in thread
From: Kushal Kumaran @ 2014-05-07 11:20 UTC (permalink / raw)
  To: notmuch

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

Xīcò <xico@atelo.org> writes:

> Also, commit 3c13bc introduced a call to fdatasync() which is not
> available on FreeBSD, and probably not either on MacOS at least.
>

fdatasync is POSIX:
http://pubs.opengroup.org/onlinepubs/009695399/functions/fdatasync.html

-- 
regards,
kushal

[-- Attachment #2: Type: application/pgp-signature, Size: 472 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linux-only fdatasync() in 3c13bc
  2014-05-07 11:20 ` Kushal Kumaran
@ 2014-05-07 12:46   ` Tomi Ollila
  2014-05-07 13:17     ` David Bremner
  0 siblings, 1 reply; 8+ messages in thread
From: Tomi Ollila @ 2014-05-07 12:46 UTC (permalink / raw)
  To: Kushal Kumaran, notmuch

On Wed, May 07 2014, Kushal Kumaran <kushal.kumaran+notmuch@gmail.com> wrote:

> Xīcò <xico@atelo.org> writes:
>
>> Also, commit 3c13bc introduced a call to fdatasync() which is not
>> available on FreeBSD, and probably not either on MacOS at least.
>>
>
> fdatasync is POSIX:
> http://pubs.opengroup.org/onlinepubs/009695399/functions/fdatasync.html

No wonder it is problematic, then >;)

> -- 
> regards,
> kushal

Tomi

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linux-only fdatasync() in 3c13bc
  2014-05-07 12:46   ` Tomi Ollila
@ 2014-05-07 13:17     ` David Bremner
  2014-05-07 17:18       ` Austin Clements
  0 siblings, 1 reply; 8+ messages in thread
From: David Bremner @ 2014-05-07 13:17 UTC (permalink / raw)
  To: Tomi Ollila, Kushal Kumaran, notmuch

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Wed, May 07 2014, Kushal Kumaran <kushal.kumaran+notmuch@gmail.com> wrote:
>
>> Xīcò <xico@atelo.org> writes:
>>
>>> Also, commit 3c13bc introduced a call to fdatasync() which is not
>>> available on FreeBSD, and probably not either on MacOS at least.
>>>
>>
>> fdatasync is POSIX:
>> http://pubs.opengroup.org/onlinepubs/009695399/functions/fdatasync.html
>
> No wonder it is problematic, then >;)
>

I seem to recall Austin saying on IRC that this usage was guaranteed to
call fsync anyway. Comments Austin?

d

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Linux-only fdatasync() in 3c13bc
  2014-05-07 13:17     ` David Bremner
@ 2014-05-07 17:18       ` Austin Clements
  2014-05-08 11:57         ` [PATCH] notmuch-dump: use fsync instead of fdatasync David Bremner
  0 siblings, 1 reply; 8+ messages in thread
From: Austin Clements @ 2014-05-07 17:18 UTC (permalink / raw)
  To: David Bremner; +Cc: Tomi Ollila, notmuch

Quoth David Bremner on May 07 at 10:17 pm:
> Tomi Ollila <tomi.ollila@iki.fi> writes:
> 
> > On Wed, May 07 2014, Kushal Kumaran <kushal.kumaran+notmuch@gmail.com> wrote:
> >
> >> Xīcò <xico@atelo.org> writes:
> >>
> >>> Also, commit 3c13bc introduced a call to fdatasync() which is not
> >>> available on FreeBSD, and probably not either on MacOS at least.
> >>>
> >>
> >> fdatasync is POSIX:
> >> http://pubs.opengroup.org/onlinepubs/009695399/functions/fdatasync.html
> >
> > No wonder it is problematic, then >;)
> >
> 
> I seem to recall Austin saying on IRC that this usage was guaranteed to
> call fsync anyway. Comments Austin?

Yes, since the size of the file will have definitely changed, the
metadata will have to be flushed anyway, so using fdatasync here has
no advantage over using fsync.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] notmuch-dump: use fsync instead of fdatasync
  2014-05-07 17:18       ` Austin Clements
@ 2014-05-08 11:57         ` David Bremner
  2014-05-08 15:36           ` Tomi Ollila
  2014-05-17 21:46           ` David Bremner
  0 siblings, 2 replies; 8+ messages in thread
From: David Bremner @ 2014-05-08 11:57 UTC (permalink / raw)
  To: notmuch

Since the file size will have changed, there is no performance benefit
to calling fdatasync.  Somewhat surprisingly, using fdatasync
apparently causes portability problems on FreeBSD.
---
 notmuch-dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 2849eab..887a208 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -169,7 +169,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,
     }
 
     if (output_file_name) {
-	ret = fdatasync (outfd);
+	ret = fsync (outfd);
 	if (ret) {
 	    fprintf (stderr, "Error syncing %s to disk: %s\n",
 		     name_for_error, strerror (errno));
-- 
1.9.2

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] notmuch-dump: use fsync instead of fdatasync
  2014-05-08 11:57         ` [PATCH] notmuch-dump: use fsync instead of fdatasync David Bremner
@ 2014-05-08 15:36           ` Tomi Ollila
  2014-05-17 21:46           ` David Bremner
  1 sibling, 0 replies; 8+ messages in thread
From: Tomi Ollila @ 2014-05-08 15:36 UTC (permalink / raw)
  To: David Bremner, notmuch

On Thu, May 08 2014, David Bremner <david@tethera.net> wrote:

> Since the file size will have changed, there is no performance benefit
> to calling fdatasync.  Somewhat surprisingly, using fdatasync
> apparently causes portability problems on FreeBSD.
> ---

Well, this is easy to review! LGTM.

Tomi


>  notmuch-dump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/notmuch-dump.c b/notmuch-dump.c
> index 2849eab..887a208 100644
> --- a/notmuch-dump.c
> +++ b/notmuch-dump.c
> @@ -169,7 +169,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,
>      }
>  
>      if (output_file_name) {
> -	ret = fdatasync (outfd);
> +	ret = fsync (outfd);
>  	if (ret) {
>  	    fprintf (stderr, "Error syncing %s to disk: %s\n",
>  		     name_for_error, strerror (errno));
> -- 
> 1.9.2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] notmuch-dump: use fsync instead of fdatasync
  2014-05-08 11:57         ` [PATCH] notmuch-dump: use fsync instead of fdatasync David Bremner
  2014-05-08 15:36           ` Tomi Ollila
@ 2014-05-17 21:46           ` David Bremner
  1 sibling, 0 replies; 8+ messages in thread
From: David Bremner @ 2014-05-17 21:46 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> Since the file size will have changed, there is no performance benefit
> to calling fdatasync.  Somewhat surprisingly, using fdatasync
> apparently causes portability problems on FreeBSD.
> ---

pushed to release and master.

d

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-05-17 21:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-06 19:59 Linux-only fdatasync() in 3c13bc Xīcò
2014-05-07 11:20 ` Kushal Kumaran
2014-05-07 12:46   ` Tomi Ollila
2014-05-07 13:17     ` David Bremner
2014-05-07 17:18       ` Austin Clements
2014-05-08 11:57         ` [PATCH] notmuch-dump: use fsync instead of fdatasync David Bremner
2014-05-08 15:36           ` Tomi Ollila
2014-05-17 21:46           ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.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).