Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1531193

Re: [POC/RFC PATCH] overlayfs: constant inode numbers

Show all headers | View raw

On Mon, Nov 28, 2016 at 10:10 AM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> @@ -258,12 +268,12 @@ static int ovl_copy_up_locked(struct den
>>>         if (err)
>>>                 goto out_cleanup;
>>>
>>> -       inode_lock(newdentry->d_inode);
>>>         err = ovl_set_attr(newdentry, stat);
>>> -       inode_unlock(newdentry->d_inode);
>>>         if (err)
>>>                 goto out_cleanup;
>>>
>>> +       ovl_dentry_set_ino(dentry, stat->ino);
>>> +
>>
>>
>
> Shouldn't we propagate stored ino all the way
> from lowest layer to preserve ino across layer recycling?

Since OVL_XATTR_INO is set every time we copy-up, layer recycling
should work fine.

Exception is overlay root, where there's no copy-up, so no
preservation of ino.  Not sure what the right solution there is.

>
> Specifically, shouldn't ino of merged dir expose the lower most ino?

It should.


>>> @@ -353,11 +354,45 @@ static struct ovl_dir_cache *ovl_cache_g
>>>         return cache;
>>>  }
>>>
>>> +static int ovl_cache_entry_update_ino(struct dentry *dir,
>>> +                                     struct ovl_cache_entry *p)
>>> +
>>> +{
>>> +       struct dentry *this;
>>> +       struct dentry *base = ovl_dentry_at_idx(dir, p->idx);
>>> +
>>> +       if (p->name[0] == '.') {
>>> +               if (p->len == 1) {
>>> +                       this = dget(base);
>>> +                       goto get;
>>> +               }
>>> +               if (p->len == 2 && p->name[1] == '.') {
>>> +                       /* ♫ we shall not be moved */
>>> +                       this = dget(ovl_dentry_real(dir->d_parent));
>>> +                       goto get;
>>> +               }
>>> +       }
>>> +       this = lookup_one_len_unlocked(p->name, base, p->len);
>>> +       if (IS_ERR(this)) {
>>> +               pr_err("overlay: failed to look up (%s) for ino (%i)\n",
>>> +                      p->name, (int) PTR_ERR(this));
>>> +               return -EIO;
>>> +       }
>>>
>>> +       get:
>>> +       p->ino = p->orig_ino;
>>> +       ovl_get_ino(this, &p->ino);
>
> I may be way off here, but why do you need to lookup entry and get ino
> from xattr at all? Wouldn't it be easier to not add the entry to the list if
> it was copied up and rely on the fact that it will be added to list in iter
> of lower layer with original ino with no extra effort?

What about renamed entries?  What about opaque ones?

I do hope we can optimize directory reading, because doing lookup and
getxattr for all entries is going to hurt...

> For that matter, I like the fact that every copied up entry will be explicitly
> marked with OVL_XATTR_INO. In a way, it is the opposite of
> OVL_XATTR_OPAQUE and if the former becomes a standard, the latter
> will become redundant. Arguably, it is preferred to mark the copy ups
> as special case rather then the pure upper files, which can then stay 'pure'.

Maybe.


>>> @@ -502,7 +549,8 @@ static int ovl_dir_open(struct inode *in
>>>                 return PTR_ERR(realfile);
>>>         }
>>>         od->realfile = realfile;
>>> -       od->is_real = !OVL_TYPE_MERGE(type);
>>> +//     od->is_real = !OVL_TYPE_MERGE(type);
>>> +       od->is_real = false;
>
>
> A major change of framing would be to treat regular file entries as merged
> if they have been ever copied up and opaque only if they are pure upper.
> Same as dirs.
>
> This would also allow keeping ino stable across rename/redirect of regular
> files. Not sure if any programs rely on that??

We do keep ino stable across rename.  We don't keep ino stable across
copy-up.  That's what this patch is trying to address.

You are saying that we should have redirects for non-dir and drop
OVL_XATTR_INO?  That's another option, but it doesn't look like it
would simplify things...

Thanks for the revirew.

Pushed patch to #overlayfs-constino (needs work but it's worth testing).

Miklos

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[POC/RFC PATCH] overlayfs: constant inode numbers Miklos Szeredi <miklos@szeredi.hu> - 2016-11-25 22:40 +0100
  Fwd: [POC/RFC PATCH] overlayfs: constant inode numbers Amir Goldstein <amir73il@gmail.com> - 2016-11-28 10:20 +0100
    Re: [POC/RFC PATCH] overlayfs: constant inode numbers Miklos Szeredi <miklos@szeredi.hu> - 2016-11-28 11:40 +0100
      Re: [POC/RFC PATCH] overlayfs: constant inode numbers Amir Goldstein <amir73il@gmail.com> - 2016-11-28 13:00 +0100
        Re: [POC/RFC PATCH] overlayfs: constant inode numbers Amir Goldstein <amir73il@gmail.com> - 2016-11-28 19:10 +0100
        Re: [POC/RFC PATCH] overlayfs: constant inode numbers Miklos Szeredi <miklos@szeredi.hu> - 2016-11-29 11:20 +0100
          Re: [POC/RFC PATCH] overlayfs: constant inode numbers Amir Goldstein <amir73il@gmail.com> - 2016-11-29 12:40 +0100
            Re: [POC/RFC PATCH] overlayfs: constant inode numbers Amir Goldstein <amir73il@gmail.com> - 2016-11-29 14:10 +0100
              Re: [POC/RFC PATCH] overlayfs: constant inode numbers Miklos Szeredi <miklos@szeredi.hu> - 2016-11-29 23:00 +0100
                Re: [POC/RFC PATCH] overlayfs: constant inode numbers Amir Goldstein <amir73il@gmail.com> - 2016-11-30 16:10 +0100
                Re: [POC/RFC PATCH] overlayfs: constant inode numbers Amir Goldstein <amir73il@gmail.com> - 2016-11-30 17:40 +0100
                Re: [POC/RFC PATCH] overlayfs: constant inode numbers Amir Goldstein <amir73il@gmail.com> - 2016-12-05 15:10 +0100

csiph-web