Dear Greg and Paul, On 2020-03-25, Paul Ripke wrote: > On Mon, Mar 16, 2020 at 08:47:27AM -0400, Greg Troxel wrote: > > [lots of test reports about fdatasync patch] > > > > Thanks -- that's enough for me to be comfortable. > > and it's been proposed for more than long enough, with no adverse > > comments, so I'll commit it soonish. > > fwiw, I missed a comment at the top of the function... fixed in > attached patch. > > -- > Paul Ripke > "Great minds discuss ideas, average minds discuss events, small minds > discuss people." > -- Disputed: Often attributed to Eleanor Roosevelt. 1948.
> diff --git a/lib/libc/sys/fdatasync.2 b/lib/libc/sys/fdatasync.2 > index 3f12119f0dbb..20da609191f5 100644 > --- a/lib/libc/sys/fdatasync.2 > +++ b/lib/libc/sys/fdatasync.2 > @@ -68,7 +68,7 @@ function will fail if: > .It Bq Er EBADF > The > .Fa fd > -argument is not a valid file descriptor open for writing. > +argument is not a valid file descriptor. > .It Bq Er EINVAL > This implementation does not support synchronized I/O for this file. > .It Bq Er ENOSYS > @@ -93,4 +93,4 @@ and outstanding I/O operations are not guaranteed to have > been completed. > The > .Fn fdatasync > function conforms to > -.St -p1003.1b-93 . > +.St -p1003.1-2008 . > diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c > index d51beedbfca9..8cfe5abe6cf8 100644 > --- a/sys/kern/vfs_syscalls.c > +++ b/sys/kern/vfs_syscalls.c > @@ -4059,8 +4059,7 @@ sys_fsync(struct lwp *l, const struct sys_fsync_args > *uap, register_t *retval) > * Sync a range of file data. API modeled after that found in AIX. > * > * FDATASYNC indicates that we need only save enough metadata to be able > - * to re-read the written data. Note we duplicate AIX's requirement that > - * the file be open for writing. > + * to re-read the written data. > */ > /* ARGSUSED */ > int > @@ -4141,10 +4140,6 @@ sys_fdatasync(struct lwp *l, const struct > sys_fdatasync_args *uap, register_t *r > /* fd_getvnode() will use the descriptor for us */ > if ((error = fd_getvnode(SCARG(uap, fd), &fp)) != 0) > return (error); > - if ((fp->f_flag & FWRITE) == 0) { > - fd_putfile(SCARG(uap, fd)); > - return (EBADF); > - } > vp = fp->f_vnode; > vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); > error = VOP_FSYNC(vp, fp->f_cred, FSYNC_WAIT|FSYNC_DATAONLY, 0, 0); On 2020-03-25, Greg Troxel wrote: > Paul Ripke <s...@stix.id.au> writes: > > > On Mon, Mar 16, 2020 at 08:47:27AM -0400, Greg Troxel wrote: > >> [lots of test reports about fdatasync patch] > >> > >> Thanks -- that's enough for me to be comfortable. > >> and it's been proposed for more than long enough, with no adverse > >> comments, so I'll commit it soonish. > > > fwiw, I missed a comment at the top of the function... fixed in > > attached patch. > > I have committed your patch, exactly as you just sent it. My full > release build worked and I have an anita test run in process, just in > case. > > Thanks for perservering on this. It takes many people to fix all the > loose ends in an operating system! I have been trying to find the cause of PR kern/55237. I am not at all familiar with the code, so please forgive me for pointing fingers! I think the call to fd_putfile results in a close of the fd, but that does not happen anymore? Should it? Apologies again if this has nothing to do with kern/55237. -- Kind regards, Yorick Hardy