From owner-freebsd-standards@FreeBSD.ORG Tue Apr 19 04:10:26 2005 Return-Path: Delivered-To: freebsd-standards@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 07CE016A4CE for ; Tue, 19 Apr 2005 04:10:26 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id BD93F43D4C for ; Tue, 19 Apr 2005 04:10:25 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.3/8.13.3) with ESMTP id j3J4APEY037095 for ; Tue, 19 Apr 2005 04:10:25 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.3/8.13.1/Submit) id j3J4APhr037094; Tue, 19 Apr 2005 04:10:25 GMT (envelope-from gnats) Date: Tue, 19 Apr 2005 04:10:25 GMT Message-Id: <200504190410.j3J4APhr037094@freefall.freebsd.org> To: freebsd-standards@FreeBSD.org From: David Schultz Subject: Re: kern/64875: Add a system call: fdatasync() X-BeenThere: freebsd-standards@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: David Schultz List-Id: Standards compliance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Apr 2005 04:10:26 -0000 The following reply was made to PR kern/64875; it has been noted by GNATS. From: David Schultz To: Kevin Lo Cc: freebsd-gnats-submit@FreeBSD.ORG, alc@FreeBSD.ORG Subject: Re: kern/64875: Add a system call: fdatasync() Date: Tue, 19 Apr 2005 00:07:48 -0400 On Sun, Mar 28, 2004, Kevin Lo wrote: > >Number: 64875 > >Category: kern > >Synopsis: Add a system call: fdatasync() [...] > >Description: > fdatasync()is part of realtime extensions in POSIX 1003.1. > > DESCRIPTION > The fdatasync() function forces all currently queued I/O > operations associated with the file indicated by file > descriptor fildes to the synchronized I/O completion state. [...] > +int > +fdatasync(td, uap) > + struct thread *td; > + struct fsync_args /* { > + int fd; > + } */ *uap; > +{ > + struct vnode *vp; > + struct file *fp; > + vm_object_t obj; > + int error; > + > + GIANT_REQUIRED; > + > + if ((error = getvnode(td->td_proc->p_fd, uap->fd, &fp)) != 0) > + return (error); > + if ((fp->f_flag & FWRITE) == 0) { > + fdrop(fp, td); > + return (EBADF); > + } > + vp = fp->f_vnode; > + vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td); > + if (VOP_GETVOBJECT(vp, &obj) == 0) { > + VM_OBJECT_LOCK(obj); > + vm_object_page_clean(obj, 0, 0, 0); > + VM_OBJECT_UNLOCK(obj); > + } > + error = VOP_FSYNC(vp, fp->f_cred, MNT_WAIT, td); > + VOP_UNLOCK(vp, 0, td); > + fdrop(fp, td); > + return (error); > +} This is a good start, but there are several problems with the patch. - The above is basically the same as fsync(). The whole point of fdatasync() is to be more efficient than fsync() by not syncing the file metadata, so it's misleading to implement it as an alias for fsync(). I think a correct implementation could be achieved by omitting the VOP_FSYNC() call and passing the OBJPC_SYNC flag to vm_object_page_clean(), but Alan should review this. - Regarding the above, what's supposed to happen if the file size changes, or if the file was recently created, and its inode has not been written at all yet? I'm not sure whether fdatasync() is supposed to omit all metadata updates or just `unimportant' ones like atime and mtime. If my suggestion above isn't adequate, the VOP_FSYNC() interface may need to be extended. - The proposed documentation for fdatasync() needs to be rewritten, for two reasons. First, although we do have permission to copy text from POSIX, it's still plagiarism to do so without acknowledgement. Second, the POSIX documentation is not very helpful in this case; what's the ``synchronized I/O completion state'' anyway?