Subject: Re: fsync_range() system call
To: Bill Studenmund <wrstuden@netbsd.org>
From: Klaus Klein <kleink@reziprozitaet.de>
List: tech-kern
Date: 10/25/2003 03:53:30
On Saturday 25 October 2003 00:23, Bill Studenmund wrote:
> Index: include/unistd.h
> ===================================================================
> RCS file: /cvsroot/wasabisrc/src/include/unistd.h,v
> retrieving revision 1.1.1.4
> diff -u -r1.1.1.4 unistd.h
> --- include/unistd.h 12 May 2003 20:51:02 -0000 1.1.1.4
> +++ include/unistd.h 17 Oct 2003 18:29:50 -0000
> @@ -311,6 +314,7 @@
> void endusershell __P((void));
> int exect __P((const char *, char * const *, char * const *));
> int fchroot __P((int));
> +int fsync_range(int, int, off_t, off_t);
> int getdomainname __P((char *, size_t));
> int getgrouplist __P((const char *, gid_t, gid_t *, int *));
> mode_t getmode __P((const void *, mode_t));
You shoudln't mix K&R-escaped with ANSI within the same file.
> Index: lib/libc/sys/fsync.2
> ===================================================================
> RCS file: /cvsroot/wasabisrc/src/lib/libc/sys/fsync.2,v
> retrieving revision 1.1.1.4
> diff -u -r1.1.1.4 fsync.2
> --- lib/libc/sys/fsync.2 17 Apr 2003 05:21:21 -0000 1.1.1.4
> +++ lib/libc/sys/fsync.2 17 Oct 2003 18:29:54 -0000
> @@ -38,6 +38,7 @@
> .Os
> .Sh NAME
> .Nm fsync
> +.Nm fsync_range
> .Nd "synchronize a file's in-core state with that on disk"
> .Sh LIBRARY
> .Lb libc
This should be ".Nm fsync ,".
> Index: lib/libpthread/pthread_cancelstub.c
> ===================================================================
> RCS file: /cvsroot/wasabisrc/src/lib/libpthread/pthread_cancelstub.c,v
> retrieving revision 1.1.1.7
> diff -u -r1.1.1.7 pthread_cancelstub.c
> --- lib/libpthread/pthread_cancelstub.c 11 Mar 2003 02:46:28 -0000 1.1.1.7
> +++ lib/libpthread/pthread_cancelstub.c 17 Oct 2003 18:29:56 -0000
> @@ -69,6 +69,7 @@
> @@ -157,6 +158,20 @@
> return retval;
> }
>
> +int
> +_fsync_range(int d, int f, off_t s, off_t e)
> +{
> + int retval;
> + pthread_t self;
> +
> + self = pthread__self();
> + pthread__testcancel(self);
> + retval = _sys_fsync(d, f, s, e);
> + pthread__testcancel(self);
> +
> + return retval;
> +}
> +
> ssize_t
> msgrcv(int msgid, void *msgp, size_t msgsz, long msgtyp, int msgflg)
> {
I think you want to call _sys_fsync_range(), here.
> Index: sys/kern/vfs_syscalls.c
> ===================================================================
> RCS file: /cvsroot/wasabisrc/src/sys/kern/vfs_syscalls.c,v
> retrieving revision 1.1.1.10
> diff -u -r1.1.1.10 vfs_syscalls.c
> --- sys/kern/vfs_syscalls.c 24 May 2003 16:16:42 -0000 1.1.1.10
> +++ sys/kern/vfs_syscalls.c 17 Oct 2003 18:30:30 -0000
> @@ -2867,6 +2867,76 @@
[...]
> +
> + if ((fp->f_flag & FWRITE) == 0) {
> + // simple_unlock(&fp->f_slock);
That looks like a leftover, given getvnode()'s implied FILE_USE().
However, you've made me aware of a conformance deficiency in fdatasync(2).
:-)
> + flags = SCARG(uap, flags);
> + if ((flags & (FDATASYNC | FFILESYNC)) == 0) {
> + /* Do we care? */
> + }
We should (EINVAL); for a precedent, look at e.g. mlockall(2).
As a general observation, I think the FSYNC_DATAONLY flag name/value should
be recycled for FSYNC_DATASYNC semantics; the current FSYNC_DATAONLY
semantics are too aggressive for its (and fdatasync(2)'s) own good.
- Klaus