* [f2fs-dev] [PATCH 1/7] fs: Expose name under lookup to d_revalidate hook
2022-06-22 19:45 [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Gabriel Krisman Bertazi
@ 2022-06-22 19:45 ` Gabriel Krisman Bertazi
2023-03-23 14:33 ` Theodore Ts'o
2022-06-22 19:45 ` [f2fs-dev] [PATCH 2/7] fs: Add DCACHE_CASEFOLD_LOOKUP flag Gabriel Krisman Bertazi
` (6 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-06-22 19:45 UTC (permalink / raw)
To: viro, tytso, jaegeuk
Cc: kernel, linux-f2fs-devel, ebiggers, linux-fsdevel, linux-ext4,
Gabriel Krisman Bertazi
Negative dentries support on case-insensitive ext4/f2fs will require
access to the name under lookup to ensure it matches the dentry. This
adds an optional new flavor of cached dentry revalidation hook to expose
this extra parameter.
I'm fine with extending d_revalidate instead of adding a new hook, if
it is considered cleaner and the approach is accepted. I wrote a new
hook to simplify reviewing.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
fs/dcache.c | 2 +-
fs/namei.c | 23 ++++++++++++++---------
include/linux/dcache.h | 1 +
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 93f4f5ee07bf..a0fe9e3676fb 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1928,7 +1928,7 @@ void d_set_d_op(struct dentry *dentry, const struct dentry_operations *op)
dentry->d_flags |= DCACHE_OP_HASH;
if (op->d_compare)
dentry->d_flags |= DCACHE_OP_COMPARE;
- if (op->d_revalidate)
+ if (op->d_revalidate || op->d_revalidate_name)
dentry->d_flags |= DCACHE_OP_REVALIDATE;
if (op->d_weak_revalidate)
dentry->d_flags |= DCACHE_OP_WEAK_REVALIDATE;
diff --git a/fs/namei.c b/fs/namei.c
index 1f28d3f463c3..c01bb19723db 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -842,11 +842,16 @@ static bool try_to_unlazy_next(struct nameidata *nd, struct dentry *dentry, unsi
return false;
}
-static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
+static inline int d_revalidate(struct dentry *dentry,
+ const struct qstr *name,
+ unsigned int flags)
{
- if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
+
+ if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE)) {
+ if (dentry->d_op->d_revalidate_name)
+ return dentry->d_op->d_revalidate_name(dentry, name, flags);
return dentry->d_op->d_revalidate(dentry, flags);
- else
+ } else
return 1;
}
@@ -1563,7 +1568,7 @@ static struct dentry *lookup_dcache(const struct qstr *name,
{
struct dentry *dentry = d_lookup(dir, name);
if (dentry) {
- int error = d_revalidate(dentry, flags);
+ int error = d_revalidate(dentry, name, flags);
if (unlikely(error <= 0)) {
if (!error)
d_invalidate(dentry);
@@ -1647,19 +1652,19 @@ static struct dentry *lookup_fast(struct nameidata *nd,
return ERR_PTR(-ECHILD);
*seqp = seq;
- status = d_revalidate(dentry, nd->flags);
+ status = d_revalidate(dentry, &nd->last, nd->flags);
if (likely(status > 0))
return dentry;
if (!try_to_unlazy_next(nd, dentry, seq))
return ERR_PTR(-ECHILD);
if (status == -ECHILD)
/* we'd been told to redo it in non-rcu mode */
- status = d_revalidate(dentry, nd->flags);
+ status = d_revalidate(dentry, &nd->last, nd->flags);
} else {
dentry = __d_lookup(parent, &nd->last);
if (unlikely(!dentry))
return NULL;
- status = d_revalidate(dentry, nd->flags);
+ status = d_revalidate(dentry, &nd->last, nd->flags);
}
if (unlikely(status <= 0)) {
if (!status)
@@ -1687,7 +1692,7 @@ static struct dentry *__lookup_slow(const struct qstr *name,
if (IS_ERR(dentry))
return dentry;
if (unlikely(!d_in_lookup(dentry))) {
- int error = d_revalidate(dentry, flags);
+ int error = d_revalidate(dentry, name, flags);
if (unlikely(error <= 0)) {
if (!error) {
d_invalidate(dentry);
@@ -3302,7 +3307,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
if (d_in_lookup(dentry))
break;
- error = d_revalidate(dentry, nd->flags);
+ error = d_revalidate(dentry, &nd->last, nd->flags);
if (likely(error > 0))
break;
if (error)
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index f5bba51480b2..871f65c8ef7f 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -126,6 +126,7 @@ enum dentry_d_lock_class
struct dentry_operations {
int (*d_revalidate)(struct dentry *, unsigned int);
+ int (*d_revalidate_name)(struct dentry *, const struct qstr *, unsigned int);
int (*d_weak_revalidate)(struct dentry *, unsigned int);
int (*d_hash)(const struct dentry *, struct qstr *);
int (*d_compare)(const struct dentry *,
--
2.36.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 1/7] fs: Expose name under lookup to d_revalidate hook
2022-06-22 19:45 ` [f2fs-dev] [PATCH 1/7] fs: Expose name under lookup to d_revalidate hook Gabriel Krisman Bertazi
@ 2023-03-23 14:33 ` Theodore Ts'o
2023-03-25 13:33 ` Theodore Ts'o
0 siblings, 1 reply; 22+ messages in thread
From: Theodore Ts'o @ 2023-03-23 14:33 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: kernel, linux-f2fs-devel, ebiggers, viro, linux-fsdevel, jaegeuk,
linux-ext4
On Wed, Jun 22, 2022 at 03:45:57PM -0400, Gabriel Krisman Bertazi wrote:
> Negative dentries support on case-insensitive ext4/f2fs will require
> access to the name under lookup to ensure it matches the dentry. This
> adds an optional new flavor of cached dentry revalidation hook to expose
> this extra parameter.
>
> I'm fine with extending d_revalidate instead of adding a new hook, if
> it is considered cleaner and the approach is accepted. I wrote a new
> hook to simplify reviewing.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
Al, could you take a look and see if you have any objections?
Also, any thoughts about adding the new d_revalidate_name() callback
as opposed to change d_revalidate() to add an extra parameter? It
looks like there are some 33 d_revalidate callbacks, from 24 file
sysetms, that would have to be changed.
Cheers,
- Ted
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 1/7] fs: Expose name under lookup to d_revalidate hook
2023-03-23 14:33 ` Theodore Ts'o
@ 2023-03-25 13:33 ` Theodore Ts'o
2023-03-26 5:03 ` Al Viro
0 siblings, 1 reply; 22+ messages in thread
From: Theodore Ts'o @ 2023-03-25 13:33 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: kernel, linux-f2fs-devel, ebiggers, viro, linux-fsdevel, jaegeuk,
linux-ext4
On Thu, Mar 23, 2023 at 10:33:20AM -0400, Theodore Ts'o wrote:
> On Wed, Jun 22, 2022 at 03:45:57PM -0400, Gabriel Krisman Bertazi wrote:
> > Negative dentries support on case-insensitive ext4/f2fs will require
> > access to the name under lookup to ensure it matches the dentry. This
> > adds an optional new flavor of cached dentry revalidation hook to expose
> > this extra parameter.
> >
> > I'm fine with extending d_revalidate instead of adding a new hook, if
> > it is considered cleaner and the approach is accepted. I wrote a new
> > hook to simplify reviewing.
> >
> > Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
>
> Reviewed-by: Theodore Ts'o <tytso@mit.edu>
>
> Al, could you take a look and see if you have any objections?
Ping, Al, any objsections if I take Gabriel's patch series via the
ext4 tree?
- Ted
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 1/7] fs: Expose name under lookup to d_revalidate hook
2023-03-25 13:33 ` Theodore Ts'o
@ 2023-03-26 5:03 ` Al Viro
0 siblings, 0 replies; 22+ messages in thread
From: Al Viro @ 2023-03-26 5:03 UTC (permalink / raw)
To: Theodore Ts'o
Cc: kernel, linux-f2fs-devel, ebiggers, linux-fsdevel, jaegeuk,
linux-ext4, Gabriel Krisman Bertazi
On Sat, Mar 25, 2023 at 09:33:10AM -0400, Theodore Ts'o wrote:
> On Thu, Mar 23, 2023 at 10:33:20AM -0400, Theodore Ts'o wrote:
> > On Wed, Jun 22, 2022 at 03:45:57PM -0400, Gabriel Krisman Bertazi wrote:
> > > Negative dentries support on case-insensitive ext4/f2fs will require
> > > access to the name under lookup to ensure it matches the dentry. This
> > > adds an optional new flavor of cached dentry revalidation hook to expose
> > > this extra parameter.
> > >
> > > I'm fine with extending d_revalidate instead of adding a new hook, if
> > > it is considered cleaner and the approach is accepted. I wrote a new
> > > hook to simplify reviewing.
> > >
> > > Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> >
> > Reviewed-by: Theodore Ts'o <tytso@mit.edu>
> >
> > Al, could you take a look and see if you have any objections?
>
> Ping, Al, any objsections if I take Gabriel's patch series via the
> ext4 tree?
The really subtle part is ->d_name stability in there. We probably are OK
as it is with the current tree (at least I hope so), but it really needs
to be documented - the proof of correctness is not straightforward and it's
going to be brittle; it's not obvious that this memcmp() relies upon the
parent being locked in all cases when we get to calling it. And if that
ever becomes not true, we have a hard-to-debug source of occasional oopsen ;-/
It can be done without reliance on locking - take a look at the vicinity of
dentry_cmp() in fs/dcache.c for example of such, but it's very much not
a blind memcmp(). And I suspect that it would be an overkill here.
In any case, that needs to be discussed in commit message and clearly
spelled out. Otherwise it's a trouble waiting to happen.
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [f2fs-dev] [PATCH 2/7] fs: Add DCACHE_CASEFOLD_LOOKUP flag
2022-06-22 19:45 [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Gabriel Krisman Bertazi
2022-06-22 19:45 ` [f2fs-dev] [PATCH 1/7] fs: Expose name under lookup to d_revalidate hook Gabriel Krisman Bertazi
@ 2022-06-22 19:45 ` Gabriel Krisman Bertazi
2023-03-23 14:33 ` Theodore Ts'o
2022-06-22 19:45 ` [f2fs-dev] [PATCH 3/7] libfs: Validate negative dentries in case-insensitive directories Gabriel Krisman Bertazi
` (5 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-06-22 19:45 UTC (permalink / raw)
To: viro, tytso, jaegeuk
Cc: kernel, linux-f2fs-devel, ebiggers, linux-fsdevel, linux-ext4,
Gabriel Krisman Bertazi
This flag marks a negative or positive dentry as being created after a
case-insensitive lookup operation. It is useful to differentiate
dentries this way to detect whether the negative dentry can be trusted
during a case-insensitive lookup.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
fs/dcache.c | 7 +++++++
include/linux/dcache.h | 8 ++++++++
2 files changed, 15 insertions(+)
diff --git a/fs/dcache.c b/fs/dcache.c
index a0fe9e3676fb..518ddb7fbe0c 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1958,6 +1958,13 @@ void d_set_fallthru(struct dentry *dentry)
}
EXPORT_SYMBOL(d_set_fallthru);
+void d_set_casefold_lookup(struct dentry *dentry)
+{
+ spin_lock(&dentry->d_lock);
+ dentry->d_flags |= DCACHE_CASEFOLD_LOOKUP;
+ spin_unlock(&dentry->d_lock);
+}
+
static unsigned d_flags_for_inode(struct inode *inode)
{
unsigned add_flags = DCACHE_REGULAR_TYPE;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 871f65c8ef7f..8b71c5e418c2 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -208,6 +208,7 @@ struct dentry_operations {
#define DCACHE_FALLTHRU 0x01000000 /* Fall through to lower layer */
#define DCACHE_NOKEY_NAME 0x02000000 /* Encrypted name encoded without key */
#define DCACHE_OP_REAL 0x04000000
+#define DCACHE_CASEFOLD_LOOKUP 0x08000000 /* Dentry comes from a casefold directory */
#define DCACHE_PAR_LOOKUP 0x10000000 /* being looked up (with parent locked shared) */
#define DCACHE_DENTRY_CURSOR 0x20000000
@@ -497,6 +498,13 @@ static inline bool d_is_fallthru(const struct dentry *dentry)
return dentry->d_flags & DCACHE_FALLTHRU;
}
+extern void d_set_casefold_lookup(struct dentry *dentry);
+
+static inline bool d_is_casefold_lookup(const struct dentry *dentry)
+{
+ return dentry->d_flags & DCACHE_CASEFOLD_LOOKUP;
+}
+
extern int sysctl_vfs_cache_pressure;
--
2.36.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 2/7] fs: Add DCACHE_CASEFOLD_LOOKUP flag
2022-06-22 19:45 ` [f2fs-dev] [PATCH 2/7] fs: Add DCACHE_CASEFOLD_LOOKUP flag Gabriel Krisman Bertazi
@ 2023-03-23 14:33 ` Theodore Ts'o
0 siblings, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2023-03-23 14:33 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: kernel, linux-f2fs-devel, ebiggers, viro, linux-fsdevel, jaegeuk,
linux-ext4
On Wed, Jun 22, 2022 at 03:45:58PM -0400, Gabriel Krisman Bertazi wrote:
> This flag marks a negative or positive dentry as being created after a
> case-insensitive lookup operation. It is useful to differentiate
> dentries this way to detect whether the negative dentry can be trusted
> during a case-insensitive lookup.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [f2fs-dev] [PATCH 3/7] libfs: Validate negative dentries in case-insensitive directories
2022-06-22 19:45 [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Gabriel Krisman Bertazi
2022-06-22 19:45 ` [f2fs-dev] [PATCH 1/7] fs: Expose name under lookup to d_revalidate hook Gabriel Krisman Bertazi
2022-06-22 19:45 ` [f2fs-dev] [PATCH 2/7] fs: Add DCACHE_CASEFOLD_LOOKUP flag Gabriel Krisman Bertazi
@ 2022-06-22 19:45 ` Gabriel Krisman Bertazi
2023-03-23 14:36 ` Theodore Ts'o
2023-03-26 4:46 ` Al Viro
2022-06-22 19:46 ` [f2fs-dev] [PATCH 4/7] libfs: Support revalidation of encrypted case-insensitive dentries Gabriel Krisman Bertazi
` (4 subsequent siblings)
7 siblings, 2 replies; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-06-22 19:45 UTC (permalink / raw)
To: viro, tytso, jaegeuk
Cc: kernel, linux-f2fs-devel, ebiggers, linux-fsdevel, linux-ext4,
Gabriel Krisman Bertazi
Introduce a dentry revalidation helper to be used by case-insensitive
filesystems to check if it is safe to reuse a negative dentry.
A negative dentry is safe to be reused on a case-insensitive lookup if
it was created during a case-insensitive lookup and this is not a lookup
that will instantiate a dentry. If this is a creation lookup, we also
need to make sure the name matches sensitively the name under lookup in
order to assure the name preserving semantics.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
fs/libfs.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/fs/libfs.c b/fs/libfs.c
index 31b0ddf01c31..de43f3f585f1 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1450,9 +1450,33 @@ static int generic_ci_d_hash(const struct dentry *dentry, struct qstr *str)
return 0;
}
+static inline int generic_ci_d_revalidate(struct dentry *dentry,
+ const struct qstr *name,
+ unsigned int flags)
+{
+ int is_creation = flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET);
+
+ if (d_is_negative(dentry)) {
+ const struct dentry *parent = READ_ONCE(dentry->d_parent);
+ const struct inode *dir = READ_ONCE(parent->d_inode);
+
+ if (dir && needs_casefold(dir)) {
+ if (!d_is_casefold_lookup(dentry))
+ return 0;
+
+ if (is_creation &&
+ (dentry->d_name.len != name->len ||
+ memcmp(dentry->d_name.name, name->name, name->len)))
+ return 0;
+ }
+ }
+ return 1;
+}
+
static const struct dentry_operations generic_ci_dentry_ops = {
.d_hash = generic_ci_d_hash,
.d_compare = generic_ci_d_compare,
+ .d_revalidate_name = generic_ci_d_revalidate,
};
#endif
--
2.36.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 3/7] libfs: Validate negative dentries in case-insensitive directories
2022-06-22 19:45 ` [f2fs-dev] [PATCH 3/7] libfs: Validate negative dentries in case-insensitive directories Gabriel Krisman Bertazi
@ 2023-03-23 14:36 ` Theodore Ts'o
2023-03-26 4:46 ` Al Viro
1 sibling, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2023-03-23 14:36 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: kernel, linux-f2fs-devel, ebiggers, viro, linux-fsdevel, jaegeuk,
linux-ext4
On Wed, Jun 22, 2022 at 03:45:59PM -0400, Gabriel Krisman Bertazi wrote:
> Introduce a dentry revalidation helper to be used by case-insensitive
> filesystems to check if it is safe to reuse a negative dentry.
>
> A negative dentry is safe to be reused on a case-insensitive lookup if
> it was created during a case-insensitive lookup and this is not a lookup
> that will instantiate a dentry. If this is a creation lookup, we also
> need to make sure the name matches sensitively the name under lookup in
> order to assure the name preserving semantics.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 3/7] libfs: Validate negative dentries in case-insensitive directories
2022-06-22 19:45 ` [f2fs-dev] [PATCH 3/7] libfs: Validate negative dentries in case-insensitive directories Gabriel Krisman Bertazi
2023-03-23 14:36 ` Theodore Ts'o
@ 2023-03-26 4:46 ` Al Viro
2023-03-31 15:31 ` Gabriel Krisman Bertazi
1 sibling, 1 reply; 22+ messages in thread
From: Al Viro @ 2023-03-26 4:46 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: kernel, tytso, linux-f2fs-devel, ebiggers, linux-fsdevel,
jaegeuk, linux-ext4
On Wed, Jun 22, 2022 at 03:45:59PM -0400, Gabriel Krisman Bertazi wrote:
> +static inline int generic_ci_d_revalidate(struct dentry *dentry,
> + const struct qstr *name,
> + unsigned int flags)
> +{
> + int is_creation = flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET);
> +
> + if (d_is_negative(dentry)) {
> + const struct dentry *parent = READ_ONCE(dentry->d_parent);
> + const struct inode *dir = READ_ONCE(parent->d_inode);
> +
> + if (dir && needs_casefold(dir)) {
> + if (!d_is_casefold_lookup(dentry))
> + return 0;
In which conditions does that happen?
> + if (is_creation &&
> + (dentry->d_name.len != name->len ||
> + memcmp(dentry->d_name.name, name->name, name->len)))
> + return 0;
> + }
> + }
> + return 1;
> +}
Analysis of stability of ->d_name, please. It's *probably* safe, but
the details are subtle and IMO should be accompanied by several asserts.
E.g. "we never get LOOKUP_CREATE in op->intent without O_CREAT in op->open_flag
for such and such reasons, and we verify that in such and such place"...
A part of that would be "the call in lookup_dcache() can only get there
with non-zero flags when coming from __lookup_hash(), and that has parent locked,
stabilizing the name; the same goes for the call in __lookup_slow(), with the
only call chain with possibly non-zero flags is through lookup_slow(), where we
have the parent locked". However, lookup_fast() and lookup_open() have the
flags come from nd->flags, and LOOKUP_CREATE can be found there in several areas.
I _think_ we are guaranteed the parent locked in all such call chains, but that
is definitely worth at least a comment.
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 3/7] libfs: Validate negative dentries in case-insensitive directories
2023-03-26 4:46 ` Al Viro
@ 2023-03-31 15:31 ` Gabriel Krisman Bertazi
0 siblings, 0 replies; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2023-03-31 15:31 UTC (permalink / raw)
To: Al Viro
Cc: linux-ext4, tytso, linux-f2fs-devel, ebiggers, linux-fsdevel,
jaegeuk, kernel
Al Viro <viro@zeniv.linux.org.uk> writes:
> On Wed, Jun 22, 2022 at 03:45:59PM -0400, Gabriel Krisman Bertazi wrote:
>
>> +static inline int generic_ci_d_revalidate(struct dentry *dentry,
>> + const struct qstr *name,
>> + unsigned int flags)
>> +{
>> + int is_creation = flags & (LOOKUP_CREATE | LOOKUP_RENAME_TARGET);
>> +
>> + if (d_is_negative(dentry)) {
>> + const struct dentry *parent = READ_ONCE(dentry->d_parent);
>> + const struct inode *dir = READ_ONCE(parent->d_inode);
>> +
>> + if (dir && needs_casefold(dir)) {
>> + if (!d_is_casefold_lookup(dentry))
>> + return 0;
>
> In which conditions does that happen?
Hi Al,
This can happen right after a case-sensitive directory is converted to
case-insensitive. A previous case-sensitive lookup could have left a
negative dentry in the dcache that we need to reject, because it doesn't
have the same assurance of absence of all-variation of names as a
negative dentry created during a case-insensitive lookup.
>> + if (is_creation &&
>> + (dentry->d_name.len != name->len ||
>> + memcmp(dentry->d_name.name, name->name, name->len)))
>> + return 0;
>> + }
>> + }
>> + return 1;
>> +}
>
> Analysis of stability of ->d_name, please. It's *probably* safe, but
> the details are subtle and IMO should be accompanied by several asserts.
> E.g. "we never get LOOKUP_CREATE in op->intent without O_CREAT in op->open_flag
> for such and such reasons, and we verify that in such and such place"...
>
> A part of that would be "the call in lookup_dcache() can only get there
> with non-zero flags when coming from __lookup_hash(), and that has parent locked,
> stabilizing the name; the same goes for the call in __lookup_slow(), with the
> only call chain with possibly non-zero flags is through lookup_slow(), where we
> have the parent locked". However, lookup_fast() and lookup_open() have the
> flags come from nd->flags, and LOOKUP_CREATE can be found there in several areas.
> I _think_ we are guaranteed the parent locked in all such call chains, but that
> is definitely worth at least a comment.
Thanks for the example of the analysis what you are looking for here.
That will help me quite a bit. I wrote this code a while ago and I
don't recall the exact details. I will go through the code again and
send a new version with the detailed analysis.
--
Gabriel Krisman Bertazi
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [f2fs-dev] [PATCH 4/7] libfs: Support revalidation of encrypted case-insensitive dentries
2022-06-22 19:45 [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Gabriel Krisman Bertazi
` (2 preceding siblings ...)
2022-06-22 19:45 ` [f2fs-dev] [PATCH 3/7] libfs: Validate negative dentries in case-insensitive directories Gabriel Krisman Bertazi
@ 2022-06-22 19:46 ` Gabriel Krisman Bertazi
2023-03-23 14:37 ` Theodore Ts'o
2022-06-22 19:46 ` [f2fs-dev] [PATCH 5/7] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops Gabriel Krisman Bertazi
` (3 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-06-22 19:46 UTC (permalink / raw)
To: viro, tytso, jaegeuk
Cc: kernel, linux-f2fs-devel, ebiggers, linux-fsdevel, linux-ext4,
Gabriel Krisman Bertazi
Preserve the existing behavior for encrypted directories, by rejecting
negative dentries of encrypted+casefolded directories. This allows
generic_ci_d_revalidate to be used by filesystems with both features
enabled, as long as the directory is either casefolded or encrypted, but
not both at the same time.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
fs/libfs.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/libfs.c b/fs/libfs.c
index de43f3f585f1..e4da68ebd618 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1461,6 +1461,9 @@ static inline int generic_ci_d_revalidate(struct dentry *dentry,
const struct inode *dir = READ_ONCE(parent->d_inode);
if (dir && needs_casefold(dir)) {
+ if (IS_ENCRYPTED(dir))
+ return 0;
+
if (!d_is_casefold_lookup(dentry))
return 0;
@@ -1470,7 +1473,8 @@ static inline int generic_ci_d_revalidate(struct dentry *dentry,
return 0;
}
}
- return 1;
+
+ return fscrypt_d_revalidate(dentry, flags);
}
static const struct dentry_operations generic_ci_dentry_ops = {
@@ -1490,7 +1494,7 @@ static const struct dentry_operations generic_encrypted_dentry_ops = {
static const struct dentry_operations generic_encrypted_ci_dentry_ops = {
.d_hash = generic_ci_d_hash,
.d_compare = generic_ci_d_compare,
- .d_revalidate = fscrypt_d_revalidate,
+ .d_revalidate_name = generic_ci_d_revalidate,
};
#endif
--
2.36.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 4/7] libfs: Support revalidation of encrypted case-insensitive dentries
2022-06-22 19:46 ` [f2fs-dev] [PATCH 4/7] libfs: Support revalidation of encrypted case-insensitive dentries Gabriel Krisman Bertazi
@ 2023-03-23 14:37 ` Theodore Ts'o
0 siblings, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2023-03-23 14:37 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: kernel, linux-f2fs-devel, ebiggers, viro, linux-fsdevel, jaegeuk,
linux-ext4
On Wed, Jun 22, 2022 at 03:46:00PM -0400, Gabriel Krisman Bertazi wrote:
> Preserve the existing behavior for encrypted directories, by rejecting
> negative dentries of encrypted+casefolded directories. This allows
> generic_ci_d_revalidate to be used by filesystems with both features
> enabled, as long as the directory is either casefolded or encrypted, but
> not both at the same time.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [f2fs-dev] [PATCH 5/7] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops
2022-06-22 19:45 [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Gabriel Krisman Bertazi
` (3 preceding siblings ...)
2022-06-22 19:46 ` [f2fs-dev] [PATCH 4/7] libfs: Support revalidation of encrypted case-insensitive dentries Gabriel Krisman Bertazi
@ 2022-06-22 19:46 ` Gabriel Krisman Bertazi
2023-03-23 14:39 ` Theodore Ts'o
2022-06-22 19:46 ` [f2fs-dev] [PATCH 6/7] ext4: Enable negative dentries on case-insensitive lookup Gabriel Krisman Bertazi
` (2 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-06-22 19:46 UTC (permalink / raw)
To: viro, tytso, jaegeuk
Cc: kernel, linux-f2fs-devel, ebiggers, linux-fsdevel, linux-ext4,
Gabriel Krisman Bertazi
Now that casefold needs d_revalidate and calls fscrypt_d_revalidate
itself, generic_encrypt_ci_dentry_ops and generic_ci_dentry_ops are now
equivalent. Merge them together and simplify the setup code.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
fs/libfs.c | 44 +++++++++++++-------------------------------
1 file changed, 13 insertions(+), 31 deletions(-)
diff --git a/fs/libfs.c b/fs/libfs.c
index e4da68ebd618..05f82e1a6f70 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1477,7 +1477,7 @@ static inline int generic_ci_d_revalidate(struct dentry *dentry,
return fscrypt_d_revalidate(dentry, flags);
}
-static const struct dentry_operations generic_ci_dentry_ops = {
+static const struct dentry_operations generic_encrypted_ci_dentry_ops = {
.d_hash = generic_ci_d_hash,
.d_compare = generic_ci_d_compare,
.d_revalidate_name = generic_ci_d_revalidate,
@@ -1490,26 +1490,20 @@ static const struct dentry_operations generic_encrypted_dentry_ops = {
};
#endif
-#if defined(CONFIG_FS_ENCRYPTION) && IS_ENABLED(CONFIG_UNICODE)
-static const struct dentry_operations generic_encrypted_ci_dentry_ops = {
- .d_hash = generic_ci_d_hash,
- .d_compare = generic_ci_d_compare,
- .d_revalidate_name = generic_ci_d_revalidate,
-};
-#endif
-
/**
* generic_set_encrypted_ci_d_ops - helper for setting d_ops for given dentry
* @dentry: dentry to set ops on
*
- * Casefolded directories need d_hash and d_compare set, so that the dentries
- * contained in them are handled case-insensitively. Note that these operations
- * are needed on the parent directory rather than on the dentries in it, and
- * while the casefolding flag can be toggled on and off on an empty directory,
- * dentry_operations can't be changed later. As a result, if the filesystem has
- * casefolding support enabled at all, we have to give all dentries the
- * casefolding operations even if their inode doesn't have the casefolding flag
- * currently (and thus the casefolding ops would be no-ops for now).
+ * Casefolded directories need d_hash, d_compare and d_revalidate set, so
+ * that the dentries contained in them are handled case-insensitively,
+ * but implement support for fs_encryption. Note that these operations
+ * are needed on the parent directory rather than on the dentries in it,
+ * and while the casefolding flag can be toggled on and off on an empty
+ * directory, dentry_operations can't be changed later. As a result, if
+ * the filesystem has casefolding support enabled at all, we have to
+ * give all dentries the casefolding operations even if their inode
+ * doesn't have the casefolding flag currently (and thus the casefolding
+ * ops would be no-ops for now).
*
* Encryption works differently in that the only dentry operation it needs is
* d_revalidate, which it only needs on dentries that have the no-key name flag.
@@ -1522,29 +1516,17 @@ static const struct dentry_operations generic_encrypted_ci_dentry_ops = {
*/
void generic_set_encrypted_ci_d_ops(struct dentry *dentry)
{
-#ifdef CONFIG_FS_ENCRYPTION
- bool needs_encrypt_ops = dentry->d_flags & DCACHE_NOKEY_NAME;
-#endif
#if IS_ENABLED(CONFIG_UNICODE)
- bool needs_ci_ops = dentry->d_sb->s_encoding;
-#endif
-#if defined(CONFIG_FS_ENCRYPTION) && IS_ENABLED(CONFIG_UNICODE)
- if (needs_encrypt_ops && needs_ci_ops) {
+ if (dentry->d_sb->s_encoding) {
d_set_d_op(dentry, &generic_encrypted_ci_dentry_ops);
return;
}
#endif
#ifdef CONFIG_FS_ENCRYPTION
- if (needs_encrypt_ops) {
+ if (dentry->d_flags & DCACHE_NOKEY_NAME) {
d_set_d_op(dentry, &generic_encrypted_dentry_ops);
return;
}
#endif
-#if IS_ENABLED(CONFIG_UNICODE)
- if (needs_ci_ops) {
- d_set_d_op(dentry, &generic_ci_dentry_ops);
- return;
- }
-#endif
}
EXPORT_SYMBOL(generic_set_encrypted_ci_d_ops);
--
2.36.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 5/7] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops
2022-06-22 19:46 ` [f2fs-dev] [PATCH 5/7] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops Gabriel Krisman Bertazi
@ 2023-03-23 14:39 ` Theodore Ts'o
0 siblings, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2023-03-23 14:39 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: kernel, linux-f2fs-devel, ebiggers, viro, linux-fsdevel, jaegeuk,
linux-ext4
On Wed, Jun 22, 2022 at 03:46:01PM -0400, Gabriel Krisman Bertazi wrote:
> Now that casefold needs d_revalidate and calls fscrypt_d_revalidate
> itself, generic_encrypt_ci_dentry_ops and generic_ci_dentry_ops are now
> equivalent. Merge them together and simplify the setup code.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [f2fs-dev] [PATCH 6/7] ext4: Enable negative dentries on case-insensitive lookup
2022-06-22 19:45 [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Gabriel Krisman Bertazi
` (4 preceding siblings ...)
2022-06-22 19:46 ` [f2fs-dev] [PATCH 5/7] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops Gabriel Krisman Bertazi
@ 2022-06-22 19:46 ` Gabriel Krisman Bertazi
2022-06-23 7:29 ` kernel test robot
2023-03-23 14:39 ` Theodore Ts'o
2022-06-22 19:46 ` [f2fs-dev] [PATCH 7/7] f2fs: " Gabriel Krisman Bertazi
2023-02-24 22:36 ` [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Daniel Rosenberg via Linux-f2fs-devel
7 siblings, 2 replies; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-06-22 19:46 UTC (permalink / raw)
To: viro, tytso, jaegeuk
Cc: kernel, linux-f2fs-devel, ebiggers, linux-fsdevel, linux-ext4,
Gabriel Krisman Bertazi
Instead of invalidating negative dentries during case-insensitive
lookups, mark them as such and let them be added to the dcache.
d_ci_revalidate is able to properly filter them out if necessary based
on the dentry casefold flag.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
fs/ext4/namei.c | 35 ++++-------------------------------
1 file changed, 4 insertions(+), 31 deletions(-)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index db4ba99d1ceb..9908ad6cb071 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1823,16 +1823,9 @@ static struct dentry *ext4_lookup(struct inode *dir, struct dentry *dentry, unsi
}
}
-#if IS_ENABLED(CONFIG_UNICODE)
- if (!inode && IS_CASEFOLDED(dir)) {
- /* Eventually we want to call d_add_ci(dentry, NULL)
- * for negative dentries in the encoding case as
- * well. For now, prevent the negative dentry
- * from being cached.
- */
- return NULL;
- }
-#endif
+ if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir))
+ d_set_casefold_lookup(dentry);
+
return d_splice_alias(inode, dentry);
}
@@ -3153,17 +3146,6 @@ static int ext4_rmdir(struct inode *dir, struct dentry *dentry)
ext4_fc_track_unlink(handle, dentry);
retval = ext4_mark_inode_dirty(handle, dir);
-#if IS_ENABLED(CONFIG_UNICODE)
- /* VFS negative dentries are incompatible with Encoding and
- * Case-insensitiveness. Eventually we'll want avoid
- * invalidating the dentries here, alongside with returning the
- * negative dentries at ext4_lookup(), when it is better
- * supported by the VFS for the CI case.
- */
- if (IS_CASEFOLDED(dir))
- d_invalidate(dentry);
-#endif
-
end_rmdir:
brelse(bh);
if (handle)
@@ -3258,16 +3240,7 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry)
retval = __ext4_unlink(handle, dir, &dentry->d_name, d_inode(dentry));
if (!retval)
ext4_fc_track_unlink(handle, dentry);
-#if IS_ENABLED(CONFIG_UNICODE)
- /* VFS negative dentries are incompatible with Encoding and
- * Case-insensitiveness. Eventually we'll want avoid
- * invalidating the dentries here, alongside with returning the
- * negative dentries at ext4_lookup(), when it is better
- * supported by the VFS for the CI case.
- */
- if (IS_CASEFOLDED(dir))
- d_invalidate(dentry);
-#endif
+
if (handle)
ext4_journal_stop(handle);
--
2.36.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 6/7] ext4: Enable negative dentries on case-insensitive lookup
2022-06-22 19:46 ` [f2fs-dev] [PATCH 6/7] ext4: Enable negative dentries on case-insensitive lookup Gabriel Krisman Bertazi
@ 2022-06-23 7:29 ` kernel test robot
2022-06-23 16:36 ` Gabriel Krisman Bertazi
2023-03-23 14:39 ` Theodore Ts'o
1 sibling, 1 reply; 22+ messages in thread
From: kernel test robot @ 2022-06-23 7:29 UTC (permalink / raw)
To: Gabriel Krisman Bertazi, viro, tytso, jaegeuk
Cc: kernel, kbuild-all, linux-f2fs-devel, ebiggers, linux-fsdevel,
linux-ext4, Gabriel Krisman Bertazi
Hi Gabriel,
I love your patch! Yet something to improve:
[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on jaegeuk-f2fs/dev-test linus/master v5.19-rc3 next-20220622]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Gabriel-Krisman-Bertazi/Support-negative-dentries-on-case-insensitive-directories/20220623-034942
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
config: x86_64-randconfig-a006 (https://download.01.org/0day-ci/archive/20220623/202206231550.0JrilBjp-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/69488ccc517a48af2f1cec0efb84651397edf6f6
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Gabriel-Krisman-Bertazi/Support-negative-dentries-on-case-insensitive-directories/20220623-034942
git checkout 69488ccc517a48af2f1cec0efb84651397edf6f6
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "d_set_casefold_lookup" [fs/ext4/ext4.ko] undefined!
--
0-DAY CI Kernel Test Service
https://01.org/lkp
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 6/7] ext4: Enable negative dentries on case-insensitive lookup
2022-06-23 7:29 ` kernel test robot
@ 2022-06-23 16:36 ` Gabriel Krisman Bertazi
0 siblings, 0 replies; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-06-23 16:36 UTC (permalink / raw)
To: kernel test robot
Cc: kernel, tytso, kbuild-all, linux-f2fs-devel, ebiggers, viro,
linux-fsdevel, jaegeuk, linux-ext4
kernel test robot <lkp@intel.com> writes:
> Hi Gabriel,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on tytso-ext4/dev]
> [also build test ERROR on jaegeuk-f2fs/dev-test linus/master v5.19-rc3 next-20220622]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Gabriel-Krisman-Bertazi/Support-negative-dentries-on-case-insensitive-directories/20220623-034942
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
> config: x86_64-randconfig-a006 (https://download.01.org/0day-ci/archive/20220623/202206231550.0JrilBjp-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
> reproduce (this is a W=1 build):
> # https://github.com/intel-lab-lkp/linux/commit/69488ccc517a48af2f1cec0efb84651397edf6f6
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Gabriel-Krisman-Bertazi/Support-negative-dentries-on-case-insensitive-directories/20220623-034942
> git checkout 69488ccc517a48af2f1cec0efb84651397edf6f6
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>, old ones prefixed by <<):
>
>>> ERROR: modpost: "d_set_casefold_lookup" [fs/ext4/ext4.ko] undefined!
Hm, missing the EXPORT_SYMBOL() since this is called from filesystems.
I will add it for v2.
--
Gabriel Krisman Bertazi
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 6/7] ext4: Enable negative dentries on case-insensitive lookup
2022-06-22 19:46 ` [f2fs-dev] [PATCH 6/7] ext4: Enable negative dentries on case-insensitive lookup Gabriel Krisman Bertazi
2022-06-23 7:29 ` kernel test robot
@ 2023-03-23 14:39 ` Theodore Ts'o
1 sibling, 0 replies; 22+ messages in thread
From: Theodore Ts'o @ 2023-03-23 14:39 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: kernel, linux-f2fs-devel, ebiggers, viro, linux-fsdevel, jaegeuk,
linux-ext4
On Wed, Jun 22, 2022 at 03:46:02PM -0400, Gabriel Krisman Bertazi wrote:
> Instead of invalidating negative dentries during case-insensitive
> lookups, mark them as such and let them be added to the dcache.
> d_ci_revalidate is able to properly filter them out if necessary based
> on the dentry casefold flag.
>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Reviewed-by: Theodore Ts'o <tytso@mit.edu>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [f2fs-dev] [PATCH 7/7] f2fs: Enable negative dentries on case-insensitive lookup
2022-06-22 19:45 [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Gabriel Krisman Bertazi
` (5 preceding siblings ...)
2022-06-22 19:46 ` [f2fs-dev] [PATCH 6/7] ext4: Enable negative dentries on case-insensitive lookup Gabriel Krisman Bertazi
@ 2022-06-22 19:46 ` Gabriel Krisman Bertazi
2022-06-23 12:44 ` kernel test robot
2023-02-24 22:36 ` [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Daniel Rosenberg via Linux-f2fs-devel
7 siblings, 1 reply; 22+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-06-22 19:46 UTC (permalink / raw)
To: viro, tytso, jaegeuk
Cc: kernel, linux-f2fs-devel, ebiggers, linux-fsdevel, linux-ext4,
Gabriel Krisman Bertazi
Instead of invalidating negative dentries during case-insensitive
lookups, mark them as such and let them be added to the dcache.
d_ci_revalidate is able to properly filter them out if necessary based
on the dentry casefold flag.
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
---
fs/f2fs/namei.c | 23 ++---------------------
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index c549acb52ac4..20c3391bb209 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -566,17 +566,8 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry,
goto out_iput;
}
out_splice:
-#if IS_ENABLED(CONFIG_UNICODE)
- if (!inode && IS_CASEFOLDED(dir)) {
- /* Eventually we want to call d_add_ci(dentry, NULL)
- * for negative dentries in the encoding case as
- * well. For now, prevent the negative dentry
- * from being cached.
- */
- trace_f2fs_lookup_end(dir, dentry, ino, err);
- return NULL;
- }
-#endif
+ if (IS_ENABLED(CONFIG_UNICODE) && IS_CASEFOLDED(dir))
+ d_set_casefold_lookup(dentry);
new = d_splice_alias(inode, dentry);
err = PTR_ERR_OR_ZERO(new);
trace_f2fs_lookup_end(dir, dentry, ino, !new ? -ENOENT : err);
@@ -627,16 +618,6 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry)
goto fail;
}
f2fs_delete_entry(de, page, dir, inode);
-#if IS_ENABLED(CONFIG_UNICODE)
- /* VFS negative dentries are incompatible with Encoding and
- * Case-insensitiveness. Eventually we'll want avoid
- * invalidating the dentries here, alongside with returning the
- * negative dentries at f2fs_lookup(), when it is better
- * supported by the VFS for the CI case.
- */
- if (IS_CASEFOLDED(dir))
- d_invalidate(dentry);
-#endif
f2fs_unlock_op(sbi);
if (IS_DIRSYNC(dir))
--
2.36.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 7/7] f2fs: Enable negative dentries on case-insensitive lookup
2022-06-22 19:46 ` [f2fs-dev] [PATCH 7/7] f2fs: " Gabriel Krisman Bertazi
@ 2022-06-23 12:44 ` kernel test robot
0 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2022-06-23 12:44 UTC (permalink / raw)
To: Gabriel Krisman Bertazi, viro, tytso, jaegeuk
Cc: kernel, kbuild-all, linux-f2fs-devel, ebiggers, linux-fsdevel,
linux-ext4, Gabriel Krisman Bertazi
Hi Gabriel,
I love your patch! Yet something to improve:
[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on jaegeuk-f2fs/dev-test linus/master v5.19-rc3 next-20220623]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Gabriel-Krisman-Bertazi/Support-negative-dentries-on-case-insensitive-directories/20220623-034942
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
config: x86_64-randconfig-a006 (https://download.01.org/0day-ci/archive/20220623/202206231838.E75vWtY3-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/ab740b793e65b54ce8f48c693b86761f5bcaa911
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Gabriel-Krisman-Bertazi/Support-negative-dentries-on-case-insensitive-directories/20220623-034942
git checkout ab740b793e65b54ce8f48c693b86761f5bcaa911
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
ERROR: modpost: "d_set_casefold_lookup" [fs/ext4/ext4.ko] undefined!
>> ERROR: modpost: "d_set_casefold_lookup" [fs/f2fs/f2fs.ko] undefined!
--
0-DAY CI Kernel Test Service
https://01.org/lkp
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories
2022-06-22 19:45 [f2fs-dev] [PATCH 0/7] Support negative dentries on case-insensitive directories Gabriel Krisman Bertazi
` (6 preceding siblings ...)
2022-06-22 19:46 ` [f2fs-dev] [PATCH 7/7] f2fs: " Gabriel Krisman Bertazi
@ 2023-02-24 22:36 ` Daniel Rosenberg via Linux-f2fs-devel
7 siblings, 0 replies; 22+ messages in thread
From: Daniel Rosenberg via Linux-f2fs-devel @ 2023-02-24 22:36 UTC (permalink / raw)
To: krisman
Cc: kernel, tytso, Daniel Rosenberg, linux-f2fs-devel, ebiggers,
viro, linux-fsdevel, jaegeuk, linux-ext4
These look good to me. It will be nice to have negative dentries back for
casefolded directories.
-Daniel Rosenberg
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 22+ messages in thread