Re: [PATCH v9 4/4] ext4: Use generic casefolding support

2020-07-07 Thread Daniel Rosenberg
On Tue, Jun 23, 2020 at 10:43 PM Gabriel Krisman Bertazi
 wrote:
>
> Daniel Rosenberg  writes:
>
> > -
> >  const struct dentry_operations ext4_dentry_ops = {
> > - .d_hash = ext4_d_hash,
> > - .d_compare = ext4_d_compare,
> > + .d_hash = generic_ci_d_hash,
> > + .d_compare = generic_ci_d_compare,
> >  };
> >  #endif
>
> Can you make the structure generic since it is the same for f2fs and
> ext4, which let you drop the code guards?  Unless that becomes a problem for
> d_revalidate with fscrypt, it is fine like this.
>
> --
> Gabriel Krisman Bertazi

I unify them in a later patch, since I end up having to deal with
fscrypt's d_revalidate. With that patch I'd end up undoing the export
I'd add for this, so I'll skip that for the moment.

-Daniel


Re: [PATCH v9 4/4] ext4: Use generic casefolding support

2020-06-24 Thread Eric Biggers
On Tue, Jun 23, 2020 at 09:33:41PM -0700, Daniel Rosenberg wrote:
> This switches ext4 over to the generic support provided in
> commit 5f829feca774 ("fs: Add standard casefolding support")

Commit IDs aren't determined until the patches are applied.  It's possible for
the person applying the patches to fix them, but in general people will forget,
so it's better not to include non-stable commit IDs.

Also, a sentence explaining *why* this change is good would be helpful.
Commit messages should always have a *why* unless it's obvious.

Likewise for the f2fs commit.

- Eric


Re: [PATCH v9 4/4] ext4: Use generic casefolding support

2020-06-23 Thread Gabriel Krisman Bertazi
Daniel Rosenberg  writes:

> -
>  const struct dentry_operations ext4_dentry_ops = {
> - .d_hash = ext4_d_hash,
> - .d_compare = ext4_d_compare,
> + .d_hash = generic_ci_d_hash,
> + .d_compare = generic_ci_d_compare,
>  };
>  #endif

Can you make the structure generic since it is the same for f2fs and
ext4, which let you drop the code guards?  Unless that becomes a problem for
d_revalidate with fscrypt, it is fine like this.

