linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miklos Szeredi <miklos@szeredi.hu>
To: Amir Goldstein <amir73il@gmail.com>
Cc: "linux-unionfs@vger.kernel.org" <linux-unionfs@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [POC/RFC PATCH] overlayfs: constant inode numbers
Date: Tue, 29 Nov 2016 11:16:37 +0100	[thread overview]
Message-ID: <CAJfpeguTp4AwcnouMAse1=seG=LboW68eNfRjMiEpeaBjxuYQQ@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxgSHah2OpbgW2nrDXjwXob=_ypjz9dbmHUO-1QrfwooXg@mail.gmail.com>

On Mon, Nov 28, 2016 at 12:56 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Nov 28, 2016 at 12:35 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Mon, Nov 28, 2016 at 10:10 AM, Amir Goldstein <amir73il@gmail.com> wrote:

[...]

>>> 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?
>
> Right. I completely missed out on the rename case..
>
>> What about opaque ones?
>
> That's exactly the point of OVL_XATTR_INO IFF !OVL_XATTR_OPAQUE
> If you have OVL_XATTR_INO means entry cannot be opaque, so it should
> be safe to skip it

So that means if OVL_XATTR_OPAQUE is set then we don't need to check
OVL_XATTR_INO and if OVL_XATTR_INO is set we don't need to check
OVL_XATTR_OPAQUE.   But that doesn't help optimize readdir, because we
really want to check neither.


>> I do hope we can optimize directory reading, because doing lookup and
>> getxattr for all entries is going to hurt...
>>
>
> Possibly silly question:
> Do you know if programs really rely of d_ino from getdents to be sane/non-zero?
> And what are the implications of overlayfs readdir not exporting the real d_ino?

I don't know of any precedent, so it's a big unknown.

>> 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...

There *is* actually a case where both 'opaque' and 'ino' make sense:
when an empty merged dir is exchanged for an empty opaque one in
ovl_clear_empty().

> Well, not sure if you noticed my redirect_fh (rediect by file handle) work.
> If differs from redirect by path in 2 major ways:
> 1. Like OVL_XATTR_INO, redirect is set on copy up (but only for dirs)
> 2. Lookup is much simpler (and most likely faster) then full path lookup
>
> It would be trivial to set oe->ino of merged dir from lower most entry in
> ovl_lookup().

Okay.  In fact always using the handle looks like a better option
overall.  File handle should be unique for the lifetime of the
filesystem, while inode numbers may be reused.

Biggest drawback of the file handle based redirects is exactly that:
makes backing up the overlay basically impossible, since file handles
won't work after a backup + restore.  But as an optimization, in
addition to path based redirects it would work fine and provide a good
way to get the stable ino.

Thanks,
Miklos

  parent reply	other threads:[~2016-11-29 10:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-25 21:29 [POC/RFC PATCH] overlayfs: constant inode numbers Miklos Szeredi
     [not found] ` <CAOQ4uxgSvo_v37aLJb8FK3c5sDqCkN2XWOd783UXaomdVAvsEQ@mail.gmail.com>
     [not found]   ` <CAOQ4uxi2Ko-G2nA_bSWT8juuss9aS9ZfiBWS95RrdfBy30Tozg@mail.gmail.com>
2016-11-28  9:10     ` Fwd: " Amir Goldstein
2016-11-28 10:35       ` Miklos Szeredi
2016-11-28 11:56         ` Amir Goldstein
2016-11-28 18:02           ` Amir Goldstein
2016-11-29 10:16           ` Miklos Szeredi [this message]
2016-11-29 11:34             ` Amir Goldstein
2016-11-29 12:03               ` Amir Goldstein
2016-11-29 21:49                 ` Miklos Szeredi
2016-11-30 15:05                   ` Amir Goldstein
2016-11-30 16:36                     ` Amir Goldstein
2016-12-05 14:05                   ` Amir Goldstein

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAJfpeguTp4AwcnouMAse1=seG=LboW68eNfRjMiEpeaBjxuYQQ@mail.gmail.com' \
    --to=miklos@szeredi.hu \
    --cc=amir73il@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).