[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOQ4uxj+42O07HxnKrp5A2A-Gk0Z_WLDk=62Y9WZ7RRypdju2w@mail.gmail.com>
Date: Sat, 9 Aug 2025 13:50:50 +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 RFC v3 5/7] ovl: Set case-insensitive dentry operations
for ovl sb
On Fri, Aug 8, 2025 at 10:59 PM André Almeida <andrealmeid@...lia.com> wrote:
>
> For filesystems with encoding (i.e. with case-insensitive support), set
> the dentry operations for the super block as ovl_dentry_ci_operations.
> Also, use the first layer encoding as the ovl super block encoding.
>
> Signed-off-by: André Almeida <andrealmeid@...lia.com>
> ---
> Changes from v2:
> - Create ovl_dentry_ci_operations to not override dentry ops set by
> ovl_dentry_operations
> - Create a new function for this
> - Instead of setting encoding just when there's a upper layer, set it
> for any first layer (ofs->fs[0].sb), regardless of it being upper or
> not.
> ---
> fs/overlayfs/super.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index bcb7f5dbf9a32e4aa09bc41596be443851e21200..68091bf8368a880d62d9425552613497d6e90b6b 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -161,6 +161,16 @@ static const struct dentry_operations ovl_dentry_operations = {
> .d_weak_revalidate = ovl_dentry_weak_revalidate,
> };
>
> +#if IS_ENABLED(CONFIG_UNICODE)
> +static const struct dentry_operations ovl_dentry_ci_operations = {
> + .d_real = ovl_d_real,
> + .d_revalidate = ovl_dentry_revalidate,
> + .d_weak_revalidate = ovl_dentry_weak_revalidate,
> + .d_hash = generic_ci_d_hash,
> + .d_compare = generic_ci_d_compare,
> +};
> +#endif
> +
> static struct kmem_cache *ovl_inode_cachep;
>
> static struct inode *ovl_alloc_inode(struct super_block *sb)
> @@ -1318,6 +1328,21 @@ static struct dentry *ovl_get_root(struct super_block *sb,
> return root;
> }
>
> +/*
> + * Set the ovl sb encoding as the same one used by the first layer
> + */
> +static void ovl_set_sb_ci_ops(struct super_block *ovl_sb, struct super_block *fs_sb)
> +{
> +#if IS_ENABLED(CONFIG_UNICODE)
> + if (sb_has_encoding(fs_sb)) {
> + ovl_sb->s_encoding = fs_sb->s_encoding;
> + ovl_sb->s_encoding_flags = fs_sb->s_encoding_flags;
> + }
> +
> + set_default_d_op(ovl_sb, &ovl_dentry_ci_operations);
I don't like it that set_default_d_op() is called twice and if anything this
helper should have been called only for the ofs->casefold enabled case.
what I suggest it to split to two helpers, first set dentry_ops based on
ofs->casefold determined before ovl_fill_super() is called:
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index df85a76597e9..00647440a566 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1307,6 +1307,19 @@ static struct dentry *ovl_get_root(struct
super_block *sb,
return root;
}
+static void ovl_set_d_op(struct super_block *sb)
+{
+ struct ovl_fs *ofs = sb->s_fs_info;
+
+#if IS_ENABLED(CONFIG_UNICODE)
+ if (ofs->casefold) {
+ set_default_d_op(ovl_sb, &ovl_dentry_ci_operations);
+ return;
+ }
+#endif
+ set_default_d_op(sb, &ovl_dentry_operations);
+}
+
int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
{
struct ovl_fs *ofs = sb->s_fs_info;
@@ -1322,7 +1335,7 @@ int ovl_fill_super(struct super_block *sb,
struct fs_context *fc)
if (WARN_ON(fc->user_ns != current_user_ns()))
goto out_err;
- set_default_d_op(sb, &ovl_dentry_operations);
+ ovl_set_d_op(sb);
> +#endif
> +}
> +
> int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
> {
> struct ovl_fs *ofs = sb->s_fs_info;
> @@ -1423,12 +1448,15 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>
> sb->s_stack_depth = upper_sb->s_stack_depth;
> sb->s_time_gran = upper_sb->s_time_gran;
> +
stray new line.
> }
> oe = ovl_get_lowerstack(sb, ctx, ofs, layers);
> err = PTR_ERR(oe);
> if (IS_ERR(oe))
> goto out_err;
>
> + ovl_set_sb_ci_ops(sb, ofs->fs[0].sb);
> +
This is wrong because ofs->fs[0].sb is NULL on overlay without an upper dir.
Please consider doing this as part of patch 4 instead of using the
local sb1 var:
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -991,6 +991,19 @@ static int ovl_get_data_fsid(struct ovl_fs *ofs)
}
+/*
+ * Set the ovl sb encoding as the same one used by the first layer
+ */
+static void ovl_set_encoding(struct super_block *sb, struct super_block *fs_sb)
+{
+#if IS_ENABLED(CONFIG_UNICODE)
+ if (sb_has_encoding(fs_sb)) {
+ sb->s_encoding = fs_sb->s_encoding;
+ sb->s_encoding_flags = fs_sb->s_encoding_flags;
+ }
+#endif
+}
+
static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
struct ovl_fs_context *ctx, struct ovl_layer *layers)
{
@@ -1024,6 +1036,8 @@ static int ovl_get_layers(struct super_block
*sb, struct ovl_fs *ofs,
if (ovl_upper_mnt(ofs)) {
ofs->fs[0].sb = ovl_upper_mnt(ofs)->mnt_sb;
ofs->fs[0].is_lower = false;
+ if (ofs->casefold)
+ ovl_set_encoding(sb, ofs->fs[0].sb);
}
nr_merged_lower = ctx->nr - ctx->nr_data;
@@ -1083,6 +1097,17 @@ static int ovl_get_layers(struct super_block
*sb, struct ovl_fs *ofs,
l->name = NULL;
ofs->numlayer++;
ofs->fs[fsid].is_lower = true;
+
+ if (ofs->casefold) {
+ if (!ovl_upper_mnt(ofs) && !sb_has_encoding(sb))
+ ovl_set_encoding(sb, ofs->fs[fsid].sb);
+
+ if (!sb_has_encoding(sb) ||
+ !sb_same_encoding(sb, mnt->mnt_sb)) {
+ pr_err("all layers must have the same
encoding\n");
+ return -EINVAL;
+ }
+ }
}
Thanks,
Amir.
Powered by blists - more mailing lists