>  #ifdef CONFIG_UNICODE
> - sbi = EXT4_SB(sb);
> - if (ext4_has_strict_mode(sbi) && IS_CASEFOLDED(dir) &&
> - sbi->s_encoding && utf8_validate(sbi->s_encoding, >d_name))
> + if (sb_has_enc_strict_mode(sb) && IS_CASEFOLDED(dir) &&

I keep reading the 'enc' in sb_has_enc_strict_mode() as 'encryption'.  What do
you think about renaming it to sb_has_strict_encoding()?

These comments apply equally to patches 3 and 4.  Other than that,

Reviewed-by: Gabriel Krisman Bertazi 

-- 
Gabriel Krisman Bertazi


[PATCH v9 4/4] ext4: Use generic casefolding support

2020-06-23 Thread Daniel Rosenberg
This switches ext4 over to the generic support provided in
commit 5f829feca774 ("fs: Add standard casefolding support")

Signed-off-by: Daniel Rosenberg 
---
 fs/ext4/dir.c   | 64 ++---
 fs/ext4/ext4.h  | 12 --
 fs/ext4/hash.c  |  2 +-
 fs/ext4/namei.c | 20 +++-
 fs/ext4/super.c | 12 +-
 5 files changed, 17 insertions(+), 93 deletions(-)

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 1d82336b1cd45..b437120f0b3f5 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -669,68 +669,8 @@ const struct file_operations ext4_dir_operations = {
 };
 
 #ifdef CONFIG_UNICODE
-static int ext4_d_compare(const struct dentry *dentry, unsigned int len,
- const char *str, const struct qstr *name)
-{
-   struct qstr qstr = {.name = str, .len = len };
-   const struct dentry *parent = READ_ONCE(dentry->d_parent);
-   const struct inode *inode = READ_ONCE(parent->d_inode);
-   char strbuf[DNAME_INLINE_LEN];
-
-   if (!inode || !IS_CASEFOLDED(inode) ||
-   !EXT4_SB(inode->i_sb)->s_encoding) {
-   if (len != name->len)
-   return -1;
-   return memcmp(str, name->name, len);
-   }
-
-   /*
-* If the dentry name is stored in-line, then it may be concurrently
-* modified by a rename.  If this happens, the VFS will eventually retry
-* the lookup, so it doesn't matter what ->d_compare() returns.
-* However, it's unsafe to call utf8_strncasecmp() with an unstable
-* string.  Therefore, we have to copy the name into a temporary buffer.
-*/
-   if (len <= DNAME_INLINE_LEN - 1) {
-   memcpy(strbuf, str, len);
-   strbuf[len] = 0;
-   qstr.name = strbuf;
-   /* prevent compiler from optimizing out the temporary buffer */
-   barrier();
-   }
-
-   return ext4_ci_compare(inode, name, , false);
-}
-
-static int ext4_d_hash(const struct dentry *dentry, struct qstr *str)
-{
-   const struct ext4_sb_info *sbi = EXT4_SB(dentry->d_sb);
-   const struct unicode_map *um = sbi->s_encoding;
-   const struct inode *inode = READ_ONCE(dentry->d_inode);
-   unsigned char *norm;
-   int len, ret = 0;
-
-   if (!inode || !IS_CASEFOLDED(inode) || !um)
-   return 0;
-
-   norm = kmalloc(PATH_MAX, GFP_ATOMIC);
-   if (!norm)
-   return -ENOMEM;
-
-   len = utf8_casefold(um, str, norm, PATH_MAX);
-   if (len < 0) {
-   if (ext4_has_strict_mode(sbi))
-   ret = -EINVAL;
-   goto out;
-   }
-   str->hash = full_name_hash(dentry, norm, len);
-out:
-   kfree(norm);
-   return ret;
-}
-
 const struct dentry_operations ext4_dentry_ops = {
-   .d_hash = ext4_d_hash,
-   .d_compare = ext4_d_compare,
+   .d_hash = generic_ci_d_hash,
+   .d_compare = generic_ci_d_compare,
 };
 #endif
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 42f5060f3cdf1..5cd8be24a4fd9 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1393,14 +1393,6 @@ struct ext4_super_block {
 
 #define EXT4_ENC_UTF8_12_1 1
 
-/*
- * Flags for ext4_sb_info.s_encoding_flags.
- */
-#define EXT4_ENC_STRICT_MODE_FL(1 << 0)
-
-#define ext4_has_strict_mode(sbi) \
-   (sbi->s_encoding_flags & EXT4_ENC_STRICT_MODE_FL)
-
 /*
  * fourth extended-fs super-block data in memory
  */
@@ -1450,10 +1442,6 @@ struct ext4_sb_info {
struct kobject s_kobj;
struct completion s_kobj_unregister;
struct super_block *s_sb;
-#ifdef CONFIG_UNICODE
-   struct unicode_map *s_encoding;
-   __u16 s_encoding_flags;
-#endif
 
/* Journaling */
struct journal_s *s_journal;
diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
index 3e133793a5a34..143b0073b3f46 100644
--- a/fs/ext4/hash.c
+++ b/fs/ext4/hash.c
@@ -275,7 +275,7 @@ int ext4fs_dirhash(const struct inode *dir, const char 
*name, int len,
   struct dx_hash_info *hinfo)
 {
 #ifdef CONFIG_UNICODE
-   const struct unicode_map *um = EXT4_SB(dir->i_sb)->s_encoding;
+   const struct unicode_map *um = dir->i_sb->s_encoding;
int r, dlen;
unsigned char *buff;
struct qstr qstr = {.name = name, .len = len };
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 56738b538ddf4..7e9fb77fd2cc7 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1286,8 +1286,8 @@ static void dx_insert_block(struct dx_frame *frame, u32 
hash, ext4_lblk_t block)
 int ext4_ci_compare(const struct inode *parent, const struct qstr *name,
const struct qstr *entry, bool quick)
 {
-   const struct ext4_sb_info *sbi = EXT4_SB(parent->i_sb);
-   const struct unicode_map *um = sbi->s_encoding;
+   const struct super_block *sb = parent->i_sb;
+   const struct unicode_map *um = sb->s_encoding;
int ret;
 
if (quick)
@@ -1299,7 +1299,7 @@ int