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

[Yorick Hardy <yorickhardy%gmail.com@localhost>]

Thanks!

On 2020-05-24, Paul Ripke wrote:
> 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.

-- 
Kind regards,

Yorick Hardy