Hello,
This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
Testing
=========
1. Tested "-g encrypt" with default configs.
2. Compiled tested on x86 & Power.
Ritesh Harjani (6):
fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
ext4: Move ext4 crypto code to its own file ext4_crypto.c
ext4: Directly opencode ext4_set_test_dummy_encryption
ext4: Cleanup function defs from ext4.h into ext4_crypto.c
ext4: Move all encryption related into a common #ifdef
ext4: Use provided macro for checking dummy_enc_policy
fs/ext4/Makefile | 1 +
fs/ext4/ext4.h | 81 +++--------------
fs/ext4/ext4_crypto.c | 192 ++++++++++++++++++++++++++++++++++++++++
fs/ext4/super.c | 158 ++++-----------------------------
include/linux/fscrypt.h | 7 ++
5 files changed, 227 insertions(+), 212 deletions(-)
create mode 100644 fs/ext4/ext4_crypto.c
--
2.31.1
This just moves left over usages of #ifdef CONFIG_FS_ENCRYPTION
into a common place for function/macro definitions.
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ext4/ext4.h | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index caf154db4680..2fb69c500063 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1440,12 +1440,6 @@ struct ext4_super_block {
#ifdef __KERNEL__
-#ifdef CONFIG_FS_ENCRYPTION
-#define DUMMY_ENCRYPTION_ENABLED(sbi) ((sbi)->s_dummy_enc_policy.policy != NULL)
-#else
-#define DUMMY_ENCRYPTION_ENABLED(sbi) (0)
-#endif
-
/* Number of quota types we support */
#define EXT4_MAXQUOTAS 3
@@ -2740,6 +2734,8 @@ int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,
void ext4_fname_free_filename(struct ext4_filename *fname);
+#define DUMMY_ENCRYPTION_ENABLED(sbi) ((sbi)->s_dummy_enc_policy.policy != NULL)
+
#else /* !CONFIG_FS_ENCRYPTION */
static inline int ext4_fname_setup_filename(struct inode *dir,
const struct qstr *iname,
@@ -2772,6 +2768,8 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname)
fname->cf_name.name = NULL;
#endif
}
+
+#define DUMMY_ENCRYPTION_ENABLED(sbi) (0)
#endif /* !CONFIG_FS_ENCRYPTION */
/* dir.c */
--
2.31.1
We have a macro which test is dummy_enc_policy is enabled or not.
Use that instead.
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ext4/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e7e5c9c057d7..73fb54c3efd3 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2685,7 +2685,7 @@ static int ext4_check_opt_consistency(struct fs_context *fc,
* it to be specified during remount, but only if there is no change.
*/
if ((ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION) &&
- is_remount && !sbi->s_dummy_enc_policy.policy) {
+ is_remount && !DUMMY_ENCRYPTION_ENABLED(sbi)) {
ext4_msg(NULL, KERN_WARNING,
"Can't set test_dummy_encryption on remount");
return -1;
--
2.31.1
Simplify the code by opencoding ext4_set_test_dummy_encryption(), since
there is only one caller of it and ext4_** variant in itself does nothing
special other than calling fscrypt_set_test_dummy_encryption(). This can
be done directly by opencoding it in the caller.
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ext4/super.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 8bb5fa15a013..e7e5c9c057d7 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1863,24 +1863,6 @@ ext4_sb_read_encoding(const struct ext4_super_block *es)
}
#endif
-static int ext4_set_test_dummy_encryption(struct super_block *sb, char *arg)
-{
-#ifdef CONFIG_FS_ENCRYPTION
- struct ext4_sb_info *sbi = EXT4_SB(sb);
- int err;
-
- err = fscrypt_set_test_dummy_encryption(sb, arg,
- &sbi->s_dummy_enc_policy);
- if (err) {
- ext4_msg(sb, KERN_WARNING,
- "Error while setting test dummy encryption [%d]", err);
- return err;
- }
- ext4_msg(sb, KERN_WARNING, "Test dummy encryption mode enabled");
-#endif
- return 0;
-}
-
#define EXT4_SPEC_JQUOTA (1 << 0)
#define EXT4_SPEC_JQFMT (1 << 1)
#define EXT4_SPEC_DATAJ (1 << 2)
@@ -2795,8 +2777,20 @@ static int ext4_apply_options(struct fs_context *fc, struct super_block *sb)
ext4_apply_quota_options(fc, sb);
- if (ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION)
- ret = ext4_set_test_dummy_encryption(sb, ctx->test_dummy_enc_arg);
+ if (IS_ENABLED(CONFIG_FS_ENCRYPTION) &&
+ (ctx->spec & EXT4_SPEC_DUMMY_ENCRYPTION)) {
+
+ ret = fscrypt_set_test_dummy_encryption(sb,
+ ctx->test_dummy_enc_arg,
+ &sbi->s_dummy_enc_policy);
+ if (ret) {
+ ext4_msg(sb, KERN_WARNING, "Error while setting test dummy encryption [%d]",
+ ret);
+ return ret;
+ }
+
+ ext4_msg(sb, KERN_WARNING, "Test dummy encryption mode enabled");
+ }
return ret;
}
--
2.31.1
Provide definition of fscrypt_set_test_dummy_encryption() when
CONFIG_FS_ENCRYPTION is disabled. This then returns -EOPNOTSUPP.
Signed-off-by: Ritesh Harjani <[email protected]>
---
include/linux/fscrypt.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 91ea9477e9bd..18dd6048bab3 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -467,6 +467,13 @@ static inline int fscrypt_set_context(struct inode *inode, void *fs_data)
struct fscrypt_dummy_policy {
};
+static inline int fscrypt_set_test_dummy_encryption(struct super_block *sb,
+ const char *arg,
+ struct fscrypt_dummy_policy *dummy_policy)
+{
+ return -EOPNOTSUPP;
+}
+
static inline void fscrypt_show_test_dummy_encryption(struct seq_file *seq,
char sep,
struct super_block *sb)
--
2.31.1
Some of these functions when CONFIG_FS_ENCRYPTION is enabled are not
really inline (let compiler be the best judge of it).
Remove inline and move them into ext4_crypto.c where they should be present.
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ext4/ext4.h | 70 ++++---------------------------------------
fs/ext4/ext4_crypto.c | 65 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 64 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 8bac8af25ed8..caf154db4680 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2731,73 +2731,15 @@ extern int ext4_fname_setup_ci_filename(struct inode *dir,
extern const struct fscrypt_operations ext4_cryptops;
#ifdef CONFIG_FS_ENCRYPTION
-static inline void ext4_fname_from_fscrypt_name(struct ext4_filename *dst,
- const struct fscrypt_name *src)
-{
- memset(dst, 0, sizeof(*dst));
-
- dst->usr_fname = src->usr_fname;
- dst->disk_name = src->disk_name;
- dst->hinfo.hash = src->hash;
- dst->hinfo.minor_hash = src->minor_hash;
- dst->crypto_buf = src->crypto_buf;
-}
-
-static inline int ext4_fname_setup_filename(struct inode *dir,
- const struct qstr *iname,
- int lookup,
- struct ext4_filename *fname)
-{
- struct fscrypt_name name;
- int err;
+int ext4_fname_setup_filename(struct inode *dir,
+ const struct qstr *iname, int lookup,
+ struct ext4_filename *fname);
- err = fscrypt_setup_filename(dir, iname, lookup, &name);
- if (err)
- return err;
+int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,
+ struct ext4_filename *fname);
- ext4_fname_from_fscrypt_name(fname, &name);
+void ext4_fname_free_filename(struct ext4_filename *fname);
-#if IS_ENABLED(CONFIG_UNICODE)
- err = ext4_fname_setup_ci_filename(dir, iname, fname);
-#endif
- return err;
-}
-
-static inline int ext4_fname_prepare_lookup(struct inode *dir,
- struct dentry *dentry,
- struct ext4_filename *fname)
-{
- struct fscrypt_name name;
- int err;
-
- err = fscrypt_prepare_lookup(dir, dentry, &name);
- if (err)
- return err;
-
- ext4_fname_from_fscrypt_name(fname, &name);
-
-#if IS_ENABLED(CONFIG_UNICODE)
- err = ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname);
-#endif
- return err;
-}
-
-static inline void ext4_fname_free_filename(struct ext4_filename *fname)
-{
- struct fscrypt_name name;
-
- name.crypto_buf = fname->crypto_buf;
- fscrypt_free_filename(&name);
-
- fname->crypto_buf.name = NULL;
- fname->usr_fname = NULL;
- fname->disk_name.name = NULL;
-
-#if IS_ENABLED(CONFIG_UNICODE)
- kfree(fname->cf_name.name);
- fname->cf_name.name = NULL;
-#endif
-}
#else /* !CONFIG_FS_ENCRYPTION */
static inline int ext4_fname_setup_filename(struct inode *dir,
const struct qstr *iname,
diff --git a/fs/ext4/ext4_crypto.c b/fs/ext4/ext4_crypto.c
index e5413c0970ee..7e89f86a4429 100644
--- a/fs/ext4/ext4_crypto.c
+++ b/fs/ext4/ext4_crypto.c
@@ -6,6 +6,71 @@
#include "xattr.h"
#include "ext4_jbd2.h"
+void ext4_fname_from_fscrypt_name(struct ext4_filename *dst,
+ const struct fscrypt_name *src)
+{
+ memset(dst, 0, sizeof(*dst));
+
+ dst->usr_fname = src->usr_fname;
+ dst->disk_name = src->disk_name;
+ dst->hinfo.hash = src->hash;
+ dst->hinfo.minor_hash = src->minor_hash;
+ dst->crypto_buf = src->crypto_buf;
+}
+
+int ext4_fname_setup_filename(struct inode *dir, const struct qstr *iname,
+ int lookup, struct ext4_filename *fname)
+{
+ struct fscrypt_name name;
+ int err;
+
+ err = fscrypt_setup_filename(dir, iname, lookup, &name);
+ if (err)
+ return err;
+
+ ext4_fname_from_fscrypt_name(fname, &name);
+
+#if IS_ENABLED(CONFIG_UNICODE)
+ err = ext4_fname_setup_ci_filename(dir, iname, fname);
+#endif
+ return err;
+}
+
+int ext4_fname_prepare_lookup(struct inode *dir, struct dentry *dentry,
+ struct ext4_filename *fname)
+{
+ struct fscrypt_name name;
+ int err;
+
+ err = fscrypt_prepare_lookup(dir, dentry, &name);
+ if (err)
+ return err;
+
+ ext4_fname_from_fscrypt_name(fname, &name);
+
+#if IS_ENABLED(CONFIG_UNICODE)
+ err = ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname);
+#endif
+ return err;
+}
+
+void ext4_fname_free_filename(struct ext4_filename *fname)
+{
+ struct fscrypt_name name;
+
+ name.crypto_buf = fname->crypto_buf;
+ fscrypt_free_filename(&name);
+
+ fname->crypto_buf.name = NULL;
+ fname->usr_fname = NULL;
+ fname->disk_name.name = NULL;
+
+#if IS_ENABLED(CONFIG_UNICODE)
+ kfree(fname->cf_name.name);
+ fname->cf_name.name = NULL;
+#endif
+}
+
static int ext4_get_context(struct inode *inode, void *ctx, size_t len)
{
return ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
--
2.31.1
This is to cleanup super.c file which has grown quite large.
So, start moving ext4 crypto related code to where it should be in the first
place i.e. fs/ext4/ext4_crypto.c
Signed-off-by: Ritesh Harjani <[email protected]>
---
fs/ext4/Makefile | 1 +
fs/ext4/ext4.h | 3 +
fs/ext4/ext4_crypto.c | 127 ++++++++++++++++++++++++++++++++++++++++++
fs/ext4/super.c | 122 ----------------------------------------
4 files changed, 131 insertions(+), 122 deletions(-)
create mode 100644 fs/ext4/ext4_crypto.c
diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
index 7d89142e1421..b340fe5f849c 100644
--- a/fs/ext4/Makefile
+++ b/fs/ext4/Makefile
@@ -17,3 +17,4 @@ ext4-$(CONFIG_EXT4_FS_SECURITY) += xattr_security.o
ext4-inode-test-objs += inode-test.o
obj-$(CONFIG_EXT4_KUNIT_TESTS) += ext4-inode-test.o
ext4-$(CONFIG_FS_VERITY) += verity.o
+ext4-$(CONFIG_FS_ENCRYPTION) += ext4_crypto.o
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1d79012c5a5b..8bac8af25ed8 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2727,6 +2727,9 @@ extern int ext4_fname_setup_ci_filename(struct inode *dir,
struct ext4_filename *fname);
#endif
+/* ext4 encryption related stuff goes here ext4_crypto.c */
+extern const struct fscrypt_operations ext4_cryptops;
+
#ifdef CONFIG_FS_ENCRYPTION
static inline void ext4_fname_from_fscrypt_name(struct ext4_filename *dst,
const struct fscrypt_name *src)
diff --git a/fs/ext4/ext4_crypto.c b/fs/ext4/ext4_crypto.c
new file mode 100644
index 000000000000..e5413c0970ee
--- /dev/null
+++ b/fs/ext4/ext4_crypto.c
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/quotaops.h>
+
+#include "ext4.h"
+#include "xattr.h"
+#include "ext4_jbd2.h"
+
+static int ext4_get_context(struct inode *inode, void *ctx, size_t len)
+{
+ return ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
+ EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, ctx, len);
+}
+
+static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
+ void *fs_data)
+{
+ handle_t *handle = fs_data;
+ int res, res2, credits, retries = 0;
+
+ /*
+ * Encrypting the root directory is not allowed because e2fsck expects
+ * lost+found to exist and be unencrypted, and encrypting the root
+ * directory would imply encrypting the lost+found directory as well as
+ * the filename "lost+found" itself.
+ */
+ if (inode->i_ino == EXT4_ROOT_INO)
+ return -EPERM;
+
+ if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
+ return -EINVAL;
+
+ if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
+ return -EOPNOTSUPP;
+
+ res = ext4_convert_inline_data(inode);
+ if (res)
+ return res;
+
+ /*
+ * If a journal handle was specified, then the encryption context is
+ * being set on a new inode via inheritance and is part of a larger
+ * transaction to create the inode. Otherwise the encryption context is
+ * being set on an existing inode in its own transaction. Only in the
+ * latter case should the "retry on ENOSPC" logic be used.
+ */
+
+ if (handle) {
+ res = ext4_xattr_set_handle(handle, inode,
+ EXT4_XATTR_INDEX_ENCRYPTION,
+ EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
+ ctx, len, 0);
+ if (!res) {
+ ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
+ ext4_clear_inode_state(inode,
+ EXT4_STATE_MAY_INLINE_DATA);
+ /*
+ * Update inode->i_flags - S_ENCRYPTED will be enabled,
+ * S_DAX may be disabled
+ */
+ ext4_set_inode_flags(inode, false);
+ }
+ return res;
+ }
+
+ res = dquot_initialize(inode);
+ if (res)
+ return res;
+retry:
+ res = ext4_xattr_set_credits(inode, len, false /* is_create */,
+ &credits);
+ if (res)
+ return res;
+
+ handle = ext4_journal_start(inode, EXT4_HT_MISC, credits);
+ if (IS_ERR(handle))
+ return PTR_ERR(handle);
+
+ res = ext4_xattr_set_handle(handle, inode, EXT4_XATTR_INDEX_ENCRYPTION,
+ EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
+ ctx, len, 0);
+ if (!res) {
+ ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
+ /*
+ * Update inode->i_flags - S_ENCRYPTED will be enabled,
+ * S_DAX may be disabled
+ */
+ ext4_set_inode_flags(inode, false);
+ res = ext4_mark_inode_dirty(handle, inode);
+ if (res)
+ EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
+ }
+ res2 = ext4_journal_stop(handle);
+
+ if (res == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
+ goto retry;
+ if (!res)
+ res = res2;
+ return res;
+}
+
+static const union fscrypt_policy *ext4_get_dummy_policy(struct super_block *sb)
+{
+ return EXT4_SB(sb)->s_dummy_enc_policy.policy;
+}
+
+static bool ext4_has_stable_inodes(struct super_block *sb)
+{
+ return ext4_has_feature_stable_inodes(sb);
+}
+
+static void ext4_get_ino_and_lblk_bits(struct super_block *sb,
+ int *ino_bits_ret, int *lblk_bits_ret)
+{
+ *ino_bits_ret = 8 * sizeof(EXT4_SB(sb)->s_es->s_inodes_count);
+ *lblk_bits_ret = 8 * sizeof(ext4_lblk_t);
+}
+
+const struct fscrypt_operations ext4_cryptops = {
+ .key_prefix = "ext4:",
+ .get_context = ext4_get_context,
+ .set_context = ext4_set_context,
+ .get_dummy_policy = ext4_get_dummy_policy,
+ .empty_dir = ext4_empty_dir,
+ .has_stable_inodes = ext4_has_stable_inodes,
+ .get_ino_and_lblk_bits = ext4_get_ino_and_lblk_bits,
+};
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ae98b07285d2..8bb5fa15a013 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1488,128 +1488,6 @@ static int ext4_nfs_commit_metadata(struct inode *inode)
return ext4_write_inode(inode, &wbc);
}
-#ifdef CONFIG_FS_ENCRYPTION
-static int ext4_get_context(struct inode *inode, void *ctx, size_t len)
-{
- return ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
- EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, ctx, len);
-}
-
-static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
- void *fs_data)
-{
- handle_t *handle = fs_data;
- int res, res2, credits, retries = 0;
-
- /*
- * Encrypting the root directory is not allowed because e2fsck expects
- * lost+found to exist and be unencrypted, and encrypting the root
- * directory would imply encrypting the lost+found directory as well as
- * the filename "lost+found" itself.
- */
- if (inode->i_ino == EXT4_ROOT_INO)
- return -EPERM;
-
- if (WARN_ON_ONCE(IS_DAX(inode) && i_size_read(inode)))
- return -EINVAL;
-
- if (ext4_test_inode_flag(inode, EXT4_INODE_DAX))
- return -EOPNOTSUPP;
-
- res = ext4_convert_inline_data(inode);
- if (res)
- return res;
-
- /*
- * If a journal handle was specified, then the encryption context is
- * being set on a new inode via inheritance and is part of a larger
- * transaction to create the inode. Otherwise the encryption context is
- * being set on an existing inode in its own transaction. Only in the
- * latter case should the "retry on ENOSPC" logic be used.
- */
-
- if (handle) {
- res = ext4_xattr_set_handle(handle, inode,
- EXT4_XATTR_INDEX_ENCRYPTION,
- EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
- ctx, len, 0);
- if (!res) {
- ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
- ext4_clear_inode_state(inode,
- EXT4_STATE_MAY_INLINE_DATA);
- /*
- * Update inode->i_flags - S_ENCRYPTED will be enabled,
- * S_DAX may be disabled
- */
- ext4_set_inode_flags(inode, false);
- }
- return res;
- }
-
- res = dquot_initialize(inode);
- if (res)
- return res;
-retry:
- res = ext4_xattr_set_credits(inode, len, false /* is_create */,
- &credits);
- if (res)
- return res;
-
- handle = ext4_journal_start(inode, EXT4_HT_MISC, credits);
- if (IS_ERR(handle))
- return PTR_ERR(handle);
-
- res = ext4_xattr_set_handle(handle, inode, EXT4_XATTR_INDEX_ENCRYPTION,
- EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
- ctx, len, 0);
- if (!res) {
- ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
- /*
- * Update inode->i_flags - S_ENCRYPTED will be enabled,
- * S_DAX may be disabled
- */
- ext4_set_inode_flags(inode, false);
- res = ext4_mark_inode_dirty(handle, inode);
- if (res)
- EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
- }
- res2 = ext4_journal_stop(handle);
-
- if (res == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
- goto retry;
- if (!res)
- res = res2;
- return res;
-}
-
-static const union fscrypt_policy *ext4_get_dummy_policy(struct super_block *sb)
-{
- return EXT4_SB(sb)->s_dummy_enc_policy.policy;
-}
-
-static bool ext4_has_stable_inodes(struct super_block *sb)
-{
- return ext4_has_feature_stable_inodes(sb);
-}
-
-static void ext4_get_ino_and_lblk_bits(struct super_block *sb,
- int *ino_bits_ret, int *lblk_bits_ret)
-{
- *ino_bits_ret = 8 * sizeof(EXT4_SB(sb)->s_es->s_inodes_count);
- *lblk_bits_ret = 8 * sizeof(ext4_lblk_t);
-}
-
-static const struct fscrypt_operations ext4_cryptops = {
- .key_prefix = "ext4:",
- .get_context = ext4_get_context,
- .set_context = ext4_set_context,
- .get_dummy_policy = ext4_get_dummy_policy,
- .empty_dir = ext4_empty_dir,
- .has_stable_inodes = ext4_has_stable_inodes,
- .get_ino_and_lblk_bits = ext4_get_ino_and_lblk_bits,
-};
-#endif
-
#ifdef CONFIG_QUOTA
static const char * const quotatypes[] = INITQFNAMES;
#define QTYPE2NAME(t) (quotatypes[t])
--
2.31.1
On 22/04/21 12:24AM, Eric Biggers wrote:
> On Thu, Apr 21, 2022 at 10:53:16AM +0530, Ritesh Harjani wrote:
> > Hello,
> >
> > This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
> > This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
> >
> > Testing
> > =========
> > 1. Tested "-g encrypt" with default configs.
> > 2. Compiled tested on x86 & Power.
> >
> >
> > Ritesh Harjani (6):
> > fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
> > ext4: Move ext4 crypto code to its own file ext4_crypto.c
> > ext4: Directly opencode ext4_set_test_dummy_encryption
> > ext4: Cleanup function defs from ext4.h into ext4_crypto.c
> > ext4: Move all encryption related into a common #ifdef
> > ext4: Use provided macro for checking dummy_enc_policy
> >
> > fs/ext4/Makefile | 1 +
> > fs/ext4/ext4.h | 81 +++--------------
> > fs/ext4/ext4_crypto.c | 192 ++++++++++++++++++++++++++++++++++++++++
> > fs/ext4/super.c | 158 ++++-----------------------------
> > include/linux/fscrypt.h | 7 ++
> > 5 files changed, 227 insertions(+), 212 deletions(-)
> > create mode 100644 fs/ext4/ext4_crypto.c
>
> How about calling it crypto.c instead of ext4_crypto.c? It is already in the
> ext4 directory, so ext4 is implied.
Sure will do so in next revision which can also cover any other comments from
others.
>
> Otherwise this patchset looks good to me.
Sure thanks. May I add your Reviewed-by in next version if I don't see any major
changes?
>
> Did you consider moving any of the other CONFIG_FS_ENCRYPTION code blocks into
> the new file as well? The implementation of FS_IOC_GET_ENCRYPTION_PWSALT might
> be a good candidate too.
Sure, thanks for pointing out. I will take a look at it for next patch series or
so.
-ritesh
On Thu, Apr 21, 2022 at 10:53:16AM +0530, Ritesh Harjani wrote:
> Hello,
>
> This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
> This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
>
> Testing
> =========
> 1. Tested "-g encrypt" with default configs.
> 2. Compiled tested on x86 & Power.
>
>
> Ritesh Harjani (6):
> fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
> ext4: Move ext4 crypto code to its own file ext4_crypto.c
> ext4: Directly opencode ext4_set_test_dummy_encryption
> ext4: Cleanup function defs from ext4.h into ext4_crypto.c
> ext4: Move all encryption related into a common #ifdef
> ext4: Use provided macro for checking dummy_enc_policy
>
> fs/ext4/Makefile | 1 +
> fs/ext4/ext4.h | 81 +++--------------
> fs/ext4/ext4_crypto.c | 192 ++++++++++++++++++++++++++++++++++++++++
> fs/ext4/super.c | 158 ++++-----------------------------
> include/linux/fscrypt.h | 7 ++
> 5 files changed, 227 insertions(+), 212 deletions(-)
> create mode 100644 fs/ext4/ext4_crypto.c
How about calling it crypto.c instead of ext4_crypto.c? It is already in the
ext4 directory, so ext4 is implied.
Otherwise this patchset looks good to me.
Did you consider moving any of the other CONFIG_FS_ENCRYPTION code blocks into
the new file as well? The implementation of FS_IOC_GET_ENCRYPTION_PWSALT might
be a good candidate too.
- Eric
On Thu, Apr 21, 2022 at 10:53:16AM +0530, Ritesh Harjani wrote:
> Hello,
>
> This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
> This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
>
> Testing
> =========
> 1. Tested "-g encrypt" with default configs.
> 2. Compiled tested on x86 & Power.
>
>
> Ritesh Harjani (6):
> fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
> ext4: Move ext4 crypto code to its own file ext4_crypto.c
> ext4: Directly opencode ext4_set_test_dummy_encryption
> ext4: Cleanup function defs from ext4.h into ext4_crypto.c
> ext4: Move all encryption related into a common #ifdef
> ext4: Use provided macro for checking dummy_enc_policy
FYI, the patchset
https://lore.kernel.org/linux-ext4/[email protected]
I just sent out cleans up how the test_dummy_encryption mount option is handled.
It would supersede patches 1, 3, 5, and 6 of this series (since those all only
deal with test_dummy_encryption-related code).
To avoid conflicting changes, maybe you should just focus on your patches 2 and
4 for now, along with possibly FS_IOC_GET_ENCRYPTION_PWSALT as I mentioned?
There shouldn't be any overlap that way.
- Eric
On 22/05/01 12:18AM, Eric Biggers wrote:
> On Thu, Apr 21, 2022 at 10:53:16AM +0530, Ritesh Harjani wrote:
> > Hello,
> >
> > This is 1st in the series to cleanup ext4/super.c, since it has grown quite large.
> > This moves out crypto related ops and few definitions to fs/ext4/ext4_crypto.c
> >
> > Testing
> > =========
> > 1. Tested "-g encrypt" with default configs.
> > 2. Compiled tested on x86 & Power.
> >
> >
> > Ritesh Harjani (6):
> > fscrypt: Provide definition of fscrypt_set_test_dummy_encryption
> > ext4: Move ext4 crypto code to its own file ext4_crypto.c
> > ext4: Directly opencode ext4_set_test_dummy_encryption
> > ext4: Cleanup function defs from ext4.h into ext4_crypto.c
> > ext4: Move all encryption related into a common #ifdef
> > ext4: Use provided macro for checking dummy_enc_policy
>
> FYI, the patchset
> https://lore.kernel.org/linux-ext4/[email protected]
> I just sent out cleans up how the test_dummy_encryption mount option is handled.
> It would supersede patches 1, 3, 5, and 6 of this series (since those all only
> deal with test_dummy_encryption-related code).
Sure got it.
>
> To avoid conflicting changes, maybe you should just focus on your patches 2 and
> 4 for now, along with possibly FS_IOC_GET_ENCRYPTION_PWSALT as I mentioned?
> There shouldn't be any overlap that way.
Yes, agreed with al of above points Eric. Will make the changes and send a new
revision.
-ritesh