On Mon, Apr 2, 2018 at 11:29 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> On Fri, Mar 30, 2018 at 01:02:32PM +0300, Amir Goldstein wrote:
>> On Thu, Mar 29, 2018 at 10:38 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>> > Right now we rely on path based lookup for data origin of metacopy upper.
>> > This will work only if upper has not been renamed. We solved this problem
>> > already for merged directories using redirect. Use same logic for metacopy
>> > files.
>> >
>> > This patch just goes on to check redirects for metacopy files.
>> >
>>
>> The patch changes the logic very subtly, but it is really hard to
>> follow because of convoluted diff. Please make changes that don't
>> change logic in a separate patch.
>>
>> > Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
>> > ---
>> > fs/overlayfs/namei.c | 24 ++++++++++++------------
>> > 1 file changed, 12 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
>> > index 1dba89e9543f..a7a9588f64b7 100644
>> > --- a/fs/overlayfs/namei.c
>> > +++ b/fs/overlayfs/namei.c
>> > @@ -260,18 +260,19 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
>> > goto out_err;
>> > d->stop = !err;
>> > d->metacopy = !!err;
>> > - goto out;
>> > - }
>> > - if (last_element)
>> > - d->is_dir = true;
>> > - if (d->last)
>> > - goto out;
>> > -
>> > - if (ovl_is_opaquedir(this)) {
>> > - d->stop = true;
>> > + if (!d->metacopy)
>> > + goto out;
>> > + } else {
>> > if (last_element)
>> > - d->opaque = true;
>> > - goto out;
>> > + d->is_dir = true;
>> > + if (d->last)
>> > + goto out;
>>
>> If I am not mistaken, d->last test is relevant to both did and non-dir,
>> because it also prevents the unneeded check redirect.
>
> It is just an optimization which we do only for directories as of now
> and this patch does not change that behavior. For non-dirs we never
> check redirects anyway as of now.
This is a moot argument.
Of course this patch changes behavior.
If adds the ability to redirect non-dir and it should add the same logic as
for dir.
>
> One could argue that this optimization should be introduced for metacopy
> files as well. I could do that. Just that I did not focus too much
> on optimizations a lot. This series has been growing and dragging
> enough, that I want to focus on correctness first. And worry about
> optimizations later.
I hear your frustration, but I think the patch set is progressing very nicely
(and got a few prep patches already in overlayfs-next), so this isn't going
to break you ;-)
if (!d->metacopy || d->last)
goto out;
Thanks,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
