On Fri, Oct 04, 2024 at 11:16:25PM +0100, Al Viro wrote: > On Fri, Oct 04, 2024 at 12:23:39PM +0200, Amir Goldstein wrote: > > ovl_fsync() with !datasync opens a backing file from the top most dentry > > in the stack, checks if this dentry is non-upper and skips the fsync. > > > > In case of an overlay dentry stack with lower data and lower metadata > > above it, but without an upper metadata above it, the backing file is > > opened from the top most lower metadata dentry and never used. > > > > Fix the helper ovl_real_fdget_meta() to return an empty struct fd in > > that case to avoid the unneeded backing file open. > > Umm... Won't that screw the callers of ovl_real_fd()? > > I mean, here > > > @@ -395,7 +397,7 @@ static int ovl_fsync(struct file *file, loff_t start, loff_t end, int datasync) > > return ret; > > > > ret = ovl_real_fdget_meta(file, &real, !datasync); > > - if (ret) > > + if (ret || fd_empty(real)) > > return ret; > > > > you are taking account of that, but what of e.g. > ret = ovl_real_fdget(file, &real); > if (ret) > return ret; > > /* > * Overlay file f_pos is the master copy that is preserved > * through copy up and modified on read/write, but only real > * fs knows how to SEEK_HOLE/SEEK_DATA and real fs may impose > * limitations that are more strict than ->s_maxbytes for specific > * files, so we use the real file to perform seeks. > */ > ovl_inode_lock(inode); > fd_file(real)->f_pos = file->f_pos; > in ovl_llseek()? Get ovl_real_fdget_meta() called by ovl_real_fdget() and > have it return 0 with NULL in fd_file(real), and you've got an oops right > there, don't you? I see... so you rely upon that thing never happening when the last argument of ovl_real_fdget_meta() is false, including the call from ovl_real_fdget(). I still don't like the calling conventions, TBH. Let me think a bit...
