[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOQ4uxjgp20vQuMO4GoMxva_8yR+kcW3EJxDuB=T-8KtvDr4kg@mail.gmail.com>
Date: Fri, 22 Aug 2025 21:17:33 +0200
From: Amir Goldstein <amir73il@...il.com>
To: André Almeida <andrealmeid@...lia.com>
Cc: Miklos Szeredi <miklos@...redi.hu>, Theodore Tso <tytso@....edu>,
Gabriel Krisman Bertazi <krisman@...nel.org>, linux-unionfs@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
kernel-dev@...lia.com
Subject: Re: [PATCH v6 9/9] ovl: Support mounting case-insensitive enabled layers
On Fri, Aug 22, 2025 at 6:47 PM André Almeida <andrealmeid@...lia.com> wrote:
>
> Em 22/08/2025 13:34, Amir Goldstein escreveu:
> > On Fri, Aug 22, 2025 at 4:17 PM André Almeida <andrealmeid@...lia.com> wrote:
> >>
> >> Drop the restriction for casefold dentries lookup to enable support for
> >> case-insensitive layers in overlayfs.
> >>
> >> Support case-insensitive layers with the condition that they should be
> >> uniformly enabled across the stack and (i.e. if the root mount dir has
> >> casefold enabled, so should all the dirs bellow for every layer).
> >>
> >> Reviewed-by: Amir Goldstein <amir73il@...il.com>
> >> Signed-off-by: André Almeida <andrealmeid@...lia.com>
> >> ---
> >> Changes from v5:
> >> - Fix mounting layers without casefold flag
> >> ---
> >> fs/overlayfs/namei.c | 17 +++++++++--------
> >> fs/overlayfs/util.c | 10 ++++++----
> >> 2 files changed, 15 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> >> index 76d6248b625e7c58e09685e421aef616aadea40a..e93bcc5727bcafdc18a499b47a7609fd41ecaec8 100644
> >> --- a/fs/overlayfs/namei.c
> >> +++ b/fs/overlayfs/namei.c
> >> @@ -239,13 +239,14 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
> >> char val;
> >>
> >> /*
> >> - * We allow filesystems that are case-folding capable but deny composing
> >> - * ovl stack from case-folded directories. If someone has enabled case
> >> - * folding on a directory on underlying layer, the warranty of the ovl
> >> - * stack is voided.
> >> + * We allow filesystems that are case-folding capable as long as the
> >> + * layers are consistently enabled in the stack, enabled for every dir
> >> + * or disabled in all dirs. If someone has modified case folding on a
> >> + * directory on underlying layer, the warranty of the ovl stack is
> >> + * voided.
> >> */
> >> - if (ovl_dentry_casefolded(base)) {
> >> - warn = "case folded parent";
> >> + if (ofs->casefold != ovl_dentry_casefolded(base)) {
> >> + warn = "parent wrong casefold";
> >> err = -ESTALE;
> >> goto out_warn;
> >> }
> >> @@ -259,8 +260,8 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
> >> goto out_err;
> >> }
> >>
> >> - if (ovl_dentry_casefolded(this)) {
> >> - warn = "case folded child";
> >> + if (ofs->casefold != ovl_dentry_casefolded(this)) {
> >> + warn = "child wrong casefold";
> >> err = -EREMOTE;
> >> goto out_warn;
> >> }
> >> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> >> index a33115e7384c129c543746326642813add63f060..52582b1da52598fbb14866f8c33eb27e36adda36 100644
> >> --- a/fs/overlayfs/util.c
> >> +++ b/fs/overlayfs/util.c
> >> @@ -203,6 +203,8 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
> >>
> >> bool ovl_dentry_weird(struct dentry *dentry)
> >> {
> >> + struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
> >> +
FWIW this was a bug that hits
WARN_ON_ONCE(sb->s_type != &ovl_fs_type)
because dentry is NOT an ovl dentry.
> >> if (!d_can_lookup(dentry) && !d_is_file(dentry) && !d_is_symlink(dentry))
> >> return true;
> >>
> >> @@ -210,11 +212,11 @@ bool ovl_dentry_weird(struct dentry *dentry)
> >> return true;
> >>
> >> /*
> >> - * Allow filesystems that are case-folding capable but deny composing
> >> - * ovl stack from case-folded directories.
> >> + * Exceptionally for layers with casefold, we accept that they have
> >> + * their own hash and compare operations
> >> */
> >> - if (sb_has_encoding(dentry->d_sb))
> >> - return IS_CASEFOLDED(d_inode(dentry));
> >> + if (ofs->casefold)
> >> + return false;
> >
> > I think this is better as:
> > if (sb_has_encoding(dentry->d_sb))
> > return false;
> >
And this still fails the test "Casefold enabled" for me.
Maybe you are confused because this does not look like
a test failure. It looks like this:
generic/999 5s ... [19:10:21][ 150.667994] overlayfs: failed lookup
in lower (ovl-lower/casefold, name='subdir', err=-116): parent wrong
casefold
[ 150.669741] overlayfs: failed lookup in lower (ovl-lower/casefold,
name='subdir', err=-116): parent wrong casefold
[ 150.760644] overlayfs: failed lookup in lower (/ovl-lower,
name='casefold', err=-66): child wrong casefold
[19:10:24] [not run]
generic/999 -- overlayfs does not support casefold enabled layers
Ran: generic/999
Not run: generic/999
Passed all 1 tests
I'm not sure I will keep the test this way. This is not very standard nor
good practice, to run half of the test and then skip it.
I would probably split it into two tests.
The first one as it is now will run to completion on kenrels >= v6.17
and the Casefold enable test will run on kernels >= v6.18.
In any case, please make sure that the test is not skipped when testing
Casefold enabled layers
And then continue with the missing test cases.
When you have a test that passes please send the test itself or
a fstest branch for me to test.
Thanks,
Amir.
Powered by blists - more mailing lists