* [PATCH 0/3] ovl: Enable support for casefold filesystems
@ 2025-04-09 15:00 André Almeida
2025-04-09 15:00 ` [PATCH 1/3] ovl: Make ovl_cache_entry_find support casefold André Almeida
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: André Almeida @ 2025-04-09 15:00 UTC (permalink / raw)
To: Miklos Szeredi, Amir Goldstein, Theodore Tso,
Gabriel Krisman Bertazi
Cc: linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev, André Almeida
Hi all,
We would like to support the usage of casefold filesystems with
overlayfs. This patchset do some of the work needed for that, but I'm
sure there are more places that need to be tweaked so please share your
feedback for this work.
* Implementation
The most obvious place that required change was the strncmp() inside of
ovl_cache_entry_find(), that I managed to convert to use d_same_name(),
that will then call the generic_ci_d_compare function if it's set for
the dentry. There are more strncmp() around ovl, but I would rather hear
feedback about this approach first than already implementing this around
the code.
* Testing
I used tmpfs to create a small ovl like this:
sudo mount -t tmpfs -o casefold tmpfs mnt/
cd mnt/
mkdir dir
chattr +F dir
cd dir/
mkdir upper lower
mkdir lower/A lower/b lower/c
mkdir upper/a upper/b upper/d
mkdir merged work
sudo mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work, merged
ls /tmp/mnt/dir/merged/
a b c d
And ovl is respecting the equivalent names. `a` points to a merged dir
between `A` and `a`, but giving that upperdir has a lowercase `a`, this
is the name displayed here.
Thanks,
André
---
André Almeida (3):
ovl: Make ovl_cache_entry_find support casefold
ovl: Make ovl_dentry_weird() accept casefold dentries
ovl: Enable support for casefold filesystems
fs/overlayfs/namei.c | 11 ++++++-----
fs/overlayfs/overlayfs.h | 2 +-
fs/overlayfs/ovl_entry.h | 1 +
fs/overlayfs/params.c | 5 +++--
fs/overlayfs/readdir.c | 32 +++++++++++++++++++++-----------
fs/overlayfs/util.c | 12 +++++++-----
6 files changed, 39 insertions(+), 24 deletions(-)
---
base-commit: a24588245776dafc227243a01bfbeb8a59bafba9
change-id: 20250409-tonyk-overlayfs-591f5e4d407a
Best regards,
--
André Almeida <andrealmeid@igalia.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] ovl: Make ovl_cache_entry_find support casefold
2025-04-09 15:00 [PATCH 0/3] ovl: Enable support for casefold filesystems André Almeida
@ 2025-04-09 15:00 ` André Almeida
2025-07-11 9:46 ` Amir Goldstein
2025-04-09 15:00 ` [PATCH 2/3] ovl: Make ovl_dentry_weird() accept casefold dentries André Almeida
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: André Almeida @ 2025-04-09 15:00 UTC (permalink / raw)
To: Miklos Szeredi, Amir Goldstein, Theodore Tso,
Gabriel Krisman Bertazi
Cc: linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev, André Almeida
To add overlayfs support casefold filesystems, make
ovl_cache_entry_find() support casefold dentries.
For the casefold support, just comparing the strings does not work
because we need the dentry enconding, so make this function find the
equivalent dentry for a giving directory, if any.
Also, if two strings are not equal, strncmp() return value sign can be
either positive or negative and this information can be used to optimize
the walk in the rb tree. utf8_strncmp(), in the other hand, just return
true or false, so replace the rb walk with a normal rb_next() function.
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
fs/overlayfs/ovl_entry.h | 1 +
fs/overlayfs/readdir.c | 32 +++++++++++++++++++++-----------
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index cb449ab310a7a89aafa0ee04ee7ff6c8141dd7d5..2ee52da85ba3e3fd704415a7ee4e9b7da88bb019 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -90,6 +90,7 @@ struct ovl_fs {
bool no_shared_whiteout;
/* r/o snapshot of upperdir sb's only taken on volatile mounts */
errseq_t errseq;
+ bool casefold;
};
/* Number of lower layers, not including data-only layers */
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 881ec5592da52dfb27a588496582e7084b2fbd3b..68f4a83713e9beab604fd23319d60567ef1feeca 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -92,21 +92,31 @@ static bool ovl_cache_entry_find_link(const char *name, int len,
}
static struct ovl_cache_entry *ovl_cache_entry_find(struct rb_root *root,
- const char *name, int len)
+ const char *name, int len,
+ struct dentry *upper)
{
+ struct ovl_fs *ofs = OVL_FS(upper->d_sb);
struct rb_node *node = root->rb_node;
- int cmp;
+ struct qstr q = { .name = name, .len = len };
while (node) {
struct ovl_cache_entry *p = ovl_cache_entry_from_node(node);
+ struct dentry *p_dentry, *real_dentry = NULL;
+
+ if (ofs->casefold && upper) {
+ p_dentry = ovl_lookup_upper(ofs, p->name, upper, p->len);
+ if (!IS_ERR(p_dentry)) {
+ real_dentry = ovl_dentry_real(p_dentry);
+ if (d_same_name(real_dentry, real_dentry->d_parent, &q))
+ return p;
+ }
+ }
- cmp = strncmp(name, p->name, len);
- if (cmp > 0)
- node = p->node.rb_right;
- else if (cmp < 0 || len < p->len)
- node = p->node.rb_left;
- else
- return p;
+ if (!real_dentry)
+ if (!strncmp(name, p->name, len))
+ return p;
+
+ node = rb_next(&p->node);
}
return NULL;
@@ -204,7 +214,7 @@ static bool ovl_fill_lowest(struct ovl_readdir_data *rdd,
{
struct ovl_cache_entry *p;
- p = ovl_cache_entry_find(rdd->root, name, namelen);
+ p = ovl_cache_entry_find(rdd->root, name, namelen, rdd->dentry);
if (p) {
list_move_tail(&p->l_node, &rdd->middle);
} else {
@@ -678,7 +688,7 @@ static bool ovl_fill_real(struct dir_context *ctx, const char *name,
} else if (rdt->cache) {
struct ovl_cache_entry *p;
- p = ovl_cache_entry_find(&rdt->cache->root, name, namelen);
+ p = ovl_cache_entry_find(&rdt->cache->root, name, namelen, NULL);
if (p)
ino = p->ino;
} else if (rdt->xinobits) {
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] ovl: Make ovl_dentry_weird() accept casefold dentries
2025-04-09 15:00 [PATCH 0/3] ovl: Enable support for casefold filesystems André Almeida
2025-04-09 15:00 ` [PATCH 1/3] ovl: Make ovl_cache_entry_find support casefold André Almeida
@ 2025-04-09 15:00 ` André Almeida
2025-04-09 17:11 ` Amir Goldstein
2025-04-09 15:00 ` [PATCH 3/3] ovl: Enable support for casefold filesystems André Almeida
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: André Almeida @ 2025-04-09 15:00 UTC (permalink / raw)
To: Miklos Szeredi, Amir Goldstein, Theodore Tso,
Gabriel Krisman Bertazi
Cc: linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev, André Almeida
ovl_dentry_weird() is used to avoid problems with filesystems that has
their d_hash and d_compare implementations. Create an exception for this
function, where only casefold filesystems are eligible to use their own
d_hash and d_compare functions, allowing to support casefold
filesystems.
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
fs/overlayfs/namei.c | 11 ++++++-----
fs/overlayfs/overlayfs.h | 2 +-
fs/overlayfs/params.c | 3 ++-
fs/overlayfs/util.c | 12 +++++++-----
4 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index be5c65d6f8484b1fba6b3fee379ba1d034c0df8a..140bc609ffebe00151cbb446720f5106dbeb2ef2 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -192,7 +192,7 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
return real;
}
- if (ovl_dentry_weird(real)) {
+ if (ovl_dentry_weird(real, ofs)) {
dput(real);
return NULL;
}
@@ -244,7 +244,7 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
goto out_err;
}
- if (ovl_dentry_weird(this)) {
+ if (ovl_dentry_weird(this, ofs)) {
/* Don't support traversing automounts and other weirdness */
err = -EREMOTE;
goto out_err;
@@ -365,6 +365,7 @@ static int ovl_lookup_data_layer(struct dentry *dentry, const char *redirect,
struct path *datapath)
{
int err;
+ struct ovl_fs *ovl = OVL_FS(layer->fs->sb);
err = vfs_path_lookup(layer->mnt->mnt_root, layer->mnt, redirect,
LOOKUP_BENEATH | LOOKUP_NO_SYMLINKS | LOOKUP_NO_XDEV,
@@ -376,7 +377,7 @@ static int ovl_lookup_data_layer(struct dentry *dentry, const char *redirect,
return err;
err = -EREMOTE;
- if (ovl_dentry_weird(datapath->dentry))
+ if (ovl_dentry_weird(datapath->dentry, ovl))
goto out_path_put;
err = -ENOENT;
@@ -767,7 +768,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh)
if (ovl_is_whiteout(index))
err = -ESTALE;
- else if (ovl_dentry_weird(index))
+ else if (ovl_dentry_weird(index, ofs))
err = -EIO;
else
return index;
@@ -815,7 +816,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
dput(index);
index = ERR_PTR(-ESTALE);
goto out;
- } else if (ovl_dentry_weird(index) || ovl_is_whiteout(index) ||
+ } else if (ovl_dentry_weird(index, ofs) || ovl_is_whiteout(index) ||
inode_wrong_type(inode, d_inode(origin)->i_mode)) {
/*
* Index should always be of the same file type as origin
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 6f2f8f4cfbbc177fbd5441483395d7ff72efe332..f3de013517598c44c15ca9f950f6c2f0a5c2084b 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -445,7 +445,7 @@ void ovl_dentry_init_reval(struct dentry *dentry, struct dentry *upperdentry,
struct ovl_entry *oe);
void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
struct ovl_entry *oe, unsigned int mask);
-bool ovl_dentry_weird(struct dentry *dentry);
+bool ovl_dentry_weird(struct dentry *dentry, struct ovl_fs *ovl);
enum ovl_path_type ovl_path_type(struct dentry *dentry);
void ovl_path_upper(struct dentry *dentry, struct path *path);
void ovl_path_lower(struct dentry *dentry, struct path *path);
diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index 6759f7d040c89d3b3c01572579c854a900411157..459e8bddf1777c12c9fa0bdfc150e2ea22eaafc3 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -277,6 +277,7 @@ static int ovl_mount_dir_check(struct fs_context *fc, const struct path *path,
enum ovl_opt layer, const char *name, bool upper)
{
struct ovl_fs_context *ctx = fc->fs_private;
+ struct ovl_fs *ovl = fc->s_fs_info;
if (!d_is_dir(path->dentry))
return invalfc(fc, "%s is not a directory", name);
@@ -290,7 +291,7 @@ static int ovl_mount_dir_check(struct fs_context *fc, const struct path *path,
if (sb_has_encoding(path->mnt->mnt_sb))
return invalfc(fc, "case-insensitive capable filesystem on %s not supported", name);
- if (ovl_dentry_weird(path->dentry))
+ if (ovl_dentry_weird(path->dentry, ovl))
return invalfc(fc, "filesystem on %s not supported", name);
/*
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 0819c739cc2ffce0dfefa84d3ff8f9f103eec191..4dd523a1a13ab0a6cf51d967d0b712873e6d8580 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -200,15 +200,17 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
spin_unlock(&dentry->d_lock);
}
-bool ovl_dentry_weird(struct dentry *dentry)
+bool ovl_dentry_weird(struct dentry *dentry, struct ovl_fs *ovl)
{
+ int flags = DCACHE_NEED_AUTOMOUNT | DCACHE_MANAGE_TRANSIT;
+
+ if (!ovl->casefold)
+ flags |= DCACHE_OP_HASH | DCACHE_OP_COMPARE;
+
if (!d_can_lookup(dentry) && !d_is_file(dentry) && !d_is_symlink(dentry))
return true;
- return dentry->d_flags & (DCACHE_NEED_AUTOMOUNT |
- DCACHE_MANAGE_TRANSIT |
- DCACHE_OP_HASH |
- DCACHE_OP_COMPARE);
+ return dentry->d_flags & flags;
}
enum ovl_path_type ovl_path_type(struct dentry *dentry)
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] ovl: Enable support for casefold filesystems
2025-04-09 15:00 [PATCH 0/3] ovl: Enable support for casefold filesystems André Almeida
2025-04-09 15:00 ` [PATCH 1/3] ovl: Make ovl_cache_entry_find support casefold André Almeida
2025-04-09 15:00 ` [PATCH 2/3] ovl: Make ovl_dentry_weird() accept casefold dentries André Almeida
@ 2025-04-09 15:00 ` André Almeida
2025-04-09 16:52 ` [PATCH 0/3] " Gabriel Krisman Bertazi
2025-04-09 17:17 ` Amir Goldstein
4 siblings, 0 replies; 13+ messages in thread
From: André Almeida @ 2025-04-09 15:00 UTC (permalink / raw)
To: Miklos Szeredi, Amir Goldstein, Theodore Tso,
Gabriel Krisman Bertazi
Cc: linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev, André Almeida
Enable support for casefold filesystems in overlayfs.
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
fs/overlayfs/params.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
index 459e8bddf1777c12c9fa0bdfc150e2ea22eaafc3..28a660f09cff2573c648a00363c153be3903aa5b 100644
--- a/fs/overlayfs/params.c
+++ b/fs/overlayfs/params.c
@@ -289,7 +289,7 @@ static int ovl_mount_dir_check(struct fs_context *fc, const struct path *path,
* failures.
*/
if (sb_has_encoding(path->mnt->mnt_sb))
- return invalfc(fc, "case-insensitive capable filesystem on %s not supported", name);
+ ovl->casefold = true;
if (ovl_dentry_weird(path->dentry, ovl))
return invalfc(fc, "filesystem on %s not supported", name);
--
2.49.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] ovl: Enable support for casefold filesystems
2025-04-09 15:00 [PATCH 0/3] ovl: Enable support for casefold filesystems André Almeida
` (2 preceding siblings ...)
2025-04-09 15:00 ` [PATCH 3/3] ovl: Enable support for casefold filesystems André Almeida
@ 2025-04-09 16:52 ` Gabriel Krisman Bertazi
2025-04-09 17:02 ` André Almeida
2025-04-09 17:17 ` Amir Goldstein
4 siblings, 1 reply; 13+ messages in thread
From: Gabriel Krisman Bertazi @ 2025-04-09 16:52 UTC (permalink / raw)
To: André Almeida
Cc: Miklos Szeredi, Amir Goldstein, Theodore Tso, linux-unionfs,
linux-kernel, linux-fsdevel, Alexander Viro, Christian Brauner,
Jan Kara, kernel-dev
André Almeida <andrealmeid@igalia.com> writes:
> Hi all,
>
> We would like to support the usage of casefold filesystems with
> overlayfs. This patchset do some of the work needed for that, but I'm
> sure there are more places that need to be tweaked so please share your
> feedback for this work.
I didn't look the patches yet, but this is going to be quite tricky.
For a start, consider the semantics when mixing volumes with different
case settings for lower/upper/work directories. And that could be any
setting, such as whether the directory has +F, the encoding version and
the encoding flags (strict mode). Any mismatch will look bonkers and
you want to forbid the mount.
Consider upperdir is case-sensitive but lowerdir is not. In this case,
I suspect the case-exact name would be hidden by the upper, but the
inexact-case would still resolve from the lower when it shouldn't, and
can't be raised again. If we have the other way around, the upper
will hide more than one file from the lower and it is a matter of luck
which file we are getting.
In addition, if we have a case-insensitive on top of a case-sensitive,
there is no way we can do the case-insensitive lookup on the lower
without doing a sequential search across the entire directory. Then it
is again a matter of luck which file we are getting.
The issue can appear even on the same volume, since case-insensitiveness
is actually per-directory and can be flipped when a directory is empty.
If something like the below is possible, we are in the same situation
again:
mkdir lower/ci
chattr +F lower/ci
touch lower/ci/BLA
mount -o overlay none upperdir=upper,lowerdir=lower,workdir=work merged
rm -r merged/ci/BLA // creates a whiteout in upper
// merged looks empty and should be allowed to drop +F
chattr -F merged/ci
So we'd also need to always forbid clearing the +F attribute and perhaps
forbid it from ever being set on the merged directory. We also want to
require the encoding version and flags to match.
> * Implementation
>
> The most obvious place that required change was the strncmp() inside of
> ovl_cache_entry_find(), that I managed to convert to use d_same_name(),
> that will then call the generic_ci_d_compare function if it's set for
> the dentry. There are more strncmp() around ovl, but I would rather hear
> feedback about this approach first than already implementing this around
> the code.
I'd suggest marking it as an RFC since it is not a functional
implementation yet, IIUC.
>> * Testing
> sudo mount -t tmpfs -o casefold tmpfs mnt/
> cd mnt/
> mkdir dir
> chattr +F dir
> cd dir/
> mkdir upper lower
> mkdir lower/A lower/b lower/c
> mkdir upper/a upper/b upper/d
> mkdir merged work
> sudo mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work, merged
> ls /tmp/mnt/dir/merged/
> a b c d
>
> And ovl is respecting the equivalent names. `a` points to a merged dir
> between `A` and `a`, but giving that upperdir has a lowercase `a`, this
> is the name displayed here.
Did you try fstests generic/556? It might require some work to make it
run over ovl, but it will exercise several cases that are quite
hard to spot.
--
Gabriel Krisman Bertazi
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] ovl: Enable support for casefold filesystems
2025-04-09 16:52 ` [PATCH 0/3] " Gabriel Krisman Bertazi
@ 2025-04-09 17:02 ` André Almeida
0 siblings, 0 replies; 13+ messages in thread
From: André Almeida @ 2025-04-09 17:02 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: Miklos Szeredi, Amir Goldstein, Theodore Tso, linux-unionfs,
linux-kernel, linux-fsdevel, Alexander Viro, Christian Brauner,
Jan Kara, kernel-dev
Hi Gabriel!
Em 09/04/2025 13:52, Gabriel Krisman Bertazi escreveu:
> André Almeida <andrealmeid@igalia.com> writes:
>
>> Hi all,
>>
>> We would like to support the usage of casefold filesystems with
>> overlayfs. This patchset do some of the work needed for that, but I'm
>> sure there are more places that need to be tweaked so please share your
>> feedback for this work.
>
> I didn't look the patches yet, but this is going to be quite tricky.
> For a start, consider the semantics when mixing volumes with different
> case settings for lower/upper/work directories. And that could be any
> setting, such as whether the directory has +F, the encoding version and
> the encoding flags (strict mode). Any mismatch will look bonkers and
> you want to forbid the mount.
>
> Consider upperdir is case-sensitive but lowerdir is not. In this case,
> I suspect the case-exact name would be hidden by the upper, but the
> inexact-case would still resolve from the lower when it shouldn't, and
> can't be raised again. If we have the other way around, the upper
> will hide more than one file from the lower and it is a matter of luck
> which file we are getting.
>
> In addition, if we have a case-insensitive on top of a case-sensitive,
> there is no way we can do the case-insensitive lookup on the lower
> without doing a sequential search across the entire directory. Then it
> is again a matter of luck which file we are getting.
>
> The issue can appear even on the same volume, since case-insensitiveness
> is actually per-directory and can be flipped when a directory is empty.
> If something like the below is possible, we are in the same situation
> again:
>
> mkdir lower/ci
> chattr +F lower/ci
> touch lower/ci/BLA
> mount -o overlay none upperdir=upper,lowerdir=lower,workdir=work merged
> rm -r merged/ci/BLA // creates a whiteout in upper
> // merged looks empty and should be allowed to drop +F
> chattr -F merged/ci
>
> So we'd also need to always forbid clearing the +F attribute and perhaps
> forbid it from ever being set on the merged directory. We also want to
> require the encoding version and flags to match.
>
Thank you for the prompt response. I agree with you, for the next
version I will implement such restrictions. I think it will be better to
start with very restrict possibilities and then slowly dropping then, if
they prove to be not problematic.
>> * Implementation
>>
>> The most obvious place that required change was the strncmp() inside of
>> ovl_cache_entry_find(), that I managed to convert to use d_same_name(),
>> that will then call the generic_ci_d_compare function if it's set for
>> the dentry. There are more strncmp() around ovl, but I would rather hear
>> feedback about this approach first than already implementing this around
>> the code.
>
> I'd suggest marking it as an RFC since it is not a functional
> implementation yet, IIUC.
>
Ops, you are right, I forgot to tag it as such.
>>> * Testing
>> sudo mount -t tmpfs -o casefold tmpfs mnt/
>> cd mnt/
>> mkdir dir
>> chattr +F dir
>> cd dir/
>> mkdir upper lower
>> mkdir lower/A lower/b lower/c
>> mkdir upper/a upper/b upper/d
>> mkdir merged work
>> sudo mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work, merged
>> ls /tmp/mnt/dir/merged/
>> a b c d
>>
>> And ovl is respecting the equivalent names. `a` points to a merged dir
>> between `A` and `a`, but giving that upperdir has a lowercase `a`, this
>> is the name displayed here.
>
> Did you try fstests generic/556? It might require some work to make it
> run over ovl, but it will exercise several cases that are quite
> hard to spot.
>
>
I haven't tried, I'm not sure which directory should I point to fstests
to use for the test, the merged one?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] ovl: Make ovl_dentry_weird() accept casefold dentries
2025-04-09 15:00 ` [PATCH 2/3] ovl: Make ovl_dentry_weird() accept casefold dentries André Almeida
@ 2025-04-09 17:11 ` Amir Goldstein
2025-07-11 9:20 ` Amir Goldstein
0 siblings, 1 reply; 13+ messages in thread
From: Amir Goldstein @ 2025-04-09 17:11 UTC (permalink / raw)
To: André Almeida
Cc: Miklos Szeredi, Theodore Tso, Gabriel Krisman Bertazi,
linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev
On Wed, Apr 9, 2025 at 5:01 PM André Almeida <andrealmeid@igalia.com> wrote:
>
> ovl_dentry_weird() is used to avoid problems with filesystems that has
> their d_hash and d_compare implementations. Create an exception for this
> function, where only casefold filesystems are eligible to use their own
> d_hash and d_compare functions, allowing to support casefold
> filesystems.
What do you mean by this sentence?
Any casefold fs can be on any layer?
All layers on the same casefold sb? same casefold fstype?
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
> fs/overlayfs/namei.c | 11 ++++++-----
> fs/overlayfs/overlayfs.h | 2 +-
> fs/overlayfs/params.c | 3 ++-
> fs/overlayfs/util.c | 12 +++++++-----
> 4 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index be5c65d6f8484b1fba6b3fee379ba1d034c0df8a..140bc609ffebe00151cbb446720f5106dbeb2ef2 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -192,7 +192,7 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
> return real;
> }
>
> - if (ovl_dentry_weird(real)) {
> + if (ovl_dentry_weird(real, ofs)) {
> dput(real);
> return NULL;
> }
> @@ -244,7 +244,7 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
> goto out_err;
> }
>
> - if (ovl_dentry_weird(this)) {
> + if (ovl_dentry_weird(this, ofs)) {
> /* Don't support traversing automounts and other weirdness */
> err = -EREMOTE;
> goto out_err;
> @@ -365,6 +365,7 @@ static int ovl_lookup_data_layer(struct dentry *dentry, const char *redirect,
> struct path *datapath)
> {
> int err;
> + struct ovl_fs *ovl = OVL_FS(layer->fs->sb);
ofs please
>
> err = vfs_path_lookup(layer->mnt->mnt_root, layer->mnt, redirect,
> LOOKUP_BENEATH | LOOKUP_NO_SYMLINKS | LOOKUP_NO_XDEV,
> @@ -376,7 +377,7 @@ static int ovl_lookup_data_layer(struct dentry *dentry, const char *redirect,
> return err;
>
> err = -EREMOTE;
> - if (ovl_dentry_weird(datapath->dentry))
> + if (ovl_dentry_weird(datapath->dentry, ovl))
> goto out_path_put;
>
> err = -ENOENT;
> @@ -767,7 +768,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh)
>
> if (ovl_is_whiteout(index))
> err = -ESTALE;
> - else if (ovl_dentry_weird(index))
> + else if (ovl_dentry_weird(index, ofs))
> err = -EIO;
> else
> return index;
> @@ -815,7 +816,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
> dput(index);
> index = ERR_PTR(-ESTALE);
> goto out;
> - } else if (ovl_dentry_weird(index) || ovl_is_whiteout(index) ||
> + } else if (ovl_dentry_weird(index, ofs) || ovl_is_whiteout(index) ||
> inode_wrong_type(inode, d_inode(origin)->i_mode)) {
> /*
> * Index should always be of the same file type as origin
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 6f2f8f4cfbbc177fbd5441483395d7ff72efe332..f3de013517598c44c15ca9f950f6c2f0a5c2084b 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -445,7 +445,7 @@ void ovl_dentry_init_reval(struct dentry *dentry, struct dentry *upperdentry,
> struct ovl_entry *oe);
> void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
> struct ovl_entry *oe, unsigned int mask);
> -bool ovl_dentry_weird(struct dentry *dentry);
> +bool ovl_dentry_weird(struct dentry *dentry, struct ovl_fs *ovl);
> enum ovl_path_type ovl_path_type(struct dentry *dentry);
> void ovl_path_upper(struct dentry *dentry, struct path *path);
> void ovl_path_lower(struct dentry *dentry, struct path *path);
> diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> index 6759f7d040c89d3b3c01572579c854a900411157..459e8bddf1777c12c9fa0bdfc150e2ea22eaafc3 100644
> --- a/fs/overlayfs/params.c
> +++ b/fs/overlayfs/params.c
> @@ -277,6 +277,7 @@ static int ovl_mount_dir_check(struct fs_context *fc, const struct path *path,
> enum ovl_opt layer, const char *name, bool upper)
> {
> struct ovl_fs_context *ctx = fc->fs_private;
> + struct ovl_fs *ovl = fc->s_fs_info;
ofs pls
>
> if (!d_is_dir(path->dentry))
> return invalfc(fc, "%s is not a directory", name);
> @@ -290,7 +291,7 @@ static int ovl_mount_dir_check(struct fs_context *fc, const struct path *path,
> if (sb_has_encoding(path->mnt->mnt_sb))
> return invalfc(fc, "case-insensitive capable filesystem on %s not supported", name);
>
> - if (ovl_dentry_weird(path->dentry))
> + if (ovl_dentry_weird(path->dentry, ovl))
> return invalfc(fc, "filesystem on %s not supported", name);
>
> /*
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 0819c739cc2ffce0dfefa84d3ff8f9f103eec191..4dd523a1a13ab0a6cf51d967d0b712873e6d8580 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -200,15 +200,17 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
> spin_unlock(&dentry->d_lock);
> }
>
> -bool ovl_dentry_weird(struct dentry *dentry)
> +bool ovl_dentry_weird(struct dentry *dentry, struct ovl_fs *ovl)
ovl_fs *ofs as first argument please
> {
> + int flags = DCACHE_NEED_AUTOMOUNT | DCACHE_MANAGE_TRANSIT;
> +
> + if (!ovl->casefold)
> + flags |= DCACHE_OP_HASH | DCACHE_OP_COMPARE;
> +
> if (!d_can_lookup(dentry) && !d_is_file(dentry) && !d_is_symlink(dentry))
> return true;
>
> - return dentry->d_flags & (DCACHE_NEED_AUTOMOUNT |
> - DCACHE_MANAGE_TRANSIT |
> - DCACHE_OP_HASH |
> - DCACHE_OP_COMPARE);
> + return dentry->d_flags & flags;
> }
>
> enum ovl_path_type ovl_path_type(struct dentry *dentry)
>
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] ovl: Enable support for casefold filesystems
2025-04-09 15:00 [PATCH 0/3] ovl: Enable support for casefold filesystems André Almeida
` (3 preceding siblings ...)
2025-04-09 16:52 ` [PATCH 0/3] " Gabriel Krisman Bertazi
@ 2025-04-09 17:17 ` Amir Goldstein
2025-07-10 20:54 ` André Almeida
4 siblings, 1 reply; 13+ messages in thread
From: Amir Goldstein @ 2025-04-09 17:17 UTC (permalink / raw)
To: André Almeida
Cc: Miklos Szeredi, Theodore Tso, Gabriel Krisman Bertazi,
linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev
On Wed, Apr 9, 2025 at 5:01 PM André Almeida <andrealmeid@igalia.com> wrote:
>
> Hi all,
>
> We would like to support the usage of casefold filesystems with
> overlayfs. This patchset do some of the work needed for that, but I'm
> sure there are more places that need to be tweaked so please share your
> feedback for this work.
>
> * Implementation
>
> The most obvious place that required change was the strncmp() inside of
> ovl_cache_entry_find(), that I managed to convert to use d_same_name(),
That's a very niche part of overlayfs where comparison of names matter.
Please look very closely at ovl_lookup() and how an overlay entry stack is
composed from several layers including the option to redirect to different names
via redirect xattr, so there is really very much to deal with other
than readdir.
I suggest that you start with a design proposal of how you intend to tackle this
task and what are your requirements?
Any combination of casefold supported layers?
Thanks,
Amir.
> that will then call the generic_ci_d_compare function if it's set for
> the dentry. There are more strncmp() around ovl, but I would rather hear
> feedback about this approach first than already implementing this around
> the code.
>
> * Testing
>
> I used tmpfs to create a small ovl like this:
>
> sudo mount -t tmpfs -o casefold tmpfs mnt/
> cd mnt/
> mkdir dir
> chattr +F dir
> cd dir/
> mkdir upper lower
> mkdir lower/A lower/b lower/c
> mkdir upper/a upper/b upper/d
> mkdir merged work
> sudo mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work, merged
> ls /tmp/mnt/dir/merged/
> a b c d
>
> And ovl is respecting the equivalent names. `a` points to a merged dir
> between `A` and `a`, but giving that upperdir has a lowercase `a`, this
> is the name displayed here.
>
> Thanks,
> André
>
> ---
> André Almeida (3):
> ovl: Make ovl_cache_entry_find support casefold
> ovl: Make ovl_dentry_weird() accept casefold dentries
> ovl: Enable support for casefold filesystems
>
> fs/overlayfs/namei.c | 11 ++++++-----
> fs/overlayfs/overlayfs.h | 2 +-
> fs/overlayfs/ovl_entry.h | 1 +
> fs/overlayfs/params.c | 5 +++--
> fs/overlayfs/readdir.c | 32 +++++++++++++++++++++-----------
> fs/overlayfs/util.c | 12 +++++++-----
> 6 files changed, 39 insertions(+), 24 deletions(-)
> ---
> base-commit: a24588245776dafc227243a01bfbeb8a59bafba9
> change-id: 20250409-tonyk-overlayfs-591f5e4d407a
>
> Best regards,
> --
> André Almeida <andrealmeid@igalia.com>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] ovl: Enable support for casefold filesystems
2025-04-09 17:17 ` Amir Goldstein
@ 2025-07-10 20:54 ` André Almeida
2025-07-11 9:08 ` Amir Goldstein
0 siblings, 1 reply; 13+ messages in thread
From: André Almeida @ 2025-07-10 20:54 UTC (permalink / raw)
To: Amir Goldstein
Cc: Miklos Szeredi, Theodore Tso, Gabriel Krisman Bertazi,
linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev
Hi Amir,
Sorry for my delay.
Em 09/04/2025 14:17, Amir Goldstein escreveu:
> On Wed, Apr 9, 2025 at 5:01 PM André Almeida <andrealmeid@igalia.com> wrote:
>>
>> Hi all,
>>
>> We would like to support the usage of casefold filesystems with
>> overlayfs. This patchset do some of the work needed for that, but I'm
>> sure there are more places that need to be tweaked so please share your
>> feedback for this work.
>>
>> * Implementation
>>
>> The most obvious place that required change was the strncmp() inside of
>> ovl_cache_entry_find(), that I managed to convert to use d_same_name(),
>
> That's a very niche part of overlayfs where comparison of names matter.
>
> Please look very closely at ovl_lookup() and how an overlay entry stack is
> composed from several layers including the option to redirect to different names
> via redirect xattr, so there is really very much to deal with other
> than readdir.
>
> I suggest that you start with a design proposal of how you intend to tackle this
> task and what are your requirements?
> Any combination of casefold supported layers?
>
The intended use case here is to use overlayfs as a container layer for
games. The lower layer will have the common libraries required for
games, and the upper layer will be a container for the running game, so
the game will be able to have write permission and even change the
common libraries if needed without impacting the original libraries. For
that, we would use case-folded enable ext4 mounting points.
This use case doesn't need layers redirection, or to combine different
layers of enabled/disable case-fold. We would have just two layers,
upper and lower, both with case-fold enabled prior to mounting. If the
layers doesn't agree on the casefold flags/version/status, we can refuse
mounting it.
To avoid complexity and corner cases, I propose to have this feature
enabled only for the layout described above: one upper and one lower
layer, with both layers with the same casefold status and to refuse
otherwise.
The implementation would be, on top of this patchset, to create
restrictions on the mounting options if casefold is enabled in a
mounting point.
Thoughts?
> Thanks,
> Amir.
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] ovl: Enable support for casefold filesystems
2025-07-10 20:54 ` André Almeida
@ 2025-07-11 9:08 ` Amir Goldstein
0 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2025-07-11 9:08 UTC (permalink / raw)
To: André Almeida
Cc: Miklos Szeredi, Theodore Tso, Gabriel Krisman Bertazi,
linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev
On Thu, Jul 10, 2025 at 10:54 PM André Almeida <andrealmeid@igalia.com> wrote:
>
> Hi Amir,
>
> Sorry for my delay.
>
> Em 09/04/2025 14:17, Amir Goldstein escreveu:
> > On Wed, Apr 9, 2025 at 5:01 PM André Almeida <andrealmeid@igalia.com> wrote:
> >>
> >> Hi all,
> >>
> >> We would like to support the usage of casefold filesystems with
> >> overlayfs. This patchset do some of the work needed for that, but I'm
> >> sure there are more places that need to be tweaked so please share your
> >> feedback for this work.
> >>
> >> * Implementation
> >>
> >> The most obvious place that required change was the strncmp() inside of
> >> ovl_cache_entry_find(), that I managed to convert to use d_same_name(),
> >
> > That's a very niche part of overlayfs where comparison of names matter.
> >
> > Please look very closely at ovl_lookup() and how an overlay entry stack is
> > composed from several layers including the option to redirect to different names
> > via redirect xattr, so there is really very much to deal with other
> > than readdir.
> >
> > I suggest that you start with a design proposal of how you intend to tackle this
> > task and what are your requirements?
> > Any combination of casefold supported layers?
> >
>
> The intended use case here is to use overlayfs as a container layer for
> games. The lower layer will have the common libraries required for
> games, and the upper layer will be a container for the running game, so
> the game will be able to have write permission and even change the
> common libraries if needed without impacting the original libraries. For
> that, we would use case-folded enable ext4 mounting points.
>
> This use case doesn't need layers redirection, or to combine different
> layers of enabled/disable case-fold. We would have just two layers,
> upper and lower, both with case-fold enabled prior to mounting. If the
> layers doesn't agree on the casefold flags/version/status, we can refuse
> mounting it.
>
> To avoid complexity and corner cases, I propose to have this feature
> enabled only for the layout described above: one upper and one lower
> layer, with both layers with the same casefold status and to refuse
> otherwise.
>
> The implementation would be, on top of this patchset, to create
> restrictions on the mounting options if casefold is enabled in a
> mounting point.
>
> Thoughts?
>
Good plan, but I don't think it is enough.
First of all take a look at this patch already queued for next:
https://lore.kernel.org/linux-unionfs/20250602171702.1941891-1-amir73il@gmail.com/
We will now check for ovl_dentry_casefolded() per dir on every lookup,
not only at mount time, so you need to rebase your patches and adjust the logic
to per-ovl-dir logic - all entries in ovl_stack need to have same casefold.
The second thing is that from vfs POV, your patches do not mark the overlayfs
dirs as casefolded and do not define d_compare()/d_hash() methods
for overlayfs. I think this is wrong and will result in odd behavior
w.r.t overlayfs
dcache.
I think that ovl_fill_super(), in the case where all layers are on same fs and
that fs sb_has_encoding(), assign ovl sb->s_encoding = upper_sb->s_encoding
and call generic_set_sb_d_ops().
As a matter of fact, I don't think we even need to restrict to all
layers on same
sb, just to all layers use the same sb->s_encoding and I think this
will be pretty
easy to implement on-the-fly.
Then in ovl_lookup() when composing the overlayfs stack for ovl_inode,
IFF ovl has_encoding make sure that all or none dentry on stack are
ovl_dentry_casefolded() and if they are casefolded, mark also the overlay
inode S_CASEFOLDED as well.
Please make sure to write fstests for the new functionality.
You can base you test on my WIP test for the patch that is in linux-next:
https://github.com/amir73il/xfstests/commits/ovl-casefold/
Your changes are going to change the results of my test
to allow lookup in a lower casefolded subdir
and so will need to change test expectation and add file name
case insensitive lookups to test this case.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] ovl: Make ovl_dentry_weird() accept casefold dentries
2025-04-09 17:11 ` Amir Goldstein
@ 2025-07-11 9:20 ` Amir Goldstein
0 siblings, 0 replies; 13+ messages in thread
From: Amir Goldstein @ 2025-07-11 9:20 UTC (permalink / raw)
To: André Almeida
Cc: Miklos Szeredi, Theodore Tso, Gabriel Krisman Bertazi,
linux-unionfs, linux-kernel, linux-fsdevel, Alexander Viro,
Christian Brauner, Jan Kara, kernel-dev
On Wed, Apr 9, 2025 at 7:11 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Wed, Apr 9, 2025 at 5:01 PM André Almeida <andrealmeid@igalia.com> wrote:
> >
> > ovl_dentry_weird() is used to avoid problems with filesystems that has
> > their d_hash and d_compare implementations. Create an exception for this
> > function, where only casefold filesystems are eligible to use their own
> > d_hash and d_compare functions, allowing to support casefold
> > filesystems.
>
> What do you mean by this sentence?
> Any casefold fs can be on any layer?
> All layers on the same casefold sb? same casefold fstype?
>
>
> >
> > Signed-off-by: André Almeida <andrealmeid@igalia.com>
> > ---
> > fs/overlayfs/namei.c | 11 ++++++-----
> > fs/overlayfs/overlayfs.h | 2 +-
> > fs/overlayfs/params.c | 3 ++-
> > fs/overlayfs/util.c | 12 +++++++-----
> > 4 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> > index be5c65d6f8484b1fba6b3fee379ba1d034c0df8a..140bc609ffebe00151cbb446720f5106dbeb2ef2 100644
> > --- a/fs/overlayfs/namei.c
> > +++ b/fs/overlayfs/namei.c
> > @@ -192,7 +192,7 @@ struct dentry *ovl_decode_real_fh(struct ovl_fs *ofs, struct ovl_fh *fh,
> > return real;
> > }
> >
> > - if (ovl_dentry_weird(real)) {
> > + if (ovl_dentry_weird(real, ofs)) {
> > dput(real);
> > return NULL;
> > }
> > @@ -244,7 +244,7 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
> > goto out_err;
> > }
> >
> > - if (ovl_dentry_weird(this)) {
> > + if (ovl_dentry_weird(this, ofs)) {
> > /* Don't support traversing automounts and other weirdness */
> > err = -EREMOTE;
> > goto out_err;
> > @@ -365,6 +365,7 @@ static int ovl_lookup_data_layer(struct dentry *dentry, const char *redirect,
> > struct path *datapath)
> > {
> > int err;
> > + struct ovl_fs *ovl = OVL_FS(layer->fs->sb);
>
> ofs please
>
> >
> > err = vfs_path_lookup(layer->mnt->mnt_root, layer->mnt, redirect,
> > LOOKUP_BENEATH | LOOKUP_NO_SYMLINKS | LOOKUP_NO_XDEV,
> > @@ -376,7 +377,7 @@ static int ovl_lookup_data_layer(struct dentry *dentry, const char *redirect,
> > return err;
> >
> > err = -EREMOTE;
> > - if (ovl_dentry_weird(datapath->dentry))
> > + if (ovl_dentry_weird(datapath->dentry, ovl))
> > goto out_path_put;
> >
> > err = -ENOENT;
> > @@ -767,7 +768,7 @@ struct dentry *ovl_get_index_fh(struct ovl_fs *ofs, struct ovl_fh *fh)
> >
> > if (ovl_is_whiteout(index))
> > err = -ESTALE;
> > - else if (ovl_dentry_weird(index))
> > + else if (ovl_dentry_weird(index, ofs))
> > err = -EIO;
> > else
> > return index;
> > @@ -815,7 +816,7 @@ struct dentry *ovl_lookup_index(struct ovl_fs *ofs, struct dentry *upper,
> > dput(index);
> > index = ERR_PTR(-ESTALE);
> > goto out;
> > - } else if (ovl_dentry_weird(index) || ovl_is_whiteout(index) ||
> > + } else if (ovl_dentry_weird(index, ofs) || ovl_is_whiteout(index) ||
> > inode_wrong_type(inode, d_inode(origin)->i_mode)) {
> > /*
> > * Index should always be of the same file type as origin
> > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> > index 6f2f8f4cfbbc177fbd5441483395d7ff72efe332..f3de013517598c44c15ca9f950f6c2f0a5c2084b 100644
> > --- a/fs/overlayfs/overlayfs.h
> > +++ b/fs/overlayfs/overlayfs.h
> > @@ -445,7 +445,7 @@ void ovl_dentry_init_reval(struct dentry *dentry, struct dentry *upperdentry,
> > struct ovl_entry *oe);
> > void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
> > struct ovl_entry *oe, unsigned int mask);
> > -bool ovl_dentry_weird(struct dentry *dentry);
> > +bool ovl_dentry_weird(struct dentry *dentry, struct ovl_fs *ovl);
> > enum ovl_path_type ovl_path_type(struct dentry *dentry);
> > void ovl_path_upper(struct dentry *dentry, struct path *path);
> > void ovl_path_lower(struct dentry *dentry, struct path *path);
> > diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c
> > index 6759f7d040c89d3b3c01572579c854a900411157..459e8bddf1777c12c9fa0bdfc150e2ea22eaafc3 100644
> > --- a/fs/overlayfs/params.c
> > +++ b/fs/overlayfs/params.c
> > @@ -277,6 +277,7 @@ static int ovl_mount_dir_check(struct fs_context *fc, const struct path *path,
> > enum ovl_opt layer, const char *name, bool upper)
> > {
> > struct ovl_fs_context *ctx = fc->fs_private;
> > + struct ovl_fs *ovl = fc->s_fs_info;
>
> ofs pls
>
> >
> > if (!d_is_dir(path->dentry))
> > return invalfc(fc, "%s is not a directory", name);
> > @@ -290,7 +291,7 @@ static int ovl_mount_dir_check(struct fs_context *fc, const struct path *path,
> > if (sb_has_encoding(path->mnt->mnt_sb))
> > return invalfc(fc, "case-insensitive capable filesystem on %s not supported", name);
I wonder how did your tests pass with this ^^^^
Please rebase your patches on top of:
https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/log/?h=vfs-6.17.file
which changes this test to ovl_dentry_casefolded()
but you need to change this logic anyway.
> >
> > - if (ovl_dentry_weird(path->dentry))
> > + if (ovl_dentry_weird(path->dentry, ovl))
> > return invalfc(fc, "filesystem on %s not supported", name);
> >
> > /*
> > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> > index 0819c739cc2ffce0dfefa84d3ff8f9f103eec191..4dd523a1a13ab0a6cf51d967d0b712873e6d8580 100644
> > --- a/fs/overlayfs/util.c
> > +++ b/fs/overlayfs/util.c
> > @@ -200,15 +200,17 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
> > spin_unlock(&dentry->d_lock);
> > }
> >
> > -bool ovl_dentry_weird(struct dentry *dentry)
> > +bool ovl_dentry_weird(struct dentry *dentry, struct ovl_fs *ovl)
>
> ovl_fs *ofs as first argument please
>
Please don't forget to address those nits also.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] ovl: Make ovl_cache_entry_find support casefold
2025-04-09 15:00 ` [PATCH 1/3] ovl: Make ovl_cache_entry_find support casefold André Almeida
@ 2025-07-11 9:46 ` Amir Goldstein
2025-07-14 20:12 ` Gabriel Krisman Bertazi
0 siblings, 1 reply; 13+ messages in thread
From: Amir Goldstein @ 2025-07-11 9:46 UTC (permalink / raw)
To: André Almeida, Gabriel Krisman Bertazi
Cc: Miklos Szeredi, Theodore Tso, linux-unionfs, linux-kernel,
linux-fsdevel, Alexander Viro, Christian Brauner, Jan Kara,
kernel-dev
On Wed, Apr 9, 2025 at 5:01 PM André Almeida <andrealmeid@igalia.com> wrote:
>
> To add overlayfs support casefold filesystems, make
> ovl_cache_entry_find() support casefold dentries.
>
> For the casefold support, just comparing the strings does not work
> because we need the dentry enconding, so make this function find the
> equivalent dentry for a giving directory, if any.
>
> Also, if two strings are not equal, strncmp() return value sign can be
> either positive or negative and this information can be used to optimize
> the walk in the rb tree. utf8_strncmp(), in the other hand, just return
> true or false, so replace the rb walk with a normal rb_next() function.
You cannot just replace a more performance implementation with a
less performant one for everyone else just for your niche use case.
Also it is the wrong approach.
This code needs to use utf8_normalize() to store the normalized
name in the rbtree instead of doing lookup and d_same_name().
and you need to do ovl_cache_entry_add_rb() with the normalized
name anotherwise you break the logic of ovl_dir_read_merged().
Gabriel,
Do you think it makes sense to use utf8_normalize() from this code
directly to generate a key for "is this name found in another layer"
search tree?
I see that utf8_normalize() has zero users, so I guess there was
an intention to use it for things like that?
More nits below, but they will not be relevant once you use the normalized name.
Thanks,
Amir.
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
> fs/overlayfs/ovl_entry.h | 1 +
> fs/overlayfs/readdir.c | 32 +++++++++++++++++++++-----------
> 2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index cb449ab310a7a89aafa0ee04ee7ff6c8141dd7d5..2ee52da85ba3e3fd704415a7ee4e9b7da88bb019 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -90,6 +90,7 @@ struct ovl_fs {
> bool no_shared_whiteout;
> /* r/o snapshot of upperdir sb's only taken on volatile mounts */
> errseq_t errseq;
> + bool casefold;
> };
>
> /* Number of lower layers, not including data-only layers */
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 881ec5592da52dfb27a588496582e7084b2fbd3b..68f4a83713e9beab604fd23319d60567ef1feeca 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -92,21 +92,31 @@ static bool ovl_cache_entry_find_link(const char *name, int len,
> }
>
> static struct ovl_cache_entry *ovl_cache_entry_find(struct rb_root *root,
> - const char *name, int len)
> + const char *name, int len,
> + struct dentry *upper)
This is not an "upper" it is an overlayfs dentry that we call "dentry"
> {
> + struct ovl_fs *ofs = OVL_FS(upper->d_sb);
OVL_FS(upper) is never correct, because OVL_FS() is only applicable to
ovl dentries.
> struct rb_node *node = root->rb_node;
> - int cmp;
> + struct qstr q = { .name = name, .len = len };
>
> while (node) {
> struct ovl_cache_entry *p = ovl_cache_entry_from_node(node);
> + struct dentry *p_dentry, *real_dentry = NULL;
> +
> + if (ofs->casefold && upper) {
> + p_dentry = ovl_lookup_upper(ofs, p->name, upper, p->len);
and here you are mixing a helper to lookup in underlying upper fs with
an overlayfs dentry. You should not do lookup in this context at all.
> + if (!IS_ERR(p_dentry)) {
> + real_dentry = ovl_dentry_real(p_dentry);
> + if (d_same_name(real_dentry, real_dentry->d_parent, &q))
> + return p;
> + }
> + }
>
> - cmp = strncmp(name, p->name, len);
> - if (cmp > 0)
> - node = p->node.rb_right;
> - else if (cmp < 0 || len < p->len)
> - node = p->node.rb_left;
> - else
> - return p;
> + if (!real_dentry)
> + if (!strncmp(name, p->name, len))
> + return p;
> +
> + node = rb_next(&p->node);
As I wrote this change is wrong and unneeded when using normalized names.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] ovl: Make ovl_cache_entry_find support casefold
2025-07-11 9:46 ` Amir Goldstein
@ 2025-07-14 20:12 ` Gabriel Krisman Bertazi
0 siblings, 0 replies; 13+ messages in thread
From: Gabriel Krisman Bertazi @ 2025-07-14 20:12 UTC (permalink / raw)
To: Amir Goldstein
Cc: André Almeida, Miklos Szeredi, Theodore Tso, linux-unionfs,
linux-kernel, linux-fsdevel, Alexander Viro, Christian Brauner,
Jan Kara, kernel-dev
Amir Goldstein <amir73il@gmail.com> writes:
> On Wed, Apr 9, 2025 at 5:01 PM André Almeida <andrealmeid@igalia.com> wrote:
>>
>> To add overlayfs support casefold filesystems, make
>> ovl_cache_entry_find() support casefold dentries.
>>
>> For the casefold support, just comparing the strings does not work
>> because we need the dentry enconding, so make this function find the
>> equivalent dentry for a giving directory, if any.
>>
>> Also, if two strings are not equal, strncmp() return value sign can be
>> either positive or negative and this information can be used to optimize
>> the walk in the rb tree. utf8_strncmp(), in the other hand, just return
>> true or false, so replace the rb walk with a normal rb_next() function.
>
> You cannot just replace a more performance implementation with a
> less performant one for everyone else just for your niche use case.
> Also it is the wrong approach.
>
> This code needs to use utf8_normalize() to store the normalized
> name in the rbtree instead of doing lookup and d_same_name().
> and you need to do ovl_cache_entry_add_rb() with the normalized
> name anotherwise you break the logic of ovl_dir_read_merged().
>
> Gabriel,
>
> Do you think it makes sense to use utf8_normalize() from this code
> directly to generate a key for "is this name found in another layer"
> search tree?
utf8_normalize is on its way out of the kernel and I don't think it
would help here, since it doesn't handle case-insensitive equivalent
names either, bug is just as expensive.
utf8_casefold might do what you want, but it is expensive as well. With
it, you can store the folded version and be sure it is a byte-per-byte
match.
Alternatively, you can keep the existing name and open code something
similar to what generic_ci_match does: check with strncmp first and only
if the mountpoint must consider case-insensitive, do a
utf8_strncasecmp_folded if the first check wasn't a match.
--
Gabriel Krisman Bertazi
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-07-14 20:12 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-09 15:00 [PATCH 0/3] ovl: Enable support for casefold filesystems André Almeida
2025-04-09 15:00 ` [PATCH 1/3] ovl: Make ovl_cache_entry_find support casefold André Almeida
2025-07-11 9:46 ` Amir Goldstein
2025-07-14 20:12 ` Gabriel Krisman Bertazi
2025-04-09 15:00 ` [PATCH 2/3] ovl: Make ovl_dentry_weird() accept casefold dentries André Almeida
2025-04-09 17:11 ` Amir Goldstein
2025-07-11 9:20 ` Amir Goldstein
2025-04-09 15:00 ` [PATCH 3/3] ovl: Enable support for casefold filesystems André Almeida
2025-04-09 16:52 ` [PATCH 0/3] " Gabriel Krisman Bertazi
2025-04-09 17:02 ` André Almeida
2025-04-09 17:17 ` Amir Goldstein
2025-07-10 20:54 ` André Almeida
2025-07-11 9:08 ` Amir Goldstein
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).