Re: [PATCH v2 2/4] ovl: stash upper real file in backing_file struct

[Al Viro <viro@zeniv.linux.org.uk>]

On Sun, Oct 06, 2024 at 10:04:26PM +0100, Al Viro wrote:
> On Sun, Oct 06, 2024 at 10:23:57AM +0200, Amir Goldstein wrote:
> > +	/*
> > +	 * Usually, if we operated on a stashed upperfile once, all following
> > +	 * operations will operate on the stashed upperfile, but there is one
> > +	 * exception - ovl_fsync(datasync = false) can populate the stashed
> > +	 * upperfile to perform fsync on upper metadata inode.  In this case,
> > +	 * following read/write operations will not use the stashed upperfile.
> > +	 */
> > +	if (upperfile && likely(ovl_is_real_file(upperfile, realpath))) {
> > +		realfile = upperfile;
> > +		goto checkflags;
> >  	}
> >  
> > +	/*
> > +	 * If realfile is lower and has been copied up since we'd opened it,
> > +	 * open the real upper file and stash it in backing_file_private().
> > +	 */
> > +	if (unlikely(!ovl_is_real_file(realfile, realpath))) {
> > +		struct file *old;
> > +
> > +		/* Either stashed realfile or upperfile must match realinode */
> > +		if (WARN_ON_ONCE(upperfile))
> > +			return -EIO;
> > +
> > +		upperfile = ovl_open_realfile(file, realpath);
> > +		if (IS_ERR(upperfile))
> > +			return PTR_ERR(upperfile);
> > +
> > +		old = cmpxchg_release(backing_file_private_ptr(realfile), NULL,
> > +				      upperfile);
> > +		if (old) {
> > +			fput(upperfile);
> > +			upperfile = old;
> > +		}
> > +
> > +		/* Stashed upperfile that won the race must match realinode */
> > +		if (WARN_ON_ONCE(!ovl_is_real_file(upperfile, realpath)))
> > +			return -EIO;
> > +
> > +		realfile = upperfile;
> > +	}
> > +
> > +checkflags:
> 
> Hmm...  That still feels awkward.  Question: can we reach that code with
> 	* non-NULL upperfile
> 	* false ovl_is_real_file(upperfile, realpath)
> 	* true ovl_is_real_file(realfile, realpath)
> Is that really possible?

read() from metacopied file after fsync(), with the data still in lower
layer?  Or am I misreading that?