linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Chris Murphy <lists@colorremedies.com>,
	Miklos Szeredi <miklos@szeredi.hu>
Cc: Btrfs BTRFS <linux-btrfs@vger.kernel.org>,
	overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: btrfs+overlayfs: upper fs does not support xattr, falling back to index=off and metacopy=off.
Date: Sat, 10 Apr 2021 11:03:51 +0300	[thread overview]
Message-ID: <CAOQ4uxht70nODhNHNwGFMSqDyOKLXOKrY0H6g849os4BQ7cokA@mail.gmail.com> (raw)
In-Reply-To: <CAJCQCtTp0aXBssEr4ZXGX=DS_+RyGghmoANCKDdxG59QWu8LVA@mail.gmail.com>

On Sat, Apr 10, 2021 at 1:04 AM Chris Murphy <lists@colorremedies.com> wrote:
>
> Hi,
>
> The primary problem is Bolt (Thunderbolt 3) tests that are
> experiencing a regression when run in a container using overlayfs,
> failing at:
>
> Bail out! ERROR:../tests/test-common.c:1413:test_io_dir_is_empty:
> 'empty' should be FALSE
>
> https://gitlab.freedesktop.org/bolt/bolt/-/issues/171#note_872119
>

To summarize, the test case is:
- create empty dir
- open empty dir
- getdents => (".", "..")
- create file at (dirfd, "a",
- lseek to offset 0 on dirfd
- getdents => (".", "..") FAIL to see "a"

It looks like a bug in ovl readdir cache invalidation only there is
not supposed to be any caching of pure upper dir.

Once thing I noticed is that ovl_dentry_version_inc() is inconsistent
with ovl_dir_is_real() - the latter checks whether readdir caching would
be used and the former checks whether invalidating readdir cache is
needed. We need to change ovl_dentry_version_inc() test to:
    if (ovl_test_flag(OVL_WHITEOUTS, dir) || impurity)
Or better yet:
    if (!ovl_dir_is_real() || impurity)

But this still doesn't explain the reported issue.

The OVL_WHITEOUTS inode flag is set in ovl_get_inode() in several
cases including:
    ovl_check_origin_xattr(ofs, upperdentry)

So now we are getting closer to something that sounds related to the
reported issue...

ovl_check_origin_xattr() would return true if
    vfs_getxattr(upperdentry, "trusted.overlay.origin", NULL, 0)
would return 0 instead of -ENODATA for some reason even though that
xattr does not exist.

But we happen to be missing a pr_debug() in  ovl_do_getxattr(), so
it's hard to say what's going on.

Chris,

As the first step, can you try the suggested fix to ovl_dentry_version_inc()
and/or adding the missing pr_debug() and including those prints in
your report?

> I can reproduce this with 5.12.0-0.rc6.184.fc35.x86_64+debug and at
> approximately the same time I see one, sometimes more, kernel
> messages:
>
> [ 6295.379283] overlayfs: upper fs does not support xattr, falling
> back to index=off and metacopy=off.
>

Can you say why there is no xattr support?
Is the overlayfs mount executed without privileges to create trusted.* xattrs?
The answer to that may be the key to understanding the bug.

> But I don't know if that kernel message relates to the bolt test failure.
>
> If I run the test outside of a container, it doesn't fail. If I run
> the test in a podman container using the btrfs driver instead of the
> overlay driver, it doesn't fail. So it seems like this is an overlayfs
> bug, but could be some kind of overlayfs+btrfs interaction.
>

My guess is it has to do with changes related to mounting overlayfs
inside userns, but I couldn't find any immediate suspects.

Do you have any idea since when the regression appeared?
A bisect would have been helpful here.

> Could this be related and just not yet merged?
> https://lore.kernel.org/linux-unionfs/20210309162654.243184-1-amir73il@gmail.com/
>

Not likely.
If you want to be sure do:
echo N > /sys/module/overlay/parameters/xino_auto
Before starting the container.
Above commit only matters for xino_auto = Y.

Thanks,
Amir.

  reply	other threads:[~2021-04-10  8:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-09 22:03 btrfs+overlayfs: upper fs does not support xattr, falling back to index=off and metacopy=off Chris Murphy
2021-04-10  8:03 ` Amir Goldstein [this message]
2021-04-10 17:36   ` Chris Murphy
2021-04-10 17:55     ` Amir Goldstein
2021-04-10 19:36       ` Chris Murphy
2021-04-10 19:42         ` Chris Murphy
2021-04-10 19:43           ` Chris Murphy
2021-04-10 19:44             ` Chris Murphy
2021-04-10 20:03               ` Chris Murphy
2021-04-11  5:12                 ` Amir Goldstein
2021-04-11  6:05                   ` Chris Murphy
2021-04-11  7:28                     ` Amir Goldstein
2021-04-13 21:50                       ` Chris Murphy
2021-04-12  8:41       ` Miklos Szeredi

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=CAOQ4uxht70nODhNHNwGFMSqDyOKLXOKrY0H6g849os4BQ7cokA@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=lists@colorremedies.com \
    --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 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).