Hi Amir,
Em 14/08/2025 09:53, Amir Goldstein escreveu:
On Thu, Aug 14, 2025 at 12:37 AM André Almeida <andrealmeid@xxxxxxxxxx> wrote:
To add overlayfs support casefold filesystems, create a new function
ovl_casefold(), to be able to do case-insensitive strncmp().
ovl_casefold() allocates a new buffer and stores the casefolded version
of the string on it. If the allocation or the casefold operation fails,
fallback to use the original string.
The case-insentive name is then used in the rb-tree search/insertion
operation. If the name is found in the rb-tree, the name can be
discarded and the buffer is freed. If the name isn't found, it's then
stored at struct ovl_cache_entry to be used later.
Signed-off-by: André Almeida <andrealmeid@xxxxxxxxxx>
---
[...]
+ }
INIT_LIST_HEAD(list);
}
@@ -260,12 +311,28 @@ static bool ovl_fill_merge(struct dir_context *ctx, const char *name,
{
struct ovl_readdir_data *rdd =
container_of(ctx, struct ovl_readdir_data, ctx);
+ struct ovl_fs *ofs = OVL_FS(rdd->dentry->d_sb);
+ const char *aux = NULL;
It looks strange to me that you need aux
and it looks strange to pair <aux, cf_len>
neither here or there...
The reason behind this `aux` var is because I need a `const char`
pointer to point to the `name` argument, and `cf_name` can't be const
because it goes through ovl_casefold(). I tried a couple approaches here
to get rid of the compiler warning regarding const, and the only way I
managed to was using a third variable like that.
+ char *cf_name = NULL;
+ int cf_len = 0;
+
+ if (ofs->casefold)
+ cf_len = ovl_casefold(rdd->map, name, namelen, &cf_name);
+
+ if (cf_len <= 0) {
+ aux = name;
why not:
cf_name = name;
+ cf_len = namelen;
+ } else {
+ aux = cf_name;
+ }
and no aux and no else needed at all?
If you don't like a var named cf_name to point at a non-casefolded
name buffer, then use other var names which are consistent such as
<c_name, c_len> (c for "canonical" or "compare" name).
rdd->count++;
if (!rdd->is_lowest)
- return ovl_cache_entry_add_rb(rdd, name, namelen, ino, d_type);
+ return ovl_cache_entry_add_rb(rdd, name, namelen, aux, cf_len,
+ ino, d_type);
else
- return ovl_fill_lowest(rdd, name, namelen, offset, ino, d_type);
+ return ovl_fill_lowest(rdd, name, namelen, aux, cf_len,
+ offset, ino, d_type);
}
What do you think about moving all the consume/free buffer logic out to caller:
That looks way cleaner to me, thanks! I will apply this approach for v5.
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index b65cdfce31ce..e77530c63207 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -174,7 +174,8 @@ static struct ovl_cache_entry
*ovl_cache_entry_new(struct ovl_readdir_data *rdd,
return p;
}
-static bool ovl_cache_entry_add_rb(struct ovl_readdir_data *rdd,
+/* Return 0 for found, >0 for added, <0 for error */
+static int ovl_cache_entry_add_rb(struct ovl_readdir_data *rdd,
const char *name, int len, u64 ino,
unsigned int d_type)
{
@@ -183,22 +184,23 @@ static bool ovl_cache_entry_add_rb(struct
ovl_readdir_data *rdd,
struct ovl_cache_entry *p;
if (ovl_cache_entry_find_link(name, len, &newp, &parent))
- return true;
+ return 0;
p = ovl_cache_entry_new(rdd, name, len, ino, d_type);
if (p == NULL) {
rdd->err = -ENOMEM;
- return false;
+ return -ENOMEM;
}
list_add_tail(&p->l_node, rdd->list);
rb_link_node(&p->node, parent, newp);
rb_insert_color(&p->node, rdd->root);
- return true;
+ return 1;
}
-static bool ovl_fill_lowest(struct ovl_readdir_data *rdd,
+/* Return 0 for found, >0 for added, <0 for error */
+static int ovl_fill_lowest(struct ovl_readdir_data *rdd,
const char *name, int namelen,
loff_t offset, u64 ino, unsigned int d_type)
{
@@ -207,6 +209,7 @@ static bool ovl_fill_lowest(struct ovl_readdir_data *rdd,
p = ovl_cache_entry_find(rdd->root, name, namelen);
if (p) {
list_move_tail(&p->l_node, &rdd->middle);
+ return 0;
} else {
p = ovl_cache_entry_new(rdd, name, namelen, ino, d_type);
if (p == NULL)
@@ -215,7 +218,7 @@ static bool ovl_fill_lowest(struct ovl_readdir_data *rdd,
list_add_tail(&p->l_node, &rdd->middle);
}
- return rdd->err == 0;
+ return rdd->err ?: 1;
}
@@ -260,12 +263,31 @@ static bool ovl_fill_merge(struct dir_context
*ctx, const char *name,
{
struct ovl_readdir_data *rdd =
container_of(ctx, struct ovl_readdir_data, ctx);
+ struct ovl_fs *ofs = OVL_FS(rdd->dentry->d_sb);
+ char *c_name = NULL;
+ int c_len = 0;
+ int ret;
+
+ if (ofs->casefold)
+ c_len = ovl_casefold(rdd->map, name, namelen, &c_name);
+
+ if (c_len <= 0) {
+ c_name = name;
+ c_len = namelen;
+ }
rdd->count++;
- if (!rdd->is_lowest)
- return ovl_cache_entry_add_rb(rdd, name, namelen, ino, d_type);
- else
- return ovl_fill_lowest(rdd, name, namelen, offset, ino, d_type);
+ if (!rdd->is_lowest) {
+ ret = ovl_cache_entry_add_rb(rdd, name, namelen, c_name, c_len,
+ ino, d_type);
+ } else {
+ ret = ovl_fill_lowest(rdd, name, namelen, c_name, c_len, offset,
+ ino, d_type);
+ }
+ // ret > 1 means c_name is consumed
+ if (ret <= 0 && c_len > 0)
+ kfree(c_name);
+ return ret >= 0;
}
Thanks,
Amir.
