On Thu, Nov 24, 2016 at 3:51 PM, Miklos Szeredi <mik...@szeredi.hu> wrote: > On Thu, Nov 24, 2016 at 2:12 PM, Amir Goldstein <amir7...@gmail.com> wrote: >> On Thu, Nov 24, 2016 at 2:03 PM, Miklos Szeredi <mik...@szeredi.hu> wrote: >>> On Thu, Nov 24, 2016 at 12:52 PM, Amir Goldstein <amir7...@gmail.com> wrote: >>>> On Thu, Nov 24, 2016 at 12:55 PM, Miklos Szeredi <mszer...@redhat.com> >>>> wrote: >>> >>>>> + /* >>>>> + * These should be intercepted, but they are very >>>>> unlikely to be >>>>> + * a problem in practice. Leave them alone for now. >>>> >>>> It could also be handled in vfs helpers. >>>> Since these ops all start with establishing that src and dest are on >>>> the same sb, >>>> then the cost of copy up of src is the cost of clone_file_range from >>>> lower to upper, >>>> so it is probably worth to copy up src and leave those fops alone. >>>> >>>>> + */ >>>>> + ofop->fops.copy_file_range = orig->copy_file_range; >>>>> + ofop->fops.clone_file_range = orig->clone_file_range; >>>>> + ofop->fops.dedupe_file_range = orig->dedupe_file_range; >>> >>> Not sure I understand. Why should we copy up src? Copy up is the >>> problem not the solution. >>> >> >> Maybe the idea is ill conceived, but the reasoning is: >> To avoid the corner case of cloning from a stale lower src, >> call d_real() in vfs helpers to always copy up src before cloning from it >> and pass the correct file onwards. > > Which correct file? src is still the wrong one after calling d_real. > We need to clone-open src, just like we do in ovl_read_iter to get the > correct file. But then what's the use of copying it up beforehand? > > We could move the whole logic into the vfs, but I don't really see the point. > > I left these ops alone because there is some confusion in there about > getting the f_op from the source or the destination file.
Yes, I saw that. Shouldn't be a problem to always use src->f_ops-> IMO. Could you please push this work to a topic branch to make it easier for me to pull and test? Thanks, Amir. > And while > it doesn't matter normally (all regular files have the same f_op, > regardless of open flags) it does matter for overlayfs intercept, > because overriding fops in the the dest file would mean additional > complexity and resources). That could easily be fixed in the vfs: > calling src->f_ops->foo works equally well, but I simply didn't want > to bother with this. We can return to it later. > > Thanks, > Miklos
