Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Miklos Szeredi <miklos@szeredi.hu>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jeff Layton <jlayton@poochiereds.net>,
	"J . Bruce Fields" <bfields@fieldses.org>,
	overlayfs <linux-unionfs@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2 10/17] ovl: decode lower file handles of unlinked but open files
Date: Tue, 16 Jan 2018 12:07:35 +0100	[thread overview]
Message-ID: <CAJfpegv0s_ptn-YA2zXgcgE3muWSyOZ2ipA=sTx=AUsS8KR5Sw@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxjeesvaciCuj1PY0zM3-CEV46q1bgxx1+98o6UW1NwXvg@mail.gmail.com>

On Tue, Jan 16, 2018 at 11:40 AM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Jan 16, 2018 at 12:10 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Tue, Jan 16, 2018 at 10:37 AM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> On Tue, Jan 16, 2018 at 11:16 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>> On Thu, Jan 4, 2018 at 6:20 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>>>> Lookup overlay inode in cache by origin inode, so we can decode a file
>>>>> handle of an open file even if the index has a whiteout index entry to
>>>>> mark this overlay inode was unlinked.
>>>>>
>>>>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>>>>> ---
>>>>>  fs/overlayfs/export.c    | 22 ++++++++++++++++++++--
>>>>>  fs/overlayfs/inode.c     | 16 ++++++++++++++++
>>>>>  fs/overlayfs/overlayfs.h |  1 +
>>>>>  3 files changed, 37 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
>>>>> index 602bada474ba..6ecb54d4b52c 100644
>>>>> --- a/fs/overlayfs/export.c
>>>>> +++ b/fs/overlayfs/export.c
>>>>> @@ -385,13 +385,21 @@ static struct dentry *ovl_lower_fh_to_d(struct super_block *sb,
>>>>>         struct ovl_path *stack = &origin;
>>>>>         struct dentry *dentry = NULL;
>>>>>         struct dentry *index = NULL;
>>>>> +       struct inode *inode = NULL;
>>>>> +       bool is_deleted = false;
>>>>>         int err;
>>>>>
>>>>>         /* First lookup indexed upper by fh */
>>>>
>>>> Why not first look up origin, then look up ovl inode by origin?  It
>>>> seems a faster path than going through the index first.  Obviously if
>>>> icache lookup fails then we need to look up index, but the common case
>>>> will the cached one, so that should be the fast one, no?
>>>>
>>>
>>> Not really, because we do not know if the file handle is dir or non-dir.
>>> If file handle is dir than decode of file handle is expensive and can
>>> reduce worst case from two file handle decodes to just one:
>>>
>>> For lower dir:
>>> - one index lookup fails
>>> - one lower dir decode
>>> - one icache lookup
>>> - maybe one ovl_lookup_real(is_upper=false)
>>>
>>> For copied up indexed dir:
>>> - one index lookup success
>>> - one upper dir decode
>>> - one ovl_lookup_real(is_upper=true)
>>>
>>> That method avoids the origin dir decode for upper indexed
>>> dir at the cost of not looking for the decoded dir in icache.
>>>
>>> How about this as in idea: hash overlay inodes for NFS export
>>> by origin fh instead of by origin inode pointer.
>>
>> Good idea.  That way we can leave out the middleman (underlying fh
>> decode) in the cached case.
>>
>>> We can also avoid the "lookup index first" for non-dir
>>> if we set a flag OVL_FH_FLAG_CONNECTABLE on exported
>>> dir file handle, but my thinking was trying to keep the first version
>>> simple with as fewer special cases as possible.
>>
>> Not sure I understand.  If cached lookup fails, then we do always need
>> to try and lookup index first before falling back to decoding origin,
>> right?
>>
>
> If you are referring to cache lookup by origin fh, then yes.
> If icache by origin fh lookup fails, we should lookup index to check
> for whiteout, before we decode origin fh, because index lookup is
> cheaper than reconnecting a connectable file handle decode.
>
> If we had marked the file handle 'non-connectable', then for non-dir
> non-connectable file handles, origin decode is actually slightly
> faster than index lookup, but I don't think it is worth the special
> casing and marking the file handle for the corner case, right?

My point is: if icache lookup fails, then for origin handles we always
have to do an index lookup to find the current version overlay object.
So no point in doing the origin decode first, since that one may not
be needed (if index is a whiteout).

Thanks,
Miklos

  reply	other threads:[~2018-01-16 11:07 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-04 17:20 [PATCH v2 00/17] Overlayfs NFS export support Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 01/17] ovl: document NFS export Amir Goldstein
2018-01-11 16:06   ` Miklos Szeredi
2018-01-11 16:26     ` Amir Goldstein
2018-01-12 15:43       ` Miklos Szeredi
2018-01-12 15:49         ` Miklos Szeredi
2018-01-12 18:50           ` Amir Goldstein
2018-01-13  8:54           ` Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 02/17] ovl: encode pure upper file handles Amir Goldstein
2018-01-18 10:31   ` Miklos Szeredi
2018-01-04 17:20 ` [PATCH v2 03/17] ovl: decode " Amir Goldstein
2018-01-18 14:09   ` Miklos Szeredi
2018-01-18 14:34     ` Amir Goldstein
2018-01-18 14:39       ` Miklos Szeredi
2018-01-18 19:49         ` Amir Goldstein
2018-01-18 20:10           ` Miklos Szeredi
2018-01-18 20:35             ` Amir Goldstein
2018-01-18 22:57               ` Amir Goldstein
2018-01-19  0:23                 ` Amir Goldstein
2018-01-19 10:39                   ` Miklos Szeredi
2018-01-19 11:07                     ` Amir Goldstein
2018-01-19 20:10                       ` Amir Goldstein
2018-01-24 10:34                         ` Miklos Szeredi
2018-01-24 11:04                           ` Amir Goldstein
2018-01-24 11:18                             ` Amir Goldstein
2018-01-24 11:55                               ` Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 04/17] ovl: decode connected upper dir " Amir Goldstein
2018-01-05 12:33   ` Amir Goldstein
2018-01-05 15:18     ` J . Bruce Fields
2018-01-05 15:34       ` Amir Goldstein
2018-01-15 11:41     ` Miklos Szeredi
2018-01-15 11:33   ` Miklos Szeredi
2018-01-15 12:20     ` Amir Goldstein
2018-01-15 14:56       ` Miklos Szeredi
2018-01-17 11:18         ` Amir Goldstein
2018-01-17 12:20           ` Amir Goldstein
2018-01-17 13:29             ` Amir Goldstein
2018-01-17 15:42           ` Miklos Szeredi
2018-01-17 16:34             ` Amir Goldstein
2018-01-17 21:36               ` Amir Goldstein
2018-01-18  8:22               ` Miklos Szeredi
2018-01-18  8:47                 ` Amir Goldstein
2018-01-18  9:12                   ` Miklos Szeredi
2018-01-18 10:28                     ` Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 05/17] ovl: encode non-indexed upper " Amir Goldstein
2018-01-15 11:58   ` Miklos Szeredi
2018-01-15 12:07     ` Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 06/17] ovl: copy up before encoding dir file handle when ofs->numlower > 1 Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 07/17] ovl: encode lower file handles Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 08/17] ovl: decode lower non-dir " Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 09/17] ovl: decode indexed " Amir Goldstein
2018-01-18 13:11   ` Miklos Szeredi
2018-01-04 17:20 ` [PATCH v2 10/17] ovl: decode lower file handles of unlinked but open files Amir Goldstein
2018-01-16  9:16   ` Miklos Szeredi
2018-01-16  9:37     ` Amir Goldstein
2018-01-16 10:10       ` Miklos Szeredi
2018-01-16 10:40         ` Amir Goldstein
2018-01-16 11:07           ` Miklos Szeredi [this message]
2018-01-17 21:05         ` Amir Goldstein
2018-01-18 14:18   ` Amir Goldstein
2018-02-27 11:35     ` Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 11/17] ovl: decode indexed dir file handles Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 12/17] ovl: decode pure lower " Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 13/17] ovl: hash directory inodes for NFS export Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 14/17] ovl: lookup connected ancestor of dir in inode cache Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 15/17] ovl: lookup indexed ancestor of lower dir Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 16/17] ovl: wire up NFS export support Amir Goldstein
2018-01-04 17:20 ` [PATCH v2 17/17] nfsd: encode stat->mtime for getattr instead of inode->i_mtime 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='CAJfpegv0s_ptn-YA2zXgcgE3muWSyOZ2ipA=sTx=AUsS8KR5Sw@mail.gmail.com' \
    --to=miklos@szeredi.hu \
    --cc=amir73il@gmail.com \
    --cc=bfields@fieldses.org \
    --cc=jlayton@poochiereds.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --subject='Re: [PATCH v2 10/17] ovl: decode lower file handles of unlinked but open files' \
    /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

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