All of lore.kernel.org
 help / color / mirror / Atom feed
From: Trond Myklebust <trondmy@primarydata.com>
To: "amir73il@gmail.com" <amir73il@gmail.com>,
	"jlayton@kernel.org" <jlayton@kernel.org>
Cc: "bfields@fieldses.org" <bfields@fieldses.org>,
	"miklos@szeredi.hu" <miklos@szeredi.hu>,
	"eddiehorng.tw@gmail.com" <eddiehorng.tw@gmail.com>,
	"linux-unionfs@vger.kernel.org" <linux-unionfs@vger.kernel.org>
Subject: Re: readdir returns d_type=DT_UNKNOWN to overlay exported dir (NFSv3)
Date: Wed, 14 Mar 2018 15:06:23 +0000	[thread overview]
Message-ID: <1521039981.40526.11.camel@primarydata.com> (raw)
In-Reply-To: <CAOQ4uxig8aM2WnVQniAXWBOWtnv8ciqqBWXZZgG-foVW04bofQ@mail.gmail.com>

On Wed, 2018-03-14 at 17:03 +0200, Amir Goldstein wrote:
> On Wed, Mar 14, 2018 at 4:30 PM, Jeff Layton <jlayton@kernel.org>
> wrote:
> > On Wed, 2018-03-14 at 14:16 +0000, Trond Myklebust wrote:
> > > On Wed, 2018-03-14 at 07:02 -0400, Jeff Layton wrote:
> > > > On Wed, 2018-03-14 at 16:42 +0800, Eddie Horng wrote:
> > > > > Hi Amir,
> > > > > Since the flock issue is clarified, I would like to start
> > > > > this new
> > > > > thread to discuss if we can find the cause of
> > > > > d_type=DT_UNKNOWN.
> > > > > First
> > > > 
> > > > This sounds like NOTABUG to me. As readdir(3) states:
> > > > 
> > > > Currently, only some filesystems (among them: Btrfs, ext2,
> > > > ext3,
> > > > and ext4) have full
> > > > support  for  returning  the  file  type  in
> > > > d_type.   All  applications  must  properly  handle  a return
> > > > of
> > > > DT_UNKNOWN.
> > > > 
> > > > Applications that rely solely on d_type are effectively broken.
> > > > You
> > > > always need to be able to follow up with a stat or equivalent.
> > > > 
> > > 
> > > Yes, but one of the main such applications is the "find" utility,
> > > which
> > > uses it to avoid calling stat() in order to discover the
> > > directories.
> > > For that reason, NFS does try to set the d_type flag when it is
> > > using
> > > readdirplus, and the server returns attributes for the entry in
> > > question. Otherwise, it is forced to default to DT_UNKNOWN.
> > > 
> > 
> > Yes, didn't mean to imply that we shouldn't try to fill these out
> > where
> > we can, just that there are situations where we might not be able
> > to do
> > so without taking a performance hit.
> > 
> > > Note that in the cases where the readdir entry has a matching
> > > dentry,
> > > we probably could try to do better by doing a d_lookup() and then
> > > filling the d_type. Is that worth doing?
> > > 
> > 
> > I like that idea. Filling out what info we can from the local cache
> > is
> > almost always worthwhile.
> > 
> > An inode's d_type can never change, so you can just vet the fileid
> > or fh
> > in the entry3 vs. the inode that comes back from d_lookup. If they
> > match
> > then you can reliably fill that out.
> > 
> 
> Ironically, this is where NFS over overlayfs may fail, because in
> overlayfs
> d_ino is not always consistent with st_ino. Since v4.15, d_ino is
> consistent
> with st_ino for the case of all layers on the same filesystem. I
> already posted
> a POC for fixing d_ino/st_ino for non-samefs, but it never got
> merged.
>
> What puzzles me w.r.t. this "nonbug" report is that I don't
> understand why
> NFS over overlayfs would behave differently vs. NFS over local fs.
> I am hoping it does not point to a different problem, so would love
> to
> get a more detailed analysis of what's going on between nfsd and
> overlayfs.

The behaviour being described is true of the regular NFS client. It has
nothing to do with overlayfs.

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

  reply	other threads:[~2018-03-14 15:06 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-14  8:42 readdir returns d_type=DT_UNKNOWN to overlay exported dir (NFSv3) Eddie Horng
2018-03-14  9:56 ` Amir Goldstein
2018-03-14 11:02 ` Jeff Layton
2018-03-14 14:16   ` Trond Myklebust
2018-03-14 14:30     ` Jeff Layton
2018-03-14 15:03       ` Amir Goldstein
2018-03-14 15:06         ` Trond Myklebust [this message]
2018-03-14 15:14           ` Amir Goldstein
2018-03-14 15:40             ` Eddie Horng
2018-03-15  9:23               ` Eddie Horng
2018-03-15  9:47 ` Eddie Horng
2018-03-15 13:13   ` Amir Goldstein
2018-03-15 13:22     ` Trond Myklebust
2018-03-15 14:30       ` Eddie Horng
2018-03-15 18:40         ` Amir Goldstein
2018-03-15 21:48           ` Amir Goldstein
2018-03-16  6:25             ` Eddie Horng
2018-03-16  7:23               ` Amir Goldstein
2018-03-16  8:06                 ` Eddie Horng
2018-03-16  9:50                   ` Amir Goldstein
2018-03-16 15:06                     ` Eddie Horng
2018-03-16 15:28                       ` 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=1521039981.40526.11.camel@primarydata.com \
    --to=trondmy@primarydata.com \
    --cc=amir73il@gmail.com \
    --cc=bfields@fieldses.org \
    --cc=eddiehorng.tw@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.