Re: PATCH: Relax fdatasync checks to IEEE Std 1003.1-2008

[Paul Ripke <stix%stix.id.au@localhost>]

On Sun, May 24, 2020 at 06:56:54AM -0400, Greg Troxel wrote:
> Yorick Hardy <yorickhardy%gmail.com@localhost> writes:
> 
> (I realize you later say this isn't it.)
> 
> >> @@ -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);
> 
> If you look at the function beyond what's in the diff, you will see (I
> think, but I really mean I see) that there is always a single
> fd_putfile.  This was just doing the put before returning, rather than
> setting error and the usaul "goto out" where the end-of-routine cleanup
> happens.  See also sys_fsync_range() in the same file.
> 
> I could be reading this wrong.

I concur - this was just the fd_putfile to match the fd_getfile in
the early error path for read-only files. The code now falls through
and calls fd_putfile regardless, to remove the fd reference.

There should be no real behaviour change here apart from a relaxing
of the success conditions.

-- 
Paul Ripke
"Great minds discuss ideas, average minds discuss events, small minds
 discuss people."
-- Disputed: Often attributed to Eleanor Roosevelt. 1948.