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