Re: [PATCH 4/7] ovl: add infrastructure for intercepting file ops

[Amir Goldstein]

On Thu, Nov 24, 2016 at 2:03 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> On Thu, Nov 24, 2016 at 12:52 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>> On Thu, Nov 24, 2016 at 12:55 PM, Miklos Szeredi <mszeredi@xxxxxxxxxx> 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.

It would have been crazy if we suggested the same for read_iter(),
so why it may make sense for clone?
because once you got that far into the vfs_clone_range() helper,
with src from lower and dst from upper, it means they are on the same sb
that supports clone, so it is likely that copy up is going to use clone and
be relatively cheap.

Pretty twisted. I agree... and not sure if this logic holds for copy_range as
well. Anyway, that's what I meant.

Amir.