| Message ID | 20220429182728.14008-4-krisman@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Clean up the case-insenstive lookup path | expand |
Hi Gabriel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on jaegeuk-f2fs/dev-test linus/master v5.18-rc6 next-20220509]
[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/Clean-up-the-case-insenstive-lookup-path/20220430-022957
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
config: x86_64-rhel-8.3-func (https://download.01.org/0day-ci/archive/20220510/202205100425.howJyxrE-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/825d568247e8fc56f2f7e657c434936c7961cefc
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Gabriel-Krisman-Bertazi/Clean-up-the-case-insenstive-lookup-path/20220430-022957
git checkout 825d568247e8fc56f2f7e657c434936c7961cefc
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/ext4/ fs/f2fs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
fs/ext4/namei.c: In function 'ext4_match':
fs/ext4/namei.c:1431:13: warning: unused variable 'ret' [-Wunused-variable]
1431 | int ret;
| ^~~
>> fs/ext4/namei.c:1429:29: warning: unused variable 'u' [-Wunused-variable]
1429 | struct unicode_name u;
| ^
vim +/u +1429 fs/ext4/namei.c
1419
1420 /*
1421 * Test whether a directory entry matches the filename being searched for.
1422 *
1423 * Return: %true if the directory entry matches, otherwise %false.
1424 */
1425 static bool ext4_match(struct inode *parent,
1426 const struct ext4_filename *fname,
1427 struct ext4_dir_entry_2 *de)
1428 {
> 1429 struct unicode_name u;
1430 struct fscrypt_name f;
1431 int ret;
1432
1433 if (!de->inode)
1434 return false;
1435
1436 f.usr_fname = fname->usr_fname;
1437 f.disk_name = fname->disk_name;
1438 #ifdef CONFIG_FS_ENCRYPTION
1439 f.crypto_buf = fname->crypto_buf;
1440 #endif
1441
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index c1a8a09369d1..5102652b5af4 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -1321,20 +1321,19 @@ static void dx_insert_block(struct dx_frame *frame, u32 hash, ext4_lblk_t block) /** * ext4_match_ci() - Match (case-insensitive) a name with a dirent. * @parent: Inode of the parent of the dentry. - * @name: name under lookup. + * @uname: name under lookup. * @de_name: Dirent name. * @de_name_len: dirent name length. - * @quick: whether @name is already casefolded. * * Test whether a case-insensitive directory entry matches the filename - * being searched. If quick is set, the @name being looked up is - * already in the casefolded form. + * being searched. * * Return: > 0 if the directory entry matches, 0 if it doesn't match, or * < 0 on error. */ -static int ext4_match_ci(const struct inode *parent, const struct qstr *name, - u8 *de_name, size_t de_name_len, bool quick) +static int ext4_match_ci(const struct inode *parent, + const struct unicode_name *uname, + u8 *de_name, size_t de_name_len) { const struct super_block *sb = parent->i_sb; const struct unicode_map *um = sb->s_encoding; @@ -1357,10 +1356,10 @@ static int ext4_match_ci(const struct inode *parent, const struct qstr *name, entry.len = decrypted_name.len; } - if (quick) - ret = utf8_strncasecmp_folded(um, name, &entry); + if (uname->folded_name->name) + ret = utf8_strncasecmp_folded(um, uname->folded_name, &entry); else - ret = utf8_strncasecmp(um, name, &entry); + ret = utf8_strncasecmp(um, uname->usr_name, &entry); if (!ret) match = true; @@ -1370,8 +1369,8 @@ static int ext4_match_ci(const struct inode *parent, const struct qstr *name, * the names have invalid characters. */ ret = 0; - match = ((name->len == entry.len) && - !memcmp(name->name, entry.name, entry.len)); + match = ((uname->usr_name->len == entry.len) && + !memcmp(uname->usr_name->name, entry.name, entry.len)); } out: @@ -1427,6 +1426,7 @@ static bool ext4_match(struct inode *parent, const struct ext4_filename *fname, struct ext4_dir_entry_2 *de) { + struct unicode_name u; struct fscrypt_name f; int ret; @@ -1451,14 +1451,12 @@ static bool ext4_match(struct inode *parent, return false; } } - - ret = ext4_match_ci(parent, &fname->cf_name, de->name, - de->name_len, true); - } else { - ret = ext4_match_ci(parent, fname->usr_fname, - de->name, de->name_len, false); } + u.folded_name = &fname->cf_name; + u.usr_name = fname->usr_fname; + + ret = ext4_match_ci(parent, &u, de->name, de->name_len); if (ret < 0) { /* * Treat comparison errors as not a match. The diff --git a/include/linux/fs.h b/include/linux/fs.h index e2d892b201b0..3f76a18a5f40 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3358,6 +3358,11 @@ extern int generic_file_fsync(struct file *, loff_t, loff_t, int); extern int generic_check_addressable(unsigned, u64); +struct unicode_name { + const struct qstr *folded_name; + const struct qstr *usr_name; +}; + extern void generic_set_encrypted_ci_d_ops(struct dentry *dentry); #ifdef CONFIG_MIGRATION
By using a new type here, we can hide most of the caching casefold logic from ext4. The condition in ext4_match is now quite redundant, but this is addressed in the next patch. This doesn't use ext4_filename to keep it generic, since the function will be moved to libfs to be shared with f2fs. Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> -- Changes since v1: - Instead of (ab)using fscrypt_name, create a new type (ebiggers). --- fs/ext4/namei.c | 32 +++++++++++++++----------------- include/linux/fs.h | 5 +++++ 2 files changed, 20 insertions(+), 17 deletions(-)