2019-08-23 23:23:01

by Chandan Rajendra

[permalink] [raw]
Subject: [PATCH V5 0/7] Consolidate FS read I/O callbacks code

This patchset moves the "FS read I/O callbacks" code into a file of its
own (i.e. fs/read_callbacks.c) and modifies the generic
do_mpage_readpge() to make use of the functionality provided.

"FS read I/O callbacks" code implements the state machine that needs
to be executed after reading data from files that are encrypted and/or
have verity metadata associated with them.

With these changes in place, the patchset changes Ext4 to use
mpage_readpage[s] instead of its own custom ext4_readpage[s]()
functions. This is done to reduce duplication of code across
filesystems. Also, "FS read I/O callbacks" source files will be built
only if CONFIG_FS_ENCRYPTION is enabled.

The patchset also modifies fs/buffer.c to get file
encryption/decryption to work with subpage-sized blocks.

The patches can also be obtained from
https://github.com/chandanr/linux.git at branch subpage-encryption-v5.

Changelog:
V4 -> V5:
1. Since F2FS uses its own workqueue and also since its
decompression logic isn't an fs independent entity like fscrypt or
fsverity, this patchset drops support for F2FS.
The patchset still helps in removing a copy of
do_mpage_readpage() from Ext4 (i.e. ext4_readpage()) and also
prevents a copy of block_read_full_page() from being added into
Ext4 by adding support to "read callbacks" API invocations into
block_read_full_page().

V3 -> V4:
1. A new buffer_head flag (i.e. BH_Read_Cb) is introduced to reliably
check if a buffer head's content has to be decrypted.
2. Fix layering violation. Now the code flow for decryption happens as shown below,
FS => read callbacks => fscrypt
3. Select FS_READ_CALLBACKS from FS specific kconfig file if FS_ENCRYPTION
is enabled.
4. Make 'struct read_callbacks_ctx' an opaque structure.
5. Make use of FS' endio function rather than implementing one in read
callbacks.
6. Make read_callbacks.h self-contained.
7. Split patchset to separate out ext4 and f2fs changes.

V2 -> V3:
1. Split the V2 patch "Consolidate 'read callbacks' into a new file" into
three patches,
- Introduce the read_callbacks functionality.
- Convert encryption to use read_callbacks.
- Remove union from struct fscrypt_context.
2. fs/Kconfig
Do not explicitly set the default value of 'n' for FS_READ_CALLBACKS.
3. fs/crypto/Kconfig
Select CONFIG_FS_READ_CALLBACKS only if CONFIG_BLOCK is selected.
4. Remove verity associated code in read_callbacks code.
5. Introduce a callback argument to read_callbacks_setup() function
which gets invoked for each page for bio. F2FS uses this to perform
custom operations like decrementing the value of f2fs_sb_info->nr_pages[].
6. Encapsulate the details of "read callbacks" (e.g. Usage of "struct
read_callbacks *ctx") within its own functions. When CONFIG_FS_READ_CALLBACKS
is set to 'n', the corresponding stub functions return approriate error
values.
7. Split fscrypt_decrypt() function into fscrypt_decrypt_bio() and
fscrypt_decrypt_bh().
8. Split end_read_callbacks() function into end_read_callbacks_bio() and
end_read_callbacks_bh().

V1 -> V2:
1. Removed the phrase "post_read_process" from file names and
functions. Instead we now use the phrase "read_callbacks" in its
place.
2. When performing changes associated with (1), the changes made by
the patch "Remove the term 'bio' from post read processing" are
made in the earlier patch "Consolidate 'read callbacks' into a new
file". Hence the patch "Remove the term 'bio' from post read
processing" is removed from the patchset.

RFC V2 -> V1:
1. Test and verify FS_CFLG_OWN_PAGES subset of fscrypt_encrypt_page()
code by executing fstests on UBIFS.
2. Implement F2fs function call back to check if the contents of a
page holding a verity file's data needs to be verified.

RFC V1 -> RFC V2:
1. Describe the purpose of "Post processing code" in the cover letter.
2. Fix build errors when CONFIG_FS_VERITY is enabled.

Chandan Rajendra (7):
buffer_head: Introduce BH_Read_Cb flag
FS: Introduce read callbacks
fs/mpage.c: Integrate read callbacks
fs/buffer.c: add decryption support via read_callbacks
ext4: Wire up ext4_readpage[s] to use mpage_readpage[s]
ext4: Enable encryption for subpage-sized blocks
fscrypt: remove struct fscrypt_ctx

Documentation/filesystems/fscrypt.rst | 4 +-
fs/Kconfig | 3 +
fs/Makefile | 1 +
fs/buffer.c | 33 ++-
fs/crypto/bio.c | 18 --
fs/crypto/crypto.c | 89 +-------
fs/crypto/fscrypt_private.h | 3 -
fs/ext4/Kconfig | 1 +
fs/ext4/Makefile | 2 +-
fs/ext4/inode.c | 5 +-
fs/ext4/readpage.c | 295 --------------------------
fs/ext4/super.c | 7 -
fs/f2fs/Kconfig | 1 +
fs/mpage.c | 24 ++-
fs/read_callbacks.c | 285 +++++++++++++++++++++++++
include/linux/buffer_head.h | 2 +
include/linux/fscrypt.h | 32 ---
include/linux/read_callbacks.h | 48 +++++
18 files changed, 391 insertions(+), 462 deletions(-)
delete mode 100644 fs/ext4/readpage.c
create mode 100644 fs/read_callbacks.c
create mode 100644 include/linux/read_callbacks.h

--
2.19.1


2019-08-23 23:23:01

by Chandan Rajendra

[permalink] [raw]
Subject: [PATCH V5 2/7] FS: Introduce read callbacks

Read callbacks implements a state machine to be executed after a
buffered read I/O is completed. They help in further processing the file
data read from the backing store. Currently, decryption is the only post
processing step to be supported.

The execution of the state machine is to be initiated by the endio
function associated with the read operation.

Signed-off-by: Chandan Rajendra <[email protected]>
---
fs/Kconfig | 3 +
fs/Makefile | 1 +
fs/ext4/Kconfig | 1 +
fs/f2fs/Kconfig | 1 +
fs/read_callbacks.c | 285 +++++++++++++++++++++++++++++++++
include/linux/read_callbacks.h | 48 ++++++
6 files changed, 339 insertions(+)
create mode 100644 fs/read_callbacks.c
create mode 100644 include/linux/read_callbacks.h

diff --git a/fs/Kconfig b/fs/Kconfig
index 3e6d3101f3ff..2d96a58d7418 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -20,6 +20,9 @@ if BLOCK
config FS_IOMAP
bool

+config FS_READ_CALLBACKS
+ bool
+
source "fs/ext2/Kconfig"
source "fs/ext4/Kconfig"
source "fs/jbd2/Kconfig"
diff --git a/fs/Makefile b/fs/Makefile
index 427fec226fae..942808f83472 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_SYSCTL) += drop_caches.o

obj-$(CONFIG_FHANDLE) += fhandle.o
obj-$(CONFIG_FS_IOMAP) += iomap.o
+obj-$(CONFIG_FS_READ_CALLBACKS) += read_callbacks.o

obj-y += quota/

diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
index 06f77ca7f36e..2e24df67f085 100644
--- a/fs/ext4/Kconfig
+++ b/fs/ext4/Kconfig
@@ -38,6 +38,7 @@ config EXT4_FS
select CRYPTO
select CRYPTO_CRC32C
select FS_IOMAP
+ select FS_READ_CALLBACKS if FS_ENCRYPTION
help
This is the next generation of the ext3 filesystem.

diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index e57cc754d543..1e1424940d1b 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -4,6 +4,7 @@ config F2FS_FS
select CRYPTO
select CRYPTO_CRC32
select F2FS_FS_XATTR if FS_ENCRYPTION
+ select FS_READ_CALLBACKS if FS_ENCRYPTION
help
F2FS is based on Log-structured File System (LFS), which supports
versatile "flash-friendly" features. The design has been focused on
diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
new file mode 100644
index 000000000000..32d9b8d17964
--- /dev/null
+++ b/fs/read_callbacks.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This file tracks the state machine that needs to be executed after reading
+ * data from files that are encrypted and/or have verity metadata associated
+ * with them.
+ */
+#include <linux/module.h>
+#include <linux/mm.h>
+#include <linux/pagemap.h>
+#include <linux/bio.h>
+#include <linux/buffer_head.h>
+#include <linux/fscrypt.h>
+#include <linux/read_callbacks.h>
+#include <linux/blk_types.h>
+
+#define NUM_PREALLOC_READ_CALLBACK_CTXS 128
+
+static struct kmem_cache *read_callbacks_ctx_cache;
+static mempool_t *read_callbacks_ctx_pool;
+
+/* Read callback state machine steps */
+enum read_callbacks_step {
+ STEP_INITIAL = 0,
+ STEP_DECRYPT,
+};
+
+struct read_callbacks_ctx {
+ struct inode *inode;
+ struct bio *bio;
+ struct buffer_head *bh;
+ union {
+ end_bio_func_t end_bio;
+
+ struct {
+ end_bh_func_t end_bh;
+ int bh_uptodate;
+ };
+ };
+ struct work_struct work;
+ unsigned int enabled_steps;
+ enum read_callbacks_step cur_step;
+};
+
+static void read_callbacks(struct read_callbacks_ctx *ctx);
+
+static void free_read_callbacks_ctx(struct read_callbacks_ctx *ctx)
+{
+ mempool_free(ctx, read_callbacks_ctx_pool);
+}
+
+static struct read_callbacks_ctx *get_read_callbacks_ctx(struct inode *inode)
+{
+ struct read_callbacks_ctx *ctx = NULL;
+ unsigned int enabled_steps = 0;
+
+ if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
+ enabled_steps |= 1 << STEP_DECRYPT;
+
+ if (enabled_steps) {
+ ctx = mempool_alloc(read_callbacks_ctx_pool, GFP_NOFS);
+ if (!ctx)
+ return ERR_PTR(-ENOMEM);
+
+ ctx->inode = inode;
+ ctx->enabled_steps = enabled_steps;
+ ctx->cur_step = STEP_INITIAL;
+ }
+
+ return ctx;
+}
+
+static void decrypt_bio(struct bio *bio)
+{
+ struct bio_vec *bv;
+ int i;
+ struct bvec_iter_all iter_all;
+
+ bio_for_each_segment_all(bv, bio, i, iter_all) {
+ struct page *page = bv->bv_page;
+ int ret = fscrypt_decrypt_pagecache_blocks(page, bv->bv_len,
+ bv->bv_offset);
+ if (ret)
+ SetPageError(page);
+ }
+}
+
+static void decrypt_bh(struct buffer_head *bh)
+{
+ struct page *page;
+ int ret;
+
+ page = bh->b_page;
+
+ ret = fscrypt_decrypt_pagecache_blocks(page, bh->b_size,
+ bh_offset(bh));
+ if (ret)
+ SetPageError(page);
+}
+
+static void decrypt_work(struct work_struct *work)
+{
+ struct read_callbacks_ctx *ctx =
+ container_of(work, struct read_callbacks_ctx, work);
+
+ if (ctx->bio)
+ decrypt_bio(ctx->bio);
+ else
+ decrypt_bh(ctx->bh);
+
+ read_callbacks(ctx);
+}
+
+static void read_callbacks(struct read_callbacks_ctx *ctx)
+{
+ /*
+ * We use different work queues for decryption and for verity because
+ * verity may require reading metadata pages that need decryption, and
+ * we shouldn't recurse to the same workqueue.
+ */
+ switch (++ctx->cur_step) {
+ case STEP_DECRYPT:
+ if (ctx->enabled_steps & (1 << STEP_DECRYPT)) {
+ INIT_WORK(&ctx->work, decrypt_work);
+ fscrypt_enqueue_decrypt_work(&ctx->work);
+ return;
+ }
+ ctx->cur_step++;
+ /* fall-through */
+ default:
+ if (ctx->bio)
+ ctx->end_bio(ctx->bio);
+ else
+ ctx->end_bh(ctx->bh, ctx->bh_uptodate);
+
+ mempool_free(ctx, read_callbacks_ctx_pool);
+ }
+}
+
+/**
+ * read_callbacks_setup_bio() - Initialize the read callbacks state machine
+ * @inode: The file on which read I/O is performed.
+ * @bio: bio holding page cache pages on which read I/O is performed.
+ * @bh: Buffer head on which read I/O is performed.
+ * @page_op: Function to perform filesystem specific operations on a page.
+ *
+ * Based on the nature of the file's data (e.g. encrypted), this function
+ * computes the steps that need to be performed after data is read of the
+ * backing disk. This information is saved in a context structure. A pointer
+ * to the context structure is then stored in bio->bi_private for later
+ * usage.
+ *
+ * Return: 0 on success; An error code on failure.
+ */
+int read_callbacks_setup_bio(struct inode *inode, struct bio *bio)
+{
+ struct read_callbacks_ctx *ctx = get_read_callbacks_ctx(inode);
+
+ if (ctx) {
+ if (IS_ERR(ctx))
+ return PTR_ERR(ctx);
+ ctx->bio = bio;
+ ctx->bh = NULL;
+ bio->bi_private = ctx;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(read_callbacks_setup_bio);
+
+/**
+ * read_callbacks_setup_bh() - Initialize the read callbacks state machine
+ * @inode: The file on which read I/O is performed.
+ * @bio: bio holding page cache pages on which read I/O is performed.
+ * @bh: Buffer head on which read I/O is performed.
+ * @page_op: Function to perform filesystem specific operations on a page.
+ *
+ * Based on the nature of the file's data (e.g. encrypted), this function
+ * computes the steps that need to be performed after data is read of the
+ * backing disk. This information is saved in a context structure. A pointer
+ * to the context structure is then stored in bio->bi_private for later
+ * usage.
+ *
+ * Return: 0 on success; An error code on failure.
+ */
+int read_callbacks_setup_bh(struct inode *inode, struct buffer_head *bh)
+{
+ struct read_callbacks_ctx *ctx = get_read_callbacks_ctx(inode);
+
+ if (ctx) {
+ if (IS_ERR(ctx))
+ return PTR_ERR(ctx);
+ ctx->bio = NULL;
+ ctx->bh = bh;
+ bh->b_private = ctx;
+ set_buffer_read_cb(bh);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(read_callbacks_setup_bh);
+
+/**
+ * read_callbacks_endio_bio() - Initiate the read callbacks state machine.
+ * @bio: bio on which read I/O operation has just been completed.
+ * @end_bio: Callback to invoke on @bio after state machine completion.
+ *
+ * Initiates the execution of the read callbacks state machine when the read
+ * operation has been completed sucessfully. If there was an error associated
+ * with the bio, this function frees the read callbacks context structure stored
+ * in bio->bi_private (if any).
+ *
+ * If read callbacks state machine isn't executed, we end up invoking the
+ * @end_bio function passed in as an argument.
+ */
+void read_callbacks_endio_bio(struct bio *bio, end_bio_func_t end_bio)
+{
+ struct read_callbacks_ctx *ctx = bio->bi_private;
+
+ if (ctx) {
+ if (!bio->bi_status) {
+ ctx->end_bio = end_bio;
+ read_callbacks(ctx);
+ return;
+ }
+
+ free_read_callbacks_ctx(ctx);
+ }
+
+ end_bio(bio);
+}
+EXPORT_SYMBOL(read_callbacks_endio_bio);
+
+/**
+ * read_callbacks_endio_bh() - Initiate the read callbacks state machine.
+ * @bh: buffer head on which read I/O operation has just been completed.
+ * @uptodate: Buffer head's I/O status.
+ * @end_bh: Callback to invoke on on @bh after state machine completion.
+ *
+ * Initiates the execution of the read callbacks state machine when the read
+ * operation has been completed sucessfully. If there was an error associated
+ * with the buffer head, this function frees the read callbacks context
+ * structure stored in bh->b_private (if any).
+ *
+ * If read callbacks state machine isn't executed, we end up invoking the
+ * @end_bh function passed in as an argument.
+ */
+void read_callbacks_endio_bh(struct buffer_head *bh, int uptodate, end_bh_func_t end_bh)
+{
+ struct read_callbacks_ctx *ctx = bh->b_private;
+
+ if (buffer_read_cb(bh)) {
+ clear_buffer_read_cb(bh);
+ if (uptodate) {
+ ctx->end_bh = end_bh;
+ ctx->bh_uptodate = uptodate;
+ read_callbacks(ctx);
+ return;
+ }
+
+ free_read_callbacks_ctx(ctx);
+ }
+
+ end_bh(bh, uptodate);
+}
+EXPORT_SYMBOL(read_callbacks_endio_bh);
+
+static int __init init_read_callbacks(void)
+{
+ read_callbacks_ctx_cache = KMEM_CACHE(read_callbacks_ctx, 0);
+ if (!read_callbacks_ctx_cache)
+ goto fail;
+ read_callbacks_ctx_pool =
+ mempool_create_slab_pool(NUM_PREALLOC_READ_CALLBACK_CTXS,
+ read_callbacks_ctx_cache);
+ if (!read_callbacks_ctx_pool)
+ goto fail_free_cache;
+ return 0;
+
+fail_free_cache:
+ kmem_cache_destroy(read_callbacks_ctx_cache);
+fail:
+ return -ENOMEM;
+}
+
+fs_initcall(init_read_callbacks);
diff --git a/include/linux/read_callbacks.h b/include/linux/read_callbacks.h
new file mode 100644
index 000000000000..0d709dd81b4e
--- /dev/null
+++ b/include/linux/read_callbacks.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _READ_CALLBACKS_H
+#define _READ_CALLBACKS_H
+
+#include <linux/blk_types.h>
+
+typedef void (*end_bio_func_t)(struct bio *bio);
+typedef void (*end_bh_func_t)(struct buffer_head *bh, int uptodate);
+
+#ifdef CONFIG_FS_READ_CALLBACKS
+int read_callbacks_setup_bio(struct inode *inode, struct bio *bio);
+int read_callbacks_setup_bh(struct inode *inode, struct buffer_head *bh);
+void read_callbacks_endio_bio(struct bio *bio, end_bio_func_t end_bio);
+void read_callbacks_endio_bh(struct buffer_head *bh, int uptodate, end_bh_func_t end_bh);
+
+static inline bool read_callbacks_failed(struct page *page)
+{
+ return PageError(page);
+}
+#else
+static inline int read_callbacks_setup_bio(struct inode *inode, struct bio *bio)
+{
+ return 0;
+}
+
+static inline int read_callbacks_setup_bh(struct inode *inode, struct buffer_head *bh)
+{
+ return 0;
+}
+
+static inline void read_callbacks_endio_bio(struct bio *bio,
+ end_bio_func_t end_bio)
+{
+ end_bio(bio);
+}
+
+static inline void read_callbacks_endio_bh(struct buffer_head *bh, int uptodate, end_bh_func_t end_bh)
+{
+ end_bh(bh, uptodate);
+}
+
+static inline bool read_callbacks_failed(struct page *page)
+{
+ return false;
+}
+#endif
+
+#endif /* _READ_CALLBACKS_H */
--
2.19.1

2019-08-23 23:23:03

by Chandan Rajendra

[permalink] [raw]
Subject: [PATCH V5 4/7] fs/buffer.c: add decryption support via read_callbacks

This commit sets up read_callbacks context for buffer heads whose
contents need to be decrypted on endio.

Signed-off-by: Chandan Rajendra <[email protected]>
---
fs/buffer.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index ce357602f471..96c4c9840746 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -45,6 +45,7 @@
#include <linux/bit_spinlock.h>
#include <linux/pagevec.h>
#include <linux/sched/mm.h>
+#include <linux/read_callbacks.h>
#include <trace/events/block.h>

static int fsync_buffers_list(spinlock_t *lock, struct list_head *list);
@@ -245,11 +246,7 @@ __find_get_block_slow(struct block_device *bdev, sector_t block)
return ret;
}

-/*
- * I/O completion handler for block_read_full_page() - pages
- * which come unlocked at the end of I/O.
- */
-static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
+void end_buffer_async_read(struct buffer_head *bh, int uptodate)
{
unsigned long flags;
struct buffer_head *first;
@@ -257,8 +254,6 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
struct page *page;
int page_uptodate = 1;

- BUG_ON(!buffer_async_read(bh));
-
page = bh->b_page;
if (uptodate) {
set_buffer_uptodate(bh);
@@ -306,6 +301,17 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate)
return;
}

+/*
+ * I/O completion handler for block_read_full_page(). Pages are unlocked
+ * after the I/O completes and the read callbacks (if any) have executed.
+ */
+static void __end_buffer_async_read(struct buffer_head *bh, int uptodate)
+{
+ BUG_ON(!buffer_async_read(bh));
+
+ read_callbacks_endio_bh(bh, uptodate, end_buffer_async_read);
+}
+
/*
* Completion handler for block_write_full_page() - pages which are unlocked
* during I/O, and which have PageWriteback cleared upon I/O completion.
@@ -378,7 +384,7 @@ EXPORT_SYMBOL(end_buffer_async_write);
*/
static void mark_buffer_async_read(struct buffer_head *bh)
{
- bh->b_end_io = end_buffer_async_read;
+ bh->b_end_io = __end_buffer_async_read;
set_buffer_async_read(bh);
}

@@ -2293,10 +2299,15 @@ int block_read_full_page(struct page *page, get_block_t *get_block)
*/
for (i = 0; i < nr; i++) {
bh = arr[i];
- if (buffer_uptodate(bh))
- end_buffer_async_read(bh, 1);
- else
+ if (buffer_uptodate(bh)) {
+ __end_buffer_async_read(bh, 1);
+ } else {
+ if (WARN_ON(read_callbacks_setup_bh(inode, bh))) {
+ __end_buffer_async_read(bh, 0);
+ continue;
+ }
submit_bh(REQ_OP_READ, 0, bh);
+ }
}
return 0;
}
--
2.19.1

2019-08-23 23:23:12

by Chandan Rajendra

[permalink] [raw]
Subject: [PATCH V5 7/7] fscrypt: remove struct fscrypt_ctx

Commit "fscrypt: remove the 'write' part of struct fscrypt_ctx" reduced
"struct fscrypt_ctx" to be used only for decryption. With "read
callbacks" being integrated into Ext4, we don't use "struct fscrypt_ctx"
anymore. Hence this commit removes the structure and the associated
code.

Signed-off-by: Chandan Rajendra <[email protected]>
---
fs/crypto/bio.c | 18 --------
fs/crypto/crypto.c | 89 +------------------------------------
fs/crypto/fscrypt_private.h | 3 --
include/linux/fscrypt.h | 32 -------------
4 files changed, 2 insertions(+), 140 deletions(-)

diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index b4f47b98ee6d..c01f068bf19b 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -51,24 +51,6 @@ void fscrypt_decrypt_bio(struct bio *bio)
}
EXPORT_SYMBOL(fscrypt_decrypt_bio);

-static void completion_pages(struct work_struct *work)
-{
- struct fscrypt_ctx *ctx = container_of(work, struct fscrypt_ctx, work);
- struct bio *bio = ctx->bio;
-
- __fscrypt_decrypt_bio(bio, true);
- fscrypt_release_ctx(ctx);
- bio_put(bio);
-}
-
-void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
-{
- INIT_WORK(&ctx->work, completion_pages);
- ctx->bio = bio;
- fscrypt_enqueue_decrypt_work(&ctx->work);
-}
-EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
-
int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
sector_t pblk, unsigned int len)
{
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index dcf630d7e446..2e5245f5639f 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -31,24 +31,16 @@
#include "fscrypt_private.h"

static unsigned int num_prealloc_crypto_pages = 32;
-static unsigned int num_prealloc_crypto_ctxs = 128;

module_param(num_prealloc_crypto_pages, uint, 0444);
MODULE_PARM_DESC(num_prealloc_crypto_pages,
"Number of crypto pages to preallocate");
-module_param(num_prealloc_crypto_ctxs, uint, 0444);
-MODULE_PARM_DESC(num_prealloc_crypto_ctxs,
- "Number of crypto contexts to preallocate");

static mempool_t *fscrypt_bounce_page_pool = NULL;

-static LIST_HEAD(fscrypt_free_ctxs);
-static DEFINE_SPINLOCK(fscrypt_ctx_lock);
-
static struct workqueue_struct *fscrypt_read_workqueue;
static DEFINE_MUTEX(fscrypt_init_mutex);

-static struct kmem_cache *fscrypt_ctx_cachep;
struct kmem_cache *fscrypt_info_cachep;

void fscrypt_enqueue_decrypt_work(struct work_struct *work)
@@ -57,62 +49,6 @@ void fscrypt_enqueue_decrypt_work(struct work_struct *work)
}
EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);

-/**
- * fscrypt_release_ctx() - Release a decryption context
- * @ctx: The decryption context to release.
- *
- * If the decryption context was allocated from the pre-allocated pool, return
- * it to that pool. Else, free it.
- */
-void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
-{
- unsigned long flags;
-
- if (ctx->flags & FS_CTX_REQUIRES_FREE_ENCRYPT_FL) {
- kmem_cache_free(fscrypt_ctx_cachep, ctx);
- } else {
- spin_lock_irqsave(&fscrypt_ctx_lock, flags);
- list_add(&ctx->free_list, &fscrypt_free_ctxs);
- spin_unlock_irqrestore(&fscrypt_ctx_lock, flags);
- }
-}
-EXPORT_SYMBOL(fscrypt_release_ctx);
-
-/**
- * fscrypt_get_ctx() - Get a decryption context
- * @gfp_flags: The gfp flag for memory allocation
- *
- * Allocate and initialize a decryption context.
- *
- * Return: A new decryption context on success; an ERR_PTR() otherwise.
- */
-struct fscrypt_ctx *fscrypt_get_ctx(gfp_t gfp_flags)
-{
- struct fscrypt_ctx *ctx;
- unsigned long flags;
-
- /*
- * First try getting a ctx from the free list so that we don't have to
- * call into the slab allocator.
- */
- spin_lock_irqsave(&fscrypt_ctx_lock, flags);
- ctx = list_first_entry_or_null(&fscrypt_free_ctxs,
- struct fscrypt_ctx, free_list);
- if (ctx)
- list_del(&ctx->free_list);
- spin_unlock_irqrestore(&fscrypt_ctx_lock, flags);
- if (!ctx) {
- ctx = kmem_cache_zalloc(fscrypt_ctx_cachep, gfp_flags);
- if (!ctx)
- return ERR_PTR(-ENOMEM);
- ctx->flags |= FS_CTX_REQUIRES_FREE_ENCRYPT_FL;
- } else {
- ctx->flags &= ~FS_CTX_REQUIRES_FREE_ENCRYPT_FL;
- }
- return ctx;
-}
-EXPORT_SYMBOL(fscrypt_get_ctx);
-
struct page *fscrypt_alloc_bounce_page(gfp_t gfp_flags)
{
return mempool_alloc(fscrypt_bounce_page_pool, gfp_flags);
@@ -399,11 +335,6 @@ const struct dentry_operations fscrypt_d_ops = {

static void fscrypt_destroy(void)
{
- struct fscrypt_ctx *pos, *n;
-
- list_for_each_entry_safe(pos, n, &fscrypt_free_ctxs, free_list)
- kmem_cache_free(fscrypt_ctx_cachep, pos);
- INIT_LIST_HEAD(&fscrypt_free_ctxs);
mempool_destroy(fscrypt_bounce_page_pool);
fscrypt_bounce_page_pool = NULL;
}
@@ -419,7 +350,7 @@ static void fscrypt_destroy(void)
*/
int fscrypt_initialize(unsigned int cop_flags)
{
- int i, res = -ENOMEM;
+ int res = -ENOMEM;

/* No need to allocate a bounce page pool if this FS won't use it. */
if (cop_flags & FS_CFLG_OWN_PAGES)
@@ -429,15 +360,6 @@ int fscrypt_initialize(unsigned int cop_flags)
if (fscrypt_bounce_page_pool)
goto already_initialized;

- for (i = 0; i < num_prealloc_crypto_ctxs; i++) {
- struct fscrypt_ctx *ctx;
-
- ctx = kmem_cache_zalloc(fscrypt_ctx_cachep, GFP_NOFS);
- if (!ctx)
- goto fail;
- list_add(&ctx->free_list, &fscrypt_free_ctxs);
- }
-
fscrypt_bounce_page_pool =
mempool_create_page_pool(num_prealloc_crypto_pages, 0);
if (!fscrypt_bounce_page_pool)
@@ -492,18 +414,12 @@ static int __init fscrypt_init(void)
if (!fscrypt_read_workqueue)
goto fail;

- fscrypt_ctx_cachep = KMEM_CACHE(fscrypt_ctx, SLAB_RECLAIM_ACCOUNT);
- if (!fscrypt_ctx_cachep)
- goto fail_free_queue;
-
fscrypt_info_cachep = KMEM_CACHE(fscrypt_info, SLAB_RECLAIM_ACCOUNT);
if (!fscrypt_info_cachep)
- goto fail_free_ctx;
+ goto fail_free_queue;

return 0;

-fail_free_ctx:
- kmem_cache_destroy(fscrypt_ctx_cachep);
fail_free_queue:
destroy_workqueue(fscrypt_read_workqueue);
fail:
@@ -520,7 +436,6 @@ static void __exit fscrypt_exit(void)

if (fscrypt_read_workqueue)
destroy_workqueue(fscrypt_read_workqueue);
- kmem_cache_destroy(fscrypt_ctx_cachep);
kmem_cache_destroy(fscrypt_info_cachep);

fscrypt_essiv_cleanup();
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 8565536feb2b..702403dc7614 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -93,9 +93,6 @@ typedef enum {
FS_ENCRYPT,
} fscrypt_direction_t;

-#define FS_CTX_REQUIRES_FREE_ENCRYPT_FL 0x00000001
-#define FS_CTX_HAS_BOUNCE_BUFFER_FL 0x00000002
-
static inline bool fscrypt_valid_enc_modes(u32 contents_mode,
u32 filenames_mode)
{
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 4d6528351f25..2e61f75feab8 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -19,7 +19,6 @@

#define FS_CRYPTO_BLOCK_SIZE 16

-struct fscrypt_ctx;
struct fscrypt_info;

struct fscrypt_str {
@@ -63,18 +62,6 @@ struct fscrypt_operations {
unsigned int max_namelen;
};

-/* Decryption work */
-struct fscrypt_ctx {
- union {
- struct {
- struct bio *bio;
- struct work_struct work;
- };
- struct list_head free_list; /* Free list */
- };
- u8 flags; /* Flags */
-};
-
static inline bool fscrypt_has_encryption_key(const struct inode *inode)
{
/* pairs with cmpxchg_release() in fscrypt_get_encryption_info() */
@@ -101,8 +88,6 @@ static inline void fscrypt_handle_d_move(struct dentry *dentry)

/* crypto.c */
extern void fscrypt_enqueue_decrypt_work(struct work_struct *);
-extern struct fscrypt_ctx *fscrypt_get_ctx(gfp_t);
-extern void fscrypt_release_ctx(struct fscrypt_ctx *);

extern struct page *fscrypt_encrypt_pagecache_blocks(struct page *page,
unsigned int len,
@@ -233,8 +218,6 @@ static inline bool fscrypt_match_name(const struct fscrypt_name *fname,

/* bio.c */
extern void fscrypt_decrypt_bio(struct bio *);
-extern void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx,
- struct bio *bio);
extern int fscrypt_zeroout_range(const struct inode *, pgoff_t, sector_t,
unsigned int);

@@ -279,16 +262,6 @@ static inline void fscrypt_enqueue_decrypt_work(struct work_struct *work)
{
}

-static inline struct fscrypt_ctx *fscrypt_get_ctx(gfp_t gfp_flags)
-{
- return ERR_PTR(-EOPNOTSUPP);
-}
-
-static inline void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
-{
- return;
-}
-
static inline struct page *fscrypt_encrypt_pagecache_blocks(struct page *page,
unsigned int len,
unsigned int offs,
@@ -430,11 +403,6 @@ static inline void fscrypt_decrypt_bio(struct bio *bio)
{
}

-static inline void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx,
- struct bio *bio)
-{
-}
-
static inline int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
sector_t pblk, unsigned int len)
{
--
2.19.1

2019-09-02 05:50:42

by Chandan Rajendra

[permalink] [raw]
Subject: Re: [PATCH V5 0/7] Consolidate FS read I/O callbacks code

On Friday, August 23, 2019 6:55 PM Chandan Rajendra wrote:
> This patchset moves the "FS read I/O callbacks" code into a file of its
> own (i.e. fs/read_callbacks.c) and modifies the generic
> do_mpage_readpge() to make use of the functionality provided.
>
> "FS read I/O callbacks" code implements the state machine that needs
> to be executed after reading data from files that are encrypted and/or
> have verity metadata associated with them.
>
> With these changes in place, the patchset changes Ext4 to use
> mpage_readpage[s] instead of its own custom ext4_readpage[s]()
> functions. This is done to reduce duplication of code across
> filesystems. Also, "FS read I/O callbacks" source files will be built
> only if CONFIG_FS_ENCRYPTION is enabled.
>
> The patchset also modifies fs/buffer.c to get file
> encryption/decryption to work with subpage-sized blocks.
>
> The patches can also be obtained from
> https://github.com/chandanr/linux.git at branch subpage-encryption-v5.

Ted and Al,

Do you have any comments on the patchset?

>
> Changelog:
> V4 -> V5:
> 1. Since F2FS uses its own workqueue and also since its
> decompression logic isn't an fs independent entity like fscrypt or
> fsverity, this patchset drops support for F2FS.
> The patchset still helps in removing a copy of
> do_mpage_readpage() from Ext4 (i.e. ext4_readpage()) and also
> prevents a copy of block_read_full_page() from being added into
> Ext4 by adding support to "read callbacks" API invocations into
> block_read_full_page().
>
> V3 -> V4:
> 1. A new buffer_head flag (i.e. BH_Read_Cb) is introduced to reliably
> check if a buffer head's content has to be decrypted.
> 2. Fix layering violation. Now the code flow for decryption happens as shown below,
> FS => read callbacks => fscrypt
> 3. Select FS_READ_CALLBACKS from FS specific kconfig file if FS_ENCRYPTION
> is enabled.
> 4. Make 'struct read_callbacks_ctx' an opaque structure.
> 5. Make use of FS' endio function rather than implementing one in read
> callbacks.
> 6. Make read_callbacks.h self-contained.
> 7. Split patchset to separate out ext4 and f2fs changes.
>
> V2 -> V3:
> 1. Split the V2 patch "Consolidate 'read callbacks' into a new file" into
> three patches,
> - Introduce the read_callbacks functionality.
> - Convert encryption to use read_callbacks.
> - Remove union from struct fscrypt_context.
> 2. fs/Kconfig
> Do not explicitly set the default value of 'n' for FS_READ_CALLBACKS.
> 3. fs/crypto/Kconfig
> Select CONFIG_FS_READ_CALLBACKS only if CONFIG_BLOCK is selected.
> 4. Remove verity associated code in read_callbacks code.
> 5. Introduce a callback argument to read_callbacks_setup() function
> which gets invoked for each page for bio. F2FS uses this to perform
> custom operations like decrementing the value of f2fs_sb_info->nr_pages[].
> 6. Encapsulate the details of "read callbacks" (e.g. Usage of "struct
> read_callbacks *ctx") within its own functions. When CONFIG_FS_READ_CALLBACKS
> is set to 'n', the corresponding stub functions return approriate error
> values.
> 7. Split fscrypt_decrypt() function into fscrypt_decrypt_bio() and
> fscrypt_decrypt_bh().
> 8. Split end_read_callbacks() function into end_read_callbacks_bio() and
> end_read_callbacks_bh().
>
> V1 -> V2:
> 1. Removed the phrase "post_read_process" from file names and
> functions. Instead we now use the phrase "read_callbacks" in its
> place.
> 2. When performing changes associated with (1), the changes made by
> the patch "Remove the term 'bio' from post read processing" are
> made in the earlier patch "Consolidate 'read callbacks' into a new
> file". Hence the patch "Remove the term 'bio' from post read
> processing" is removed from the patchset.
>
> RFC V2 -> V1:
> 1. Test and verify FS_CFLG_OWN_PAGES subset of fscrypt_encrypt_page()
> code by executing fstests on UBIFS.
> 2. Implement F2fs function call back to check if the contents of a
> page holding a verity file's data needs to be verified.
>
> RFC V1 -> RFC V2:
> 1. Describe the purpose of "Post processing code" in the cover letter.
> 2. Fix build errors when CONFIG_FS_VERITY is enabled.
>
> Chandan Rajendra (7):
> buffer_head: Introduce BH_Read_Cb flag
> FS: Introduce read callbacks
> fs/mpage.c: Integrate read callbacks
> fs/buffer.c: add decryption support via read_callbacks
> ext4: Wire up ext4_readpage[s] to use mpage_readpage[s]
> ext4: Enable encryption for subpage-sized blocks
> fscrypt: remove struct fscrypt_ctx
>
> Documentation/filesystems/fscrypt.rst | 4 +-
> fs/Kconfig | 3 +
> fs/Makefile | 1 +
> fs/buffer.c | 33 ++-
> fs/crypto/bio.c | 18 --
> fs/crypto/crypto.c | 89 +-------
> fs/crypto/fscrypt_private.h | 3 -
> fs/ext4/Kconfig | 1 +
> fs/ext4/Makefile | 2 +-
> fs/ext4/inode.c | 5 +-
> fs/ext4/readpage.c | 295 --------------------------
> fs/ext4/super.c | 7 -
> fs/f2fs/Kconfig | 1 +
> fs/mpage.c | 24 ++-
> fs/read_callbacks.c | 285 +++++++++++++++++++++++++
> include/linux/buffer_head.h | 2 +
> include/linux/fscrypt.h | 32 ---
> include/linux/read_callbacks.h | 48 +++++
> 18 files changed, 391 insertions(+), 462 deletions(-)
> delete mode 100644 fs/ext4/readpage.c
> create mode 100644 fs/read_callbacks.c
> create mode 100644 include/linux/read_callbacks.h
>
>


--
chandan