* [PATCH V3 0/7] Consolidate FS read I/O callbacks code
@ 2019-06-16 16:08 Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 1/7] FS: Introduce read callbacks Chandan Rajendra
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-16 16:08 UTC (permalink / raw)
To: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt
Cc: Chandan Rajendra, tytso, adilger.kernel, ebiggers, jaegeuk, yuchao0, hch
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-v3.
Changelog:
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):
FS: Introduce read callbacks
Integrate read callbacks into Ext4 and F2FS
fscrypt: remove struct fscrypt_ctx
fs/mpage.c: Integrate read callbacks
ext4: Wire up ext4_readpage[s] to use mpage_readpage[s]
Add decryption support for sub-pagesized blocks
ext4: Enable encryption for subpage-sized blocks
Documentation/filesystems/fscrypt.rst | 4 +-
fs/Kconfig | 3 +
fs/Makefile | 2 +
fs/buffer.c | 55 +++--
fs/crypto/Kconfig | 1 +
fs/crypto/bio.c | 44 ++--
fs/crypto/crypto.c | 90 +-------
fs/crypto/fscrypt_private.h | 3 +
fs/ext4/Makefile | 2 +-
fs/ext4/inode.c | 5 +-
fs/ext4/readpage.c | 295 --------------------------
fs/ext4/super.c | 7 -
fs/f2fs/data.c | 124 ++---------
fs/f2fs/super.c | 9 +-
fs/mpage.c | 11 +-
fs/read_callbacks.c | 233 ++++++++++++++++++++
include/linux/buffer_head.h | 1 +
include/linux/fscrypt.h | 38 ----
include/linux/read_callbacks.h | 45 ++++
19 files changed, 390 insertions(+), 582 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V3 1/7] FS: Introduce read callbacks
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
@ 2019-06-16 16:08 ` Chandan Rajendra
2019-06-21 20:03 ` Eric Biggers
2019-06-16 16:08 ` [PATCH V3 2/7] Integrate read callbacks into Ext4 and F2FS Chandan Rajendra
` (6 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-16 16:08 UTC (permalink / raw)
To: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt
Cc: Chandan Rajendra, tytso, adilger.kernel, ebiggers, jaegeuk, yuchao0, hch
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 <chandan@linux.ibm.com>
---
fs/Kconfig | 3 +
fs/Makefile | 2 +
fs/crypto/Kconfig | 1 +
fs/crypto/bio.c | 11 +++
fs/read_callbacks.c | 174 +++++++++++++++++++++++++++++++++
include/linux/fscrypt.h | 5 +
include/linux/read_callbacks.h | 38 +++++++
7 files changed, 234 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 f1046cf6ad85..d869859c88da 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -21,6 +21,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 c9aea23aba56..a1a06f0db5c1 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -21,6 +21,8 @@ else
obj-y += no-block.o
endif
+obj-$(CONFIG_FS_READ_CALLBACKS) += read_callbacks.o
+
obj-$(CONFIG_PROC_FS) += proc_namespace.o
obj-y += notify/
diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 24ed99e2eca0..7752f9964280 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -9,6 +9,7 @@ config FS_ENCRYPTION
select CRYPTO_CTS
select CRYPTO_SHA256
select KEYS
+ select FS_READ_CALLBACKS if BLOCK
help
Enable encryption of files and directories. This
feature is similar to ecryptfs, but it is more memory
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 82da2510721f..f677ff93d464 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -24,6 +24,7 @@
#include <linux/module.h>
#include <linux/bio.h>
#include <linux/namei.h>
+#include <linux/read_callbacks.h>
#include "fscrypt_private.h"
static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
@@ -68,6 +69,16 @@ void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
}
EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
+void fscrypt_decrypt_work(struct work_struct *work)
+{
+ struct read_callbacks_ctx *ctx =
+ container_of(work, struct read_callbacks_ctx, work);
+
+ fscrypt_decrypt_bio(ctx->bio);
+
+ read_callbacks(ctx);
+}
+
int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
sector_t pblk, unsigned int len)
{
diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
new file mode 100644
index 000000000000..a4196e3de05f
--- /dev/null
+++ b/fs/read_callbacks.c
@@ -0,0 +1,174 @@
+// 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/fscrypt.h>
+#include <linux/read_callbacks.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,
+};
+
+static void put_read_callbacks_ctx(struct read_callbacks_ctx *ctx)
+{
+ mempool_free(ctx, read_callbacks_ctx_pool);
+}
+
+static void end_read_callbacks_bio(struct bio *bio)
+{
+ struct read_callbacks_ctx *ctx;
+ struct page *page;
+ struct bio_vec *bv;
+ struct bvec_iter_all iter_all;
+
+ ctx = bio->bi_private;
+
+ bio_for_each_segment_all(bv, bio, iter_all) {
+ page = bv->bv_page;
+
+ if (bio->bi_status || PageError(page)) {
+ ClearPageUptodate(page);
+ SetPageError(page);
+ } else {
+ SetPageUptodate(page);
+ }
+
+ if (ctx->page_op)
+ ctx->page_op(bio, page);
+
+ unlock_page(page);
+ }
+
+ put_read_callbacks_ctx(ctx);
+
+ bio_put(bio);
+}
+
+/**
+ * read_callbacks() - Execute the read callbacks state machine.
+ * @ctx: The context structure tracking the current state.
+ *
+ * For each state, this function enqueues a work into appropriate subsystem's
+ * work queue. After performing further processing of the data in the bio's
+ * pages, the subsystem should invoke read_calbacks() to continue with the next
+ * state in the state machine.
+ */
+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, fscrypt_decrypt_work);
+ fscrypt_enqueue_decrypt_work(&ctx->work);
+ return;
+ }
+ ctx->cur_step++;
+ /* fall-through */
+ default:
+ end_read_callbacks_bio(ctx->bio);
+ }
+}
+EXPORT_SYMBOL(read_callbacks);
+
+/**
+ * read_callbacks_end_bio() - Initiate the read callbacks state machine.
+ * @bio: bio on which read I/O operation has just been completed.
+ *
+ * Initiates the execution of the read callbacks state machine when the read
+ * operation has been completed successfully. If there was an error associated
+ * with the bio, this function frees the read callbacks context structure stored
+ * in bio->bi_private (if any).
+ *
+ * Return: 1 to indicate that the bio has been handled (including unlocking the
+ * pages); 0 otherwise.
+ */
+int read_callbacks_end_bio(struct bio *bio)
+{
+ if (!bio->bi_status && bio->bi_private) {
+ read_callbacks((struct read_callbacks_ctx *)(bio->bi_private));
+ return 1;
+ }
+
+ if (bio->bi_private)
+ put_read_callbacks_ctx((struct read_callbacks_ctx *)(bio->bi_private));
+
+ return 0;
+}
+EXPORT_SYMBOL(read_callbacks_end_bio);
+
+/**
+ * read_callbacks_setup() - 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.
+ * @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(struct inode *inode, struct bio *bio,
+ end_page_op_t page_op)
+{
+ 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 -ENOMEM;
+ ctx->bio = bio;
+ ctx->inode = inode;
+ ctx->enabled_steps = enabled_steps;
+ ctx->cur_step = STEP_INITIAL;
+ ctx->page_op = page_op;
+ bio->bi_private = ctx;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(read_callbacks_setup);
+
+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/fscrypt.h b/include/linux/fscrypt.h
index bd8f207a2fb6..159b8ddcd670 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -235,6 +235,7 @@ static inline bool fscrypt_match_name(const struct fscrypt_name *fname,
extern void fscrypt_decrypt_bio(struct bio *);
extern void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx,
struct bio *bio);
+extern void fscrypt_decrypt_work(struct work_struct *work);
extern int fscrypt_zeroout_range(const struct inode *, pgoff_t, sector_t,
unsigned int);
@@ -440,6 +441,10 @@ static inline void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx,
{
}
+static inline void fscrypt_decrypt_work(struct work_struct *work)
+{
+}
+
static inline int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
sector_t pblk, unsigned int len)
{
diff --git a/include/linux/read_callbacks.h b/include/linux/read_callbacks.h
new file mode 100644
index 000000000000..aa1ec8ed7a6a
--- /dev/null
+++ b/include/linux/read_callbacks.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _READ_CALLBACKS_H
+#define _READ_CALLBACKS_H
+
+typedef void (*end_page_op_t)(struct bio *bio, struct page *page);
+
+struct read_callbacks_ctx {
+ struct bio *bio;
+ struct inode *inode;
+ struct work_struct work;
+ unsigned int cur_step;
+ unsigned int enabled_steps;
+ end_page_op_t page_op;
+};
+
+#ifdef CONFIG_FS_READ_CALLBACKS
+void read_callbacks(struct read_callbacks_ctx *ctx);
+int read_callbacks_end_bio(struct bio *bio);
+int read_callbacks_setup(struct inode *inode, struct bio *bio,
+ end_page_op_t page_op);
+#else
+static inline void read_callbacks(struct read_callbacks_ctx *ctx)
+{
+}
+
+static inline int read_callbacks_end_bio(struct bio *bio)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int read_callbacks_setup(struct inode *inode,
+ struct bio *bio, end_page_op_t page_op)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
+#endif /* _READ_CALLBACKS_H */
--
2.19.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V3 2/7] Integrate read callbacks into Ext4 and F2FS
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 1/7] FS: Introduce read callbacks Chandan Rajendra
@ 2019-06-16 16:08 ` Chandan Rajendra
2019-06-21 21:08 ` Eric Biggers
2019-06-16 16:08 ` [PATCH V3 3/7] fscrypt: remove struct fscrypt_ctx Chandan Rajendra
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-16 16:08 UTC (permalink / raw)
To: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt
Cc: Chandan Rajendra, tytso, adilger.kernel, ebiggers, jaegeuk, yuchao0, hch
This commit gets Ext4 and F2FS to make use of read callbacks API to
perform decryption of file data read from the disk.
---
fs/crypto/bio.c | 30 +--------
fs/crypto/crypto.c | 1 +
fs/crypto/fscrypt_private.h | 3 +
fs/ext4/readpage.c | 29 +++------
fs/f2fs/data.c | 124 +++++++-----------------------------
fs/f2fs/super.c | 9 +--
fs/read_callbacks.c | 1 -
include/linux/fscrypt.h | 18 ------
8 files changed, 40 insertions(+), 175 deletions(-)
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index f677ff93d464..4076d704e2e4 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -27,7 +27,7 @@
#include <linux/read_callbacks.h>
#include "fscrypt_private.h"
-static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
+static void fscrypt_decrypt_bio(struct bio *bio)
{
struct bio_vec *bv;
struct bvec_iter_all iter_all;
@@ -38,37 +38,9 @@ static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
bv->bv_offset);
if (ret)
SetPageError(page);
- else if (done)
- SetPageUptodate(page);
- if (done)
- unlock_page(page);
}
}
-void fscrypt_decrypt_bio(struct bio *bio)
-{
- __fscrypt_decrypt_bio(bio, false);
-}
-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);
-
void fscrypt_decrypt_work(struct work_struct *work)
{
struct read_callbacks_ctx *ctx =
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 45c3d0427fb2..e0aa20cfcaa7 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -54,6 +54,7 @@ struct kmem_cache *fscrypt_info_cachep;
void fscrypt_enqueue_decrypt_work(struct work_struct *work)
{
+ INIT_WORK(work, fscrypt_decrypt_work);
queue_work(fscrypt_read_workqueue, work);
}
EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 8978eec9d766..0bd65921efbf 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -113,6 +113,9 @@ static inline bool fscrypt_valid_enc_modes(u32 contents_mode,
return false;
}
+/* bio.c */
+void fscrypt_decrypt_work(struct work_struct *work);
+
/* crypto.c */
extern struct kmem_cache *fscrypt_info_cachep;
extern int fscrypt_initialize(unsigned int cop_flags);
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index c916017db334..652796b95545 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -44,6 +44,7 @@
#include <linux/backing-dev.h>
#include <linux/pagevec.h>
#include <linux/cleancache.h>
+#include <linux/read_callbacks.h>
#include "ext4.h"
@@ -73,14 +74,9 @@ static void mpage_end_io(struct bio *bio)
struct bio_vec *bv;
struct bvec_iter_all iter_all;
- if (ext4_bio_encrypted(bio)) {
- if (bio->bi_status) {
- fscrypt_release_ctx(bio->bi_private);
- } else {
- fscrypt_enqueue_decrypt_bio(bio->bi_private, bio);
- return;
- }
- }
+ if (read_callbacks_end_bio(bio))
+ return;
+
bio_for_each_segment_all(bv, bio, iter_all) {
struct page *page = bv->bv_page;
@@ -241,24 +237,19 @@ int ext4_mpage_readpages(struct address_space *mapping,
bio = NULL;
}
if (bio == NULL) {
- struct fscrypt_ctx *ctx = NULL;
-
- if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode)) {
- ctx = fscrypt_get_ctx(GFP_NOFS);
- if (IS_ERR(ctx))
- goto set_error_page;
- }
bio = bio_alloc(GFP_KERNEL,
min_t(int, nr_pages, BIO_MAX_PAGES));
- if (!bio) {
- if (ctx)
- fscrypt_release_ctx(ctx);
+ if (!bio)
+ goto set_error_page;
+
+ if (read_callbacks_setup(inode, bio, NULL)) {
+ bio_put(bio);
goto set_error_page;
}
+
bio_set_dev(bio, bdev);
bio->bi_iter.bi_sector = blocks[0] << (blkbits - 9);
bio->bi_end_io = mpage_end_io;
- bio->bi_private = ctx;
bio_set_op_attrs(bio, REQ_OP_READ,
is_readahead ? REQ_RAHEAD : 0);
}
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index a546ac8685ea..23b34399d809 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -18,6 +18,7 @@
#include <linux/uio.h>
#include <linux/cleancache.h>
#include <linux/sched/signal.h>
+#include <linux/read_callbacks.h>
#include "f2fs.h"
#include "node.h"
@@ -25,11 +26,6 @@
#include "trace.h"
#include <trace/events/f2fs.h>
-#define NUM_PREALLOC_POST_READ_CTXS 128
-
-static struct kmem_cache *bio_post_read_ctx_cache;
-static mempool_t *bio_post_read_ctx_pool;
-
static bool __is_cp_guaranteed(struct page *page)
{
struct address_space *mapping = page->mapping;
@@ -69,18 +65,19 @@ static enum count_type __read_io_type(struct page *page)
return F2FS_RD_DATA;
}
-/* postprocessing steps for read bios */
-enum bio_post_read_step {
- STEP_INITIAL = 0,
- STEP_DECRYPT,
-};
+static void f2fs_end_page_op(struct bio *bio, struct page *page)
+{
+ BUG_ON(!PageLocked(page));
-struct bio_post_read_ctx {
- struct bio *bio;
- struct work_struct work;
- unsigned int cur_step;
- unsigned int enabled_steps;
-};
+ /* PG_error was set if any post_read step failed */
+ if (bio->bi_status || PageError(page)) {
+ ClearPageUptodate(page);
+ /* will re-read again later */
+ ClearPageError(page);
+ }
+
+ dec_page_count(F2FS_P_SB(page), __read_io_type(page));
+}
static void __read_end_io(struct bio *bio)
{
@@ -91,53 +88,15 @@ static void __read_end_io(struct bio *bio)
bio_for_each_segment_all(bv, bio, iter_all) {
page = bv->bv_page;
- /* PG_error was set if any post_read step failed */
- if (bio->bi_status || PageError(page)) {
- ClearPageUptodate(page);
- /* will re-read again later */
- ClearPageError(page);
- } else {
+ if (!bio->bi_status && !PageError(page))
SetPageUptodate(page);
- }
- dec_page_count(F2FS_P_SB(page), __read_io_type(page));
- unlock_page(page);
- }
- if (bio->bi_private)
- mempool_free(bio->bi_private, bio_post_read_ctx_pool);
- bio_put(bio);
-}
-static void bio_post_read_processing(struct bio_post_read_ctx *ctx);
-
-static void decrypt_work(struct work_struct *work)
-{
- struct bio_post_read_ctx *ctx =
- container_of(work, struct bio_post_read_ctx, work);
-
- fscrypt_decrypt_bio(ctx->bio);
-
- bio_post_read_processing(ctx);
-}
+ f2fs_end_page_op(bio, page);
-static void bio_post_read_processing(struct bio_post_read_ctx *ctx)
-{
- 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:
- __read_end_io(ctx->bio);
+ unlock_page(page);
}
-}
-static bool f2fs_bio_post_read_required(struct bio *bio)
-{
- return bio->bi_private && !bio->bi_status;
+ bio_put(bio);
}
static void f2fs_read_end_io(struct bio *bio)
@@ -148,13 +107,8 @@ static void f2fs_read_end_io(struct bio *bio)
bio->bi_status = BLK_STS_IOERR;
}
- if (f2fs_bio_post_read_required(bio)) {
- struct bio_post_read_ctx *ctx = bio->bi_private;
-
- ctx->cur_step = STEP_INITIAL;
- bio_post_read_processing(ctx);
+ if (read_callbacks_end_bio(bio))
return;
- }
__read_end_io(bio);
}
@@ -557,8 +511,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
{
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct bio *bio;
- struct bio_post_read_ctx *ctx;
- unsigned int post_read_steps = 0;
+ int ret;
bio = f2fs_bio_alloc(sbi, min_t(int, nr_pages, BIO_MAX_PAGES), false);
if (!bio)
@@ -567,17 +520,10 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
bio->bi_end_io = f2fs_read_end_io;
bio_set_op_attrs(bio, REQ_OP_READ, op_flag);
- if (f2fs_encrypted_file(inode))
- post_read_steps |= 1 << STEP_DECRYPT;
- if (post_read_steps) {
- ctx = mempool_alloc(bio_post_read_ctx_pool, GFP_NOFS);
- if (!ctx) {
- bio_put(bio);
- return ERR_PTR(-ENOMEM);
- }
- ctx->bio = bio;
- ctx->enabled_steps = post_read_steps;
- bio->bi_private = ctx;
+ ret = read_callbacks_setup(inode, bio, f2fs_end_page_op);
+ if (ret) {
+ bio_put(bio);
+ return ERR_PTR(ret);
}
return bio;
@@ -2902,27 +2848,3 @@ void f2fs_clear_page_cache_dirty_tag(struct page *page)
PAGECACHE_TAG_DIRTY);
xa_unlock_irqrestore(&mapping->i_pages, flags);
}
-
-int __init f2fs_init_post_read_processing(void)
-{
- bio_post_read_ctx_cache = KMEM_CACHE(bio_post_read_ctx, 0);
- if (!bio_post_read_ctx_cache)
- goto fail;
- bio_post_read_ctx_pool =
- mempool_create_slab_pool(NUM_PREALLOC_POST_READ_CTXS,
- bio_post_read_ctx_cache);
- if (!bio_post_read_ctx_pool)
- goto fail_free_cache;
- return 0;
-
-fail_free_cache:
- kmem_cache_destroy(bio_post_read_ctx_cache);
-fail:
- return -ENOMEM;
-}
-
-void __exit f2fs_destroy_post_read_processing(void)
-{
- mempool_destroy(bio_post_read_ctx_pool);
- kmem_cache_destroy(bio_post_read_ctx_cache);
-}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6b959bbb336a..e2e6a8c7236c 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3627,15 +3627,11 @@ static int __init init_f2fs_fs(void)
err = register_filesystem(&f2fs_fs_type);
if (err)
goto free_shrinker;
+
f2fs_create_root_stats();
- err = f2fs_init_post_read_processing();
- if (err)
- goto free_root_stats;
+
return 0;
-free_root_stats:
- f2fs_destroy_root_stats();
- unregister_filesystem(&f2fs_fs_type);
free_shrinker:
unregister_shrinker(&f2fs_shrinker_info);
free_sysfs:
@@ -3656,7 +3652,6 @@ static int __init init_f2fs_fs(void)
static void __exit exit_f2fs_fs(void)
{
- f2fs_destroy_post_read_processing();
f2fs_destroy_root_stats();
unregister_filesystem(&f2fs_fs_type);
unregister_shrinker(&f2fs_shrinker_info);
diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
index a4196e3de05f..4b7fc2a349cd 100644
--- a/fs/read_callbacks.c
+++ b/fs/read_callbacks.c
@@ -76,7 +76,6 @@ void read_callbacks(struct read_callbacks_ctx *ctx)
switch (++ctx->cur_step) {
case STEP_DECRYPT:
if (ctx->enabled_steps & (1 << STEP_DECRYPT)) {
- INIT_WORK(&ctx->work, fscrypt_decrypt_work);
fscrypt_enqueue_decrypt_work(&ctx->work);
return;
}
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 159b8ddcd670..4ec0bd7cc0ba 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -232,10 +232,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 void fscrypt_decrypt_work(struct work_struct *work);
extern int fscrypt_zeroout_range(const struct inode *, pgoff_t, sector_t,
unsigned int);
@@ -431,20 +427,6 @@ static inline bool fscrypt_match_name(const struct fscrypt_name *fname,
return !memcmp(de_name, fname->disk_name.name, fname->disk_name.len);
}
-/* bio.c */
-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 void fscrypt_decrypt_work(struct work_struct *work)
-{
-}
-
static inline int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
sector_t pblk, unsigned int len)
{
--
2.19.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V3 3/7] fscrypt: remove struct fscrypt_ctx
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 1/7] FS: Introduce read callbacks Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 2/7] Integrate read callbacks into Ext4 and F2FS Chandan Rajendra
@ 2019-06-16 16:08 ` Chandan Rajendra
2019-06-21 22:00 ` Eric Biggers
2019-06-16 16:08 ` [PATCH V3 4/7] fs/mpage.c: Integrate read callbacks Chandan Rajendra
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-16 16:08 UTC (permalink / raw)
To: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt
Cc: Chandan Rajendra, tytso, adilger.kernel, ebiggers, jaegeuk, yuchao0, hch
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 and F2FS, we don't use "struct
fscrypt_ctx" anymore. Hence this commit removes the structure and the
associated code.
Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
---
fs/crypto/crypto.c | 89 +----------------------------------------
include/linux/fscrypt.h | 25 ------------
2 files changed, 2 insertions(+), 112 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index e0aa20cfcaa7..18c6b664b042 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -32,24 +32,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)
@@ -59,62 +51,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);
@@ -401,11 +337,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;
}
@@ -421,7 +352,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)
@@ -431,15 +362,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)
@@ -494,18 +416,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:
@@ -522,7 +438,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/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 4ec0bd7cc0ba..13012c589aa3 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,
@@ -281,16 +266,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,
--
2.19.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V3 4/7] fs/mpage.c: Integrate read callbacks
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
` (2 preceding siblings ...)
2019-06-16 16:08 ` [PATCH V3 3/7] fscrypt: remove struct fscrypt_ctx Chandan Rajendra
@ 2019-06-16 16:08 ` Chandan Rajendra
2019-06-21 21:14 ` Eric Biggers
2019-06-16 16:08 ` [PATCH V3 5/7] ext4: Wire up ext4_readpage[s] to use mpage_readpage[s] Chandan Rajendra
` (3 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-16 16:08 UTC (permalink / raw)
To: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt
Cc: Chandan Rajendra, tytso, adilger.kernel, ebiggers, jaegeuk, yuchao0, hch
This commit adds code to make do_mpage_readpage() to be "read callbacks"
aware i.e. for files requiring decryption, do_mpage_readpage() now
sets up the read callbacks state machine when allocating a bio and later
starts execution of the state machine after file data is read from the
underlying disk.
Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
---
fs/mpage.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/mpage.c b/fs/mpage.c
index 436a85260394..611ad122fc92 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -30,6 +30,7 @@
#include <linux/backing-dev.h>
#include <linux/pagevec.h>
#include <linux/cleancache.h>
+#include <linux/read_callbacks.h>
#include "internal.h"
/*
@@ -49,6 +50,8 @@ static void mpage_end_io(struct bio *bio)
struct bio_vec *bv;
struct bvec_iter_all iter_all;
+ if (read_callbacks_end_bio(bio))
+ return;
bio_for_each_segment_all(bv, bio, iter_all) {
struct page *page = bv->bv_page;
page_endio(page, bio_op(bio),
@@ -309,6 +312,12 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
gfp);
if (args->bio == NULL)
goto confused;
+
+ if (read_callbacks_setup(inode, args->bio, NULL)) {
+ bio_put(args->bio);
+ args->bio = NULL;
+ goto confused;
+ }
}
length = first_hole << blkbits;
@@ -330,7 +339,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
confused:
if (args->bio)
args->bio = mpage_bio_submit(REQ_OP_READ, op_flags, args->bio);
- if (!PageUptodate(page))
+ if (!PageUptodate(page) && !PageError(page))
block_read_full_page(page, args->get_block);
else
unlock_page(page);
--
2.19.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V3 5/7] ext4: Wire up ext4_readpage[s] to use mpage_readpage[s]
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
` (3 preceding siblings ...)
2019-06-16 16:08 ` [PATCH V3 4/7] fs/mpage.c: Integrate read callbacks Chandan Rajendra
@ 2019-06-16 16:08 ` Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 6/7] Add decryption support for sub-pagesized blocks Chandan Rajendra
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-16 16:08 UTC (permalink / raw)
To: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt
Cc: Chandan Rajendra, tytso, adilger.kernel, ebiggers, jaegeuk, yuchao0, hch
Now that do_mpage_readpage() is "post read process" aware, this commit
gets ext4_readpage[s] to use mpage_readpage[s] and deletes ext4's
readpage.c since the associated functionality is not required anymore.
Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
---
fs/ext4/Makefile | 2 +-
fs/ext4/inode.c | 5 +-
fs/ext4/readpage.c | 286 ---------------------------------------------
3 files changed, 3 insertions(+), 290 deletions(-)
delete mode 100644 fs/ext4/readpage.c
diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
index 8fdfcd3c3e04..7c38803a808d 100644
--- a/fs/ext4/Makefile
+++ b/fs/ext4/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_EXT4_FS) += ext4.o
ext4-y := balloc.o bitmap.o block_validity.o dir.o ext4_jbd2.o extents.o \
extents_status.o file.o fsmap.o fsync.o hash.o ialloc.o \
indirect.o inline.o inode.o ioctl.o mballoc.o migrate.o \
- mmp.o move_extent.o namei.o page-io.o readpage.o resize.o \
+ mmp.o move_extent.o namei.o page-io.o resize.o \
super.o symlink.o sysfs.o xattr.o xattr_trusted.o xattr_user.o
ext4-$(CONFIG_EXT4_FS_POSIX_ACL) += acl.o
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index f65357735a1a..60010b76b566 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3363,8 +3363,7 @@ static int ext4_readpage(struct file *file, struct page *page)
ret = ext4_readpage_inline(inode, page);
if (ret == -EAGAIN)
- return ext4_mpage_readpages(page->mapping, NULL, page, 1,
- false);
+ return mpage_readpage(page, ext4_get_block);
return ret;
}
@@ -3379,7 +3378,7 @@ ext4_readpages(struct file *file, struct address_space *mapping,
if (ext4_has_inline_data(inode))
return 0;
- return ext4_mpage_readpages(mapping, pages, NULL, nr_pages, true);
+ return mpage_readpages(mapping, pages, nr_pages, ext4_get_block);
}
static void ext4_invalidatepage(struct page *page, unsigned int offset,
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
deleted file mode 100644
index 652796b95545..000000000000
--- a/fs/ext4/readpage.c
+++ /dev/null
@@ -1,286 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * linux/fs/ext4/readpage.c
- *
- * Copyright (C) 2002, Linus Torvalds.
- * Copyright (C) 2015, Google, Inc.
- *
- * This was originally taken from fs/mpage.c
- *
- * The intent is the ext4_mpage_readpages() function here is intended
- * to replace mpage_readpages() in the general case, not just for
- * encrypted files. It has some limitations (see below), where it
- * will fall back to read_block_full_page(), but these limitations
- * should only be hit when page_size != block_size.
- *
- * This will allow us to attach a callback function to support ext4
- * encryption.
- *
- * If anything unusual happens, such as:
- *
- * - encountering a page which has buffers
- * - encountering a page which has a non-hole after a hole
- * - encountering a page with non-contiguous blocks
- *
- * then this code just gives up and calls the buffer_head-based read function.
- * It does handle a page which has holes at the end - that is a common case:
- * the end-of-file on blocksize < PAGE_SIZE setups.
- *
- */
-
-#include <linux/kernel.h>
-#include <linux/export.h>
-#include <linux/mm.h>
-#include <linux/kdev_t.h>
-#include <linux/gfp.h>
-#include <linux/bio.h>
-#include <linux/fs.h>
-#include <linux/buffer_head.h>
-#include <linux/blkdev.h>
-#include <linux/highmem.h>
-#include <linux/prefetch.h>
-#include <linux/mpage.h>
-#include <linux/writeback.h>
-#include <linux/backing-dev.h>
-#include <linux/pagevec.h>
-#include <linux/cleancache.h>
-#include <linux/read_callbacks.h>
-
-#include "ext4.h"
-
-static inline bool ext4_bio_encrypted(struct bio *bio)
-{
-#ifdef CONFIG_FS_ENCRYPTION
- return unlikely(bio->bi_private != NULL);
-#else
- return false;
-#endif
-}
-
-/*
- * I/O completion handler for multipage BIOs.
- *
- * The mpage code never puts partial pages into a BIO (except for end-of-file).
- * If a page does not map to a contiguous run of blocks then it simply falls
- * back to block_read_full_page().
- *
- * Why is this? If a page's completion depends on a number of different BIOs
- * which can complete in any order (or at the same time) then determining the
- * status of that page is hard. See end_buffer_async_read() for the details.
- * There is no point in duplicating all that complexity.
- */
-static void mpage_end_io(struct bio *bio)
-{
- struct bio_vec *bv;
- struct bvec_iter_all iter_all;
-
- if (read_callbacks_end_bio(bio))
- return;
-
- bio_for_each_segment_all(bv, bio, iter_all) {
- struct page *page = bv->bv_page;
-
- if (!bio->bi_status) {
- SetPageUptodate(page);
- } else {
- ClearPageUptodate(page);
- SetPageError(page);
- }
- unlock_page(page);
- }
-
- bio_put(bio);
-}
-
-int ext4_mpage_readpages(struct address_space *mapping,
- struct list_head *pages, struct page *page,
- unsigned nr_pages, bool is_readahead)
-{
- struct bio *bio = NULL;
- sector_t last_block_in_bio = 0;
-
- struct inode *inode = mapping->host;
- const unsigned blkbits = inode->i_blkbits;
- const unsigned blocks_per_page = PAGE_SIZE >> blkbits;
- const unsigned blocksize = 1 << blkbits;
- sector_t block_in_file;
- sector_t last_block;
- sector_t last_block_in_file;
- sector_t blocks[MAX_BUF_PER_PAGE];
- unsigned page_block;
- struct block_device *bdev = inode->i_sb->s_bdev;
- int length;
- unsigned relative_block = 0;
- struct ext4_map_blocks map;
-
- map.m_pblk = 0;
- map.m_lblk = 0;
- map.m_len = 0;
- map.m_flags = 0;
-
- for (; nr_pages; nr_pages--) {
- int fully_mapped = 1;
- unsigned first_hole = blocks_per_page;
-
- if (pages) {
- page = lru_to_page(pages);
-
- prefetchw(&page->flags);
- list_del(&page->lru);
- if (add_to_page_cache_lru(page, mapping, page->index,
- readahead_gfp_mask(mapping)))
- goto next_page;
- }
-
- if (page_has_buffers(page))
- goto confused;
-
- block_in_file = (sector_t)page->index << (PAGE_SHIFT - blkbits);
- last_block = block_in_file + nr_pages * blocks_per_page;
- last_block_in_file = (i_size_read(inode) + blocksize - 1) >> blkbits;
- if (last_block > last_block_in_file)
- last_block = last_block_in_file;
- page_block = 0;
-
- /*
- * Map blocks using the previous result first.
- */
- if ((map.m_flags & EXT4_MAP_MAPPED) &&
- block_in_file > map.m_lblk &&
- block_in_file < (map.m_lblk + map.m_len)) {
- unsigned map_offset = block_in_file - map.m_lblk;
- unsigned last = map.m_len - map_offset;
-
- for (relative_block = 0; ; relative_block++) {
- if (relative_block == last) {
- /* needed? */
- map.m_flags &= ~EXT4_MAP_MAPPED;
- break;
- }
- if (page_block == blocks_per_page)
- break;
- blocks[page_block] = map.m_pblk + map_offset +
- relative_block;
- page_block++;
- block_in_file++;
- }
- }
-
- /*
- * Then do more ext4_map_blocks() calls until we are
- * done with this page.
- */
- while (page_block < blocks_per_page) {
- if (block_in_file < last_block) {
- map.m_lblk = block_in_file;
- map.m_len = last_block - block_in_file;
-
- if (ext4_map_blocks(NULL, inode, &map, 0) < 0) {
- set_error_page:
- SetPageError(page);
- zero_user_segment(page, 0,
- PAGE_SIZE);
- unlock_page(page);
- goto next_page;
- }
- }
- if ((map.m_flags & EXT4_MAP_MAPPED) == 0) {
- fully_mapped = 0;
- if (first_hole == blocks_per_page)
- first_hole = page_block;
- page_block++;
- block_in_file++;
- continue;
- }
- if (first_hole != blocks_per_page)
- goto confused; /* hole -> non-hole */
-
- /* Contiguous blocks? */
- if (page_block && blocks[page_block-1] != map.m_pblk-1)
- goto confused;
- for (relative_block = 0; ; relative_block++) {
- if (relative_block == map.m_len) {
- /* needed? */
- map.m_flags &= ~EXT4_MAP_MAPPED;
- break;
- } else if (page_block == blocks_per_page)
- break;
- blocks[page_block] = map.m_pblk+relative_block;
- page_block++;
- block_in_file++;
- }
- }
- if (first_hole != blocks_per_page) {
- zero_user_segment(page, first_hole << blkbits,
- PAGE_SIZE);
- if (first_hole == 0) {
- SetPageUptodate(page);
- unlock_page(page);
- goto next_page;
- }
- } else if (fully_mapped) {
- SetPageMappedToDisk(page);
- }
- if (fully_mapped && blocks_per_page == 1 &&
- !PageUptodate(page) && cleancache_get_page(page) == 0) {
- SetPageUptodate(page);
- goto confused;
- }
-
- /*
- * This page will go to BIO. Do we need to send this
- * BIO off first?
- */
- if (bio && (last_block_in_bio != blocks[0] - 1)) {
- submit_and_realloc:
- submit_bio(bio);
- bio = NULL;
- }
- if (bio == NULL) {
- bio = bio_alloc(GFP_KERNEL,
- min_t(int, nr_pages, BIO_MAX_PAGES));
- if (!bio)
- goto set_error_page;
-
- if (read_callbacks_setup(inode, bio, NULL)) {
- bio_put(bio);
- goto set_error_page;
- }
-
- bio_set_dev(bio, bdev);
- bio->bi_iter.bi_sector = blocks[0] << (blkbits - 9);
- bio->bi_end_io = mpage_end_io;
- bio_set_op_attrs(bio, REQ_OP_READ,
- is_readahead ? REQ_RAHEAD : 0);
- }
-
- length = first_hole << blkbits;
- if (bio_add_page(bio, page, length, 0) < length)
- goto submit_and_realloc;
-
- if (((map.m_flags & EXT4_MAP_BOUNDARY) &&
- (relative_block == map.m_len)) ||
- (first_hole != blocks_per_page)) {
- submit_bio(bio);
- bio = NULL;
- } else
- last_block_in_bio = blocks[blocks_per_page - 1];
- goto next_page;
- confused:
- if (bio) {
- submit_bio(bio);
- bio = NULL;
- }
- if (!PageUptodate(page))
- block_read_full_page(page, ext4_get_block);
- else
- unlock_page(page);
- next_page:
- if (pages)
- put_page(page);
- }
- BUG_ON(pages && !list_empty(pages));
- if (bio)
- submit_bio(bio);
- return 0;
-}
--
2.19.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V3 6/7] Add decryption support for sub-pagesized blocks
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
` (4 preceding siblings ...)
2019-06-16 16:08 ` [PATCH V3 5/7] ext4: Wire up ext4_readpage[s] to use mpage_readpage[s] Chandan Rajendra
@ 2019-06-16 16:08 ` Chandan Rajendra
2019-06-21 21:29 ` Eric Biggers
2019-06-16 16:08 ` [PATCH V3 7/7] ext4: Enable encryption for subpage-sized blocks Chandan Rajendra
2019-06-21 22:15 ` [PATCH V3 0/7] Consolidate FS read I/O callbacks code Eric Biggers
7 siblings, 1 reply; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-16 16:08 UTC (permalink / raw)
To: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt
Cc: Chandan Rajendra, tytso, adilger.kernel, ebiggers, jaegeuk, yuchao0, hch
To support decryption of sub-pagesized blocks this commit adds code to,
1. Track buffer head in "struct read_callbacks_ctx".
2. Pass buffer head argument to all read callbacks.
3. Add new fscrypt helper to decrypt the file data referred to by a
buffer head.
Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
---
fs/buffer.c | 55 +++++++++------
fs/crypto/bio.c | 21 +++++-
fs/f2fs/data.c | 2 +-
fs/mpage.c | 2 +-
fs/read_callbacks.c | 118 +++++++++++++++++++++++++--------
include/linux/buffer_head.h | 1 +
include/linux/read_callbacks.h | 13 +++-
7 files changed, 158 insertions(+), 54 deletions(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index e450c55f6434..dcb67525dac9 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -46,6 +46,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);
@@ -246,11 +247,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)
{
unsigned long flags;
struct buffer_head *first;
@@ -258,17 +255,7 @@ 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);
- } else {
- clear_buffer_uptodate(bh);
- buffer_io_error(bh, ", async page read");
- SetPageError(page);
- }
-
/*
* Be _very_ careful from here on. Bad things can happen if
* two buffer heads end IO at almost the same time and both
@@ -307,6 +294,31 @@ 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)
+{
+ struct page *page;
+
+ BUG_ON(!buffer_async_read(bh));
+
+ if (read_callbacks_end_bh(bh, uptodate))
+ return;
+
+ page = bh->b_page;
+ if (uptodate) {
+ set_buffer_uptodate(bh);
+ } else {
+ clear_buffer_uptodate(bh);
+ buffer_io_error(bh, ", async page read");
+ SetPageError(page);
+ }
+
+ end_buffer_async_read(bh);
+}
+
/*
* Completion handler for block_write_full_page() - pages which are unlocked
* during I/O, and which have PageWriteback cleared upon I/O completion.
@@ -379,7 +391,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);
}
@@ -2294,10 +2306,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(inode, NULL, bh, NULL))) {
+ __end_buffer_async_read(bh, 0);
+ continue;
+ }
submit_bh(REQ_OP_READ, 0, bh);
+ }
}
return 0;
}
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index 4076d704e2e4..b836d648fd27 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -24,6 +24,7 @@
#include <linux/module.h>
#include <linux/bio.h>
#include <linux/namei.h>
+#include <linux/buffer_head.h>
#include <linux/read_callbacks.h>
#include "fscrypt_private.h"
@@ -41,12 +42,30 @@ static void fscrypt_decrypt_bio(struct bio *bio)
}
}
+static void fscrypt_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);
+}
+
void fscrypt_decrypt_work(struct work_struct *work)
{
struct read_callbacks_ctx *ctx =
container_of(work, struct read_callbacks_ctx, work);
- fscrypt_decrypt_bio(ctx->bio);
+ WARN_ON(!ctx->bh && !ctx->bio);
+
+ if (ctx->bio)
+ fscrypt_decrypt_bio(ctx->bio);
+ else
+ fscrypt_decrypt_bh(ctx->bh);
read_callbacks(ctx);
}
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 23b34399d809..1e8b1eb68a90 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -520,7 +520,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
bio->bi_end_io = f2fs_read_end_io;
bio_set_op_attrs(bio, REQ_OP_READ, op_flag);
- ret = read_callbacks_setup(inode, bio, f2fs_end_page_op);
+ ret = read_callbacks_setup(inode, bio, NULL, f2fs_end_page_op);
if (ret) {
bio_put(bio);
return ERR_PTR(ret);
diff --git a/fs/mpage.c b/fs/mpage.c
index 611ad122fc92..387c23b529eb 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -313,7 +313,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
if (args->bio == NULL)
goto confused;
- if (read_callbacks_setup(inode, args->bio, NULL)) {
+ if (read_callbacks_setup(inode, args->bio, NULL, NULL)) {
bio_put(args->bio);
args->bio = NULL;
goto confused;
diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
index 4b7fc2a349cd..7b3ab11c1652 100644
--- a/fs/read_callbacks.c
+++ b/fs/read_callbacks.c
@@ -8,6 +8,7 @@
#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>
@@ -57,35 +58,27 @@ static void end_read_callbacks_bio(struct bio *bio)
bio_put(bio);
}
-/**
- * read_callbacks() - Execute the read callbacks state machine.
- * @ctx: The context structure tracking the current state.
- *
- * For each state, this function enqueues a work into appropriate subsystem's
- * work queue. After performing further processing of the data in the bio's
- * pages, the subsystem should invoke read_calbacks() to continue with the next
- * state in the state machine.
- */
-void read_callbacks(struct read_callbacks_ctx *ctx)
+static void end_read_callbacks_bh(struct buffer_head *bh)
{
- /*
- * 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)) {
- fscrypt_enqueue_decrypt_work(&ctx->work);
- return;
- }
- ctx->cur_step++;
- /* fall-through */
- default:
- end_read_callbacks_bio(ctx->bio);
- }
+ struct read_callbacks_ctx *ctx;
+
+ if (!PageError(bh->b_page))
+ set_buffer_uptodate(bh);
+
+ ctx = bh->b_private;
+
+ end_buffer_async_read(bh);
+
+ put_read_callbacks_ctx(ctx);
+}
+
+static void end_read_callbacks(struct bio *bio, struct buffer_head *bh)
+{
+ if (bio)
+ end_read_callbacks_bio(bio);
+ else
+ end_read_callbacks_bh(bh);
}
-EXPORT_SYMBOL(read_callbacks);
/**
* read_callbacks_end_bio() - Initiate the read callbacks state machine.
@@ -113,10 +106,69 @@ int read_callbacks_end_bio(struct bio *bio)
}
EXPORT_SYMBOL(read_callbacks_end_bio);
+/**
+ * read_callbacks_end_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.
+ *
+ * Initiates the execution of the read callbacks state machine when the read
+ * operation has been completed successfully. 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).
+ *
+ * Return: 1 to indicate that the buffer head has been handled (including
+ * unlocking the buffer head and the corresponding page if applicable); 0
+ * otherwise.
+ */
+int read_callbacks_end_bh(struct buffer_head *bh, int uptodate)
+{
+ if (uptodate && bh->b_private) {
+ read_callbacks((struct read_callbacks_ctx *)(bh->b_private));
+ return 1;
+ }
+
+ if (bh->b_private)
+ put_read_callbacks_ctx((struct read_callbacks_ctx *)(bh->b_private));
+
+ return 0;
+}
+EXPORT_SYMBOL(read_callbacks_end_bh);
+
+/**
+ * read_callbacks() - Execute the read callbacks state machine.
+ * @ctx: The context structure tracking the current state.
+ *
+ * For each state, this function enqueues a work into appropriate subsystem's
+ * work queue. After performing further processing of the data in the bio's
+ * pages, the subsystem should invoke read_calbacks() to continue with the next
+ * state in the state machine.
+ */
+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)) {
+ fscrypt_enqueue_decrypt_work(&ctx->work);
+ return;
+ }
+ ctx->cur_step++;
+ /* fall-through */
+ default:
+ end_read_callbacks(ctx->bio, ctx->bh);
+ }
+}
+EXPORT_SYMBOL(read_callbacks);
+
/**
* read_callbacks_setup() - 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
@@ -128,11 +180,14 @@ EXPORT_SYMBOL(read_callbacks_end_bio);
* Return: 0 on success; An error code on failure.
*/
int read_callbacks_setup(struct inode *inode, struct bio *bio,
- end_page_op_t page_op)
+ struct buffer_head *bh, end_page_op_t page_op)
{
struct read_callbacks_ctx *ctx = NULL;
unsigned int enabled_steps = 0;
+ if (!bh && !bio)
+ return -EINVAL;
+
if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
enabled_steps |= 1 << STEP_DECRYPT;
@@ -140,12 +195,17 @@ int read_callbacks_setup(struct inode *inode, struct bio *bio,
ctx = mempool_alloc(read_callbacks_ctx_pool, GFP_NOFS);
if (!ctx)
return -ENOMEM;
+
+ ctx->bh = bh;
ctx->bio = bio;
ctx->inode = inode;
ctx->enabled_steps = enabled_steps;
ctx->cur_step = STEP_INITIAL;
ctx->page_op = page_op;
- bio->bi_private = ctx;
+ if (bio)
+ bio->bi_private = ctx;
+ else
+ bh->b_private = ctx;
}
return 0;
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 7b73ef7f902d..42d0d63c7a3b 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -165,6 +165,7 @@ void create_empty_buffers(struct page *, unsigned long,
void end_buffer_read_sync(struct buffer_head *bh, int uptodate);
void end_buffer_write_sync(struct buffer_head *bh, int uptodate);
void end_buffer_async_write(struct buffer_head *bh, int uptodate);
+void end_buffer_async_read(struct buffer_head *bh);
/* Things to do with buffers at mapping->private_list */
void mark_buffer_dirty_inode(struct buffer_head *bh, struct inode *inode);
diff --git a/include/linux/read_callbacks.h b/include/linux/read_callbacks.h
index aa1ec8ed7a6a..0b52d7961fb2 100644
--- a/include/linux/read_callbacks.h
+++ b/include/linux/read_callbacks.h
@@ -5,6 +5,7 @@
typedef void (*end_page_op_t)(struct bio *bio, struct page *page);
struct read_callbacks_ctx {
+ struct buffer_head *bh;
struct bio *bio;
struct inode *inode;
struct work_struct work;
@@ -16,8 +17,9 @@ struct read_callbacks_ctx {
#ifdef CONFIG_FS_READ_CALLBACKS
void read_callbacks(struct read_callbacks_ctx *ctx);
int read_callbacks_end_bio(struct bio *bio);
+int read_callbacks_end_bh(struct buffer_head *bh, int uptodate);
int read_callbacks_setup(struct inode *inode, struct bio *bio,
- end_page_op_t page_op);
+ struct buffer_head *bh, end_page_op_t page_op);
#else
static inline void read_callbacks(struct read_callbacks_ctx *ctx)
{
@@ -28,8 +30,13 @@ static inline int read_callbacks_end_bio(struct bio *bio)
return -EOPNOTSUPP;
}
-static inline int read_callbacks_setup(struct inode *inode,
- struct bio *bio, end_page_op_t page_op)
+static inline int read_callbacks_end_bh(struct buffer_head *bh, int uptodate)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int read_callbacks_setup(struct inode *inode, struct bio *bio,
+ struct buffer_head *bh, end_page_op_t page_op)
{
return -EOPNOTSUPP;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH V3 7/7] ext4: Enable encryption for subpage-sized blocks
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
` (5 preceding siblings ...)
2019-06-16 16:08 ` [PATCH V3 6/7] Add decryption support for sub-pagesized blocks Chandan Rajendra
@ 2019-06-16 16:08 ` Chandan Rajendra
2019-06-21 22:15 ` [PATCH V3 0/7] Consolidate FS read I/O callbacks code Eric Biggers
7 siblings, 0 replies; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-16 16:08 UTC (permalink / raw)
To: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt
Cc: Chandan Rajendra, tytso, adilger.kernel, ebiggers, jaegeuk, yuchao0, hch
Now that we have the code to support encryption for subpage-sized
blocks, this commit removes the conditional check in filesystem mount
code.
The commit also changes the support statement in
Documentation/filesystems/fscrypt.rst to reflect the fact that
encryption of filesystems with blocksize less than page size now works.
Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
---
Documentation/filesystems/fscrypt.rst | 4 ++--
fs/ext4/super.c | 7 -------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 08c23b60e016..c3efe86bf2b2 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -213,8 +213,8 @@ Contents encryption
-------------------
For file contents, each filesystem block is encrypted independently.
-Currently, only the case where the filesystem block size is equal to
-the system's page size (usually 4096 bytes) is supported.
+Starting from Linux kernel 5.4, encryption of filesystems with block
+size less than system's page size is supported.
Each block's IV is set to the logical block number within the file as
a little endian number, except that:
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4079605d437a..63661a86d148 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4412,13 +4412,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
}
}
- if ((DUMMY_ENCRYPTION_ENABLED(sbi) || ext4_has_feature_encrypt(sb)) &&
- (blocksize != PAGE_SIZE)) {
- ext4_msg(sb, KERN_ERR,
- "Unsupported blocksize for fs encryption");
- goto failed_mount_wq;
- }
-
if (DUMMY_ENCRYPTION_ENABLED(sbi) && !sb_rdonly(sb) &&
!ext4_has_feature_encrypt(sb)) {
ext4_set_feature_encrypt(sb);
--
2.19.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH V3 1/7] FS: Introduce read callbacks
2019-06-16 16:08 ` [PATCH V3 1/7] FS: Introduce read callbacks Chandan Rajendra
@ 2019-06-21 20:03 ` Eric Biggers
2019-06-25 4:59 ` Chandan Rajendra
0 siblings, 1 reply; 19+ messages in thread
From: Eric Biggers @ 2019-06-21 20:03 UTC (permalink / raw)
To: Chandan Rajendra
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
Hi Chandan,
On Sun, Jun 16, 2019 at 09:38:07PM +0530, Chandan Rajendra wrote:
> 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 <chandan@linux.ibm.com>
> ---
> fs/Kconfig | 3 +
> fs/Makefile | 2 +
> fs/crypto/Kconfig | 1 +
> fs/crypto/bio.c | 11 +++
> fs/read_callbacks.c | 174 +++++++++++++++++++++++++++++++++
> include/linux/fscrypt.h | 5 +
> include/linux/read_callbacks.h | 38 +++++++
> 7 files changed, 234 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 f1046cf6ad85..d869859c88da 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -21,6 +21,9 @@ if BLOCK
> config FS_IOMAP
> bool
>
> +config FS_READ_CALLBACKS
> + bool
This should be intended with a tab, not spaces.
> +
> source "fs/ext2/Kconfig"
> source "fs/ext4/Kconfig"
> source "fs/jbd2/Kconfig"
> diff --git a/fs/Makefile b/fs/Makefile
> index c9aea23aba56..a1a06f0db5c1 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -21,6 +21,8 @@ else
> obj-y += no-block.o
> endif
>
> +obj-$(CONFIG_FS_READ_CALLBACKS) += read_callbacks.o
> +
> obj-$(CONFIG_PROC_FS) += proc_namespace.o
Nit: maybe move this to just below the line for iomap.o, to be consistent with
where FS_READ_CALLBACKS is in the Kconfig file.
>
> obj-y += notify/
> diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> index 24ed99e2eca0..7752f9964280 100644
> --- a/fs/crypto/Kconfig
> +++ b/fs/crypto/Kconfig
> @@ -9,6 +9,7 @@ config FS_ENCRYPTION
> select CRYPTO_CTS
> select CRYPTO_SHA256
> select KEYS
> + select FS_READ_CALLBACKS if BLOCK
> help
> Enable encryption of files and directories. This
> feature is similar to ecryptfs, but it is more memory
> diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> index 82da2510721f..f677ff93d464 100644
> --- a/fs/crypto/bio.c
> +++ b/fs/crypto/bio.c
> @@ -24,6 +24,7 @@
> #include <linux/module.h>
> #include <linux/bio.h>
> #include <linux/namei.h>
> +#include <linux/read_callbacks.h>
> #include "fscrypt_private.h"
>
> static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> @@ -68,6 +69,16 @@ void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
> }
> EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
>
> +void fscrypt_decrypt_work(struct work_struct *work)
> +{
> + struct read_callbacks_ctx *ctx =
> + container_of(work, struct read_callbacks_ctx, work);
> +
> + fscrypt_decrypt_bio(ctx->bio);
> +
> + read_callbacks(ctx);
> +}
> +
This seems like a layering violation, since it means that read_callbacks.c calls
fs/crypto/ *and* fs/crypto/ calls read_callbacks.c. I don't think fs/crypto/
should be aware of read_callbacks at all. Instead we should have a clear
ordering between the components: the filesystem uses read_callbacks.c and
fs/crypto/, and read_callbacks.c uses fs/crypto/. So how about:
- Move fscrypt_decrypt_work(), fscrypt_decrypt_bh(), and fscrypt_decrypt_bio()
into fs/read_callbacks.c and remove the "fscrypt_" prefix from them.
- Instead of selecting FS_READ_CALLBACKS from FS_ENCRYPTION, select it from
EXT4_FS and F2FS_FS (if FS_ENCRYPTION). I.e., it's really the *filesystems*
themselves that use read_callbacks, not fs/crypto/.
- Move the definition of read_callbacks_ctx into fs/read_callbacks.c, and make
read_callbacks() static, so these are private to the read_callbacks component.
> int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
> sector_t pblk, unsigned int len)
> {
> diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
> new file mode 100644
> index 000000000000..a4196e3de05f
> --- /dev/null
> +++ b/fs/read_callbacks.c
> @@ -0,0 +1,174 @@
> +// 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/fscrypt.h>
> +#include <linux/read_callbacks.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,
> +};
> +
> +static void put_read_callbacks_ctx(struct read_callbacks_ctx *ctx)
> +{
> + mempool_free(ctx, read_callbacks_ctx_pool);
> +}
Maybe call this free_read_callbacks_ctx(), so that it doesn't sound like it's
doing reference counting.
> +
> +static void end_read_callbacks_bio(struct bio *bio)
> +{
> + struct read_callbacks_ctx *ctx;
> + struct page *page;
> + struct bio_vec *bv;
> + struct bvec_iter_all iter_all;
> +
> + ctx = bio->bi_private;
> +
> + bio_for_each_segment_all(bv, bio, iter_all) {
> + page = bv->bv_page;
> +
> + if (bio->bi_status || PageError(page)) {
> + ClearPageUptodate(page);
> + SetPageError(page);
> + } else {
> + SetPageUptodate(page);
> + }
> +
> + if (ctx->page_op)
> + ctx->page_op(bio, page);
> +
> + unlock_page(page);
> + }
> +
> + put_read_callbacks_ctx(ctx);
> +
> + bio_put(bio);
> +}
The filesystem itself (or fs/mpage.c) actually has to implement almost all this
logic as well anyway, because CONFIG_FS_READ_CALLBACKS may be unset. And the
->page_op() callback, which exists only because f2fs needs to update some
counter, is very ugly.
IMO, it would be simpler to just make this whole function filesystem-specific,
as a 'typedef void (*end_bio_func_t)(struct bio *bio);' which gets passed to the
read_callbacks endio hook.
Of course, each end_bio_func_t would have to check PageError() to check whether
any read_callbacks step failed. But to make that a bit easier and to make it
get compiled out when CONFIG_FS_READ_CALLBACKS is unset, there could be a helper
function in read_callbacks.h:
#ifdef CONFIG_FS_READ_CALLBACKS
static inline bool read_callbacks_failed(struct page *page)
{
return PageError(page);
}
#else
static inline bool read_callbacks_failed(struct page *page)
{
return false;
}
#endif
> +
> +/**
> + * read_callbacks() - Execute the read callbacks state machine.
> + * @ctx: The context structure tracking the current state.
> + *
> + * For each state, this function enqueues a work into appropriate subsystem's
> + * work queue. After performing further processing of the data in the bio's
> + * pages, the subsystem should invoke read_calbacks() to continue with the next
> + * state in the state machine.
> + */
> +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, fscrypt_decrypt_work);
> + fscrypt_enqueue_decrypt_work(&ctx->work);
> + return;
> + }
> + ctx->cur_step++;
> + /* fall-through */
> + default:
> + end_read_callbacks_bio(ctx->bio);
> + }
> +}
> +EXPORT_SYMBOL(read_callbacks);
As I mentioned, I think the work functions should be defined in this file rather
than in fs/crypto/ or fs/verity/, since they're specific to the read_callbacks.
fs/crypto/ and fs/verity/ should not be aware of read_callbacks at all.
Moreover, the 'read_callbacks()' function should be static.
> +
> +/**
> + * read_callbacks_end_bio() - Initiate the read callbacks state machine.
> + * @bio: bio on which read I/O operation has just been completed.
> + *
> + * Initiates the execution of the read callbacks state machine when the read
> + * operation has been completed successfully. If there was an error associated
> + * with the bio, this function frees the read callbacks context structure stored
> + * in bio->bi_private (if any).
> + *
> + * Return: 1 to indicate that the bio has been handled (including unlocking the
> + * pages); 0 otherwise.
> + */
> +int read_callbacks_end_bio(struct bio *bio)
> +{
> + if (!bio->bi_status && bio->bi_private) {
> + read_callbacks((struct read_callbacks_ctx *)(bio->bi_private));
> + return 1;
> + }
> +
> + if (bio->bi_private)
> + put_read_callbacks_ctx((struct read_callbacks_ctx *)(bio->bi_private));
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(read_callbacks_end_bio);
Okay, several issues here...
First, the naming of this is really confusing, since it's actually *beginning*
the read callbacks, not ending them; and it's basically the same name as
end_read_callbacks_bio(), which actually *is* for ending the read callbacks.
Since this is the endio hook, how about calling it read_callbacks_endio_bio(),
and likewise read_callbacks_endio_bh()?
But more importantly, this doesn't need to have a return value, since the
read_callbacks layer has to know how to end the bio (meaning unlock the pages
and mark them uptodate or not) *anyway*, or at least know how to ask the
filesystem to do it. So it should just do it if needed, rather than returning 0
and making the caller do it.
Also just assign 'ctx = bio->bi_private' at the start of the function; no need
to access the field 4 times and have unnecessary casts.
So IMO, the below would be much better:
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);
And then the !CONFIG_FS_READ_CALLBACKS stub:
static inline void read_callbacks_endio_bio(struct bio *bio,
end_bio_func_t end_bio)
{
end_bio(bio);
}
Similarly for the buffer_head versions.
> +
> +/**
> + * read_callbacks_setup() - 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.
> + * @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(struct inode *inode, struct bio *bio,
> + end_page_op_t page_op)
> +{
> + 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 -ENOMEM;
> + ctx->bio = bio;
> + ctx->inode = inode;
> + ctx->enabled_steps = enabled_steps;
> + ctx->cur_step = STEP_INITIAL;
> + ctx->page_op = page_op;
> + bio->bi_private = ctx;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(read_callbacks_setup);
Please call it:
read_callbacks_setup_bio()
Then when you add buffer_head support later in the patchset, rather than adding
a buffer_head argument to this function, add a new function:
read_callbacks_setup_bh()
So that all the users don't have to pass both the buffer_head and bio arguments.
These can use a common function get_read_callbacks_ctx() that creates a
read_callbacks_ctx for the inode. E.g., the bio version could look like:
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);
Also, a nit: can you move the read_callbacks_setup_*() functions to near the top
of the file, since they're called first (before the endio functions)? Likewise
in read_callbacks.h. It would nice to keep the functions in a logical order.
> diff --git a/include/linux/read_callbacks.h b/include/linux/read_callbacks.h
> new file mode 100644
> index 000000000000..aa1ec8ed7a6a
> --- /dev/null
> +++ b/include/linux/read_callbacks.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _READ_CALLBACKS_H
> +#define _READ_CALLBACKS_H
Headers should be self-contained, but this is missing some prerequisite headers
and forward declarations. Try compiling a .c file with this header included
first.
> +
> +typedef void (*end_page_op_t)(struct bio *bio, struct page *page);
> +
> +struct read_callbacks_ctx {
> + struct bio *bio;
> + struct inode *inode;
> + struct work_struct work;
> + unsigned int cur_step;
> + unsigned int enabled_steps;
> + end_page_op_t page_op;
> +};
As I mentioned, I don't think read_callbacks_ctx should be exposed to
filesystems like this. Instead just forward declare it here, and put the actual
definition in fs/read_callbacks.c.
> +
> +#ifdef CONFIG_FS_READ_CALLBACKS
> +void read_callbacks(struct read_callbacks_ctx *ctx);
> +int read_callbacks_end_bio(struct bio *bio);
> +int read_callbacks_setup(struct inode *inode, struct bio *bio,
> + end_page_op_t page_op);
> +#else
> +static inline void read_callbacks(struct read_callbacks_ctx *ctx)
> +{
> +}
> +
> +static inline int read_callbacks_end_bio(struct bio *bio)
> +{
> + return -EOPNOTSUPP;
> +}
This stub needs to return 0, otherwise it breaks fs/mpage.c and f2fs for
everyone when CONFIG_FS_READ_CALLBACKS is unset.
But as I mentioned elsewhere, I think this should actually call an end_bio()
callback itself and return void, which would also avoid this issue.
> +
> +static inline int read_callbacks_setup(struct inode *inode,
> + struct bio *bio, end_page_op_t page_op)
> +{
> + return -EOPNOTSUPP;
> +}
Similarly here, this stub needs to return 0.
Thanks!
- Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 2/7] Integrate read callbacks into Ext4 and F2FS
2019-06-16 16:08 ` [PATCH V3 2/7] Integrate read callbacks into Ext4 and F2FS Chandan Rajendra
@ 2019-06-21 21:08 ` Eric Biggers
2019-06-25 6:05 ` Chandan Rajendra
0 siblings, 1 reply; 19+ messages in thread
From: Eric Biggers @ 2019-06-21 21:08 UTC (permalink / raw)
To: Chandan Rajendra
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
Hi Chandan,
On Sun, Jun 16, 2019 at 09:38:08PM +0530, Chandan Rajendra wrote:
> This commit gets Ext4 and F2FS to make use of read callbacks API to
> perform decryption of file data read from the disk.
> ---
> fs/crypto/bio.c | 30 +--------
> fs/crypto/crypto.c | 1 +
> fs/crypto/fscrypt_private.h | 3 +
> fs/ext4/readpage.c | 29 +++------
> fs/f2fs/data.c | 124 +++++++-----------------------------
> fs/f2fs/super.c | 9 +--
> fs/read_callbacks.c | 1 -
> include/linux/fscrypt.h | 18 ------
> 8 files changed, 40 insertions(+), 175 deletions(-)
>
This patch changes many different components. It would be much easier to
review, and might get more attention from the other ext4 and f2fs developers, if
it were split into 3 patches:
a. Convert ext4 to use read_callbacks.
b. Convert f2fs to use read_callbacks.
c. Remove the functions from fs/crypto/ that became unused as a result of
patches (a) and (b). (Actually, this part probably should be merged with the
patch that removes the fscrypt_ctx, and the patch renamed to something like
"fscrypt: remove decryption I/O path helpers")
Any reason why this wouldn't work? AFAICS, you couldn't do it only because you
made this patch change fscrypt_enqueue_decrypt_work() to be responsible for
initializing the work function. But as per my comments on patch 1, I don't
think we should do that, since it would make much more sense to put the work
function in read_callbacks.c.
However, since you're converting ext4 to use mpage_readpages() anyway, I don't
think we should bother with the intermediate change to ext4_mpage_readpages().
It's useless, and that intermediate state of the ext4 code inevitably won't get
tested very well. So perhaps order the whole series as:
- fs: introduce read_callbacks
- fs/mpage.c: add decryption support via read_callbacks
- fs/buffer.c: add decryption support via read_callbacks
- f2fs: convert to use read_callbacks
- ext4: convert to use mpage_readpages[s]
- ext4: support encryption with subpage-sized blocks
- fscrypt: remove decryption I/O path helpers
That order would also give the flexibility to possibly apply the fs/ changes
first, without having to update both ext4 and f2fs simultaneously with them.
> @@ -557,8 +511,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> struct bio *bio;
> - struct bio_post_read_ctx *ctx;
> - unsigned int post_read_steps = 0;
> + int ret;
Nit: 'err' rather than 'ret', since this is 0 or a -errno value.
> -int __init f2fs_init_post_read_processing(void)
> -{
> - bio_post_read_ctx_cache = KMEM_CACHE(bio_post_read_ctx, 0);
> - if (!bio_post_read_ctx_cache)
> - goto fail;
> - bio_post_read_ctx_pool =
> - mempool_create_slab_pool(NUM_PREALLOC_POST_READ_CTXS,
> - bio_post_read_ctx_cache);
> - if (!bio_post_read_ctx_pool)
> - goto fail_free_cache;
> - return 0;
> -
> -fail_free_cache:
> - kmem_cache_destroy(bio_post_read_ctx_cache);
> -fail:
> - return -ENOMEM;
> -}
> -
> -void __exit f2fs_destroy_post_read_processing(void)
> -{
> - mempool_destroy(bio_post_read_ctx_pool);
> - kmem_cache_destroy(bio_post_read_ctx_cache);
> -}
Need to remove the declarations of these functions from fs/f2fs/f2fs.h to.
> diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
> index a4196e3de05f..4b7fc2a349cd 100644
> --- a/fs/read_callbacks.c
> +++ b/fs/read_callbacks.c
> @@ -76,7 +76,6 @@ void read_callbacks(struct read_callbacks_ctx *ctx)
> switch (++ctx->cur_step) {
> case STEP_DECRYPT:
> if (ctx->enabled_steps & (1 << STEP_DECRYPT)) {
> - INIT_WORK(&ctx->work, fscrypt_decrypt_work);
> fscrypt_enqueue_decrypt_work(&ctx->work);
> return;
> }
Again, I think the work initialization should remain here as:
INIT_WORK(&ctx->work, decrypt_work);
rather than moving it to fs/crypto/.
Thanks!
- Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 4/7] fs/mpage.c: Integrate read callbacks
2019-06-16 16:08 ` [PATCH V3 4/7] fs/mpage.c: Integrate read callbacks Chandan Rajendra
@ 2019-06-21 21:14 ` Eric Biggers
2019-06-25 6:21 ` Chandan Rajendra
0 siblings, 1 reply; 19+ messages in thread
From: Eric Biggers @ 2019-06-21 21:14 UTC (permalink / raw)
To: Chandan Rajendra
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Sun, Jun 16, 2019 at 09:38:10PM +0530, Chandan Rajendra wrote:
> This commit adds code to make do_mpage_readpage() to be "read callbacks"
> aware i.e. for files requiring decryption, do_mpage_readpage() now
> sets up the read callbacks state machine when allocating a bio and later
> starts execution of the state machine after file data is read from the
> underlying disk.
>
> Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
> ---
> fs/mpage.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/fs/mpage.c b/fs/mpage.c
> index 436a85260394..611ad122fc92 100644
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -30,6 +30,7 @@
> #include <linux/backing-dev.h>
> #include <linux/pagevec.h>
> #include <linux/cleancache.h>
> +#include <linux/read_callbacks.h>
> #include "internal.h"
>
> /*
> @@ -49,6 +50,8 @@ static void mpage_end_io(struct bio *bio)
> struct bio_vec *bv;
> struct bvec_iter_all iter_all;
>
> + if (read_callbacks_end_bio(bio))
> + return;
> bio_for_each_segment_all(bv, bio, iter_all) {
> struct page *page = bv->bv_page;
> page_endio(page, bio_op(bio),
> @@ -309,6 +312,12 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
> gfp);
> if (args->bio == NULL)
> goto confused;
> +
> + if (read_callbacks_setup(inode, args->bio, NULL)) {
> + bio_put(args->bio);
> + args->bio = NULL;
> + goto confused;
> + }
> }
>
> length = first_hole << blkbits;
> @@ -330,7 +339,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
> confused:
> if (args->bio)
> args->bio = mpage_bio_submit(REQ_OP_READ, op_flags, args->bio);
> - if (!PageUptodate(page))
> + if (!PageUptodate(page) && !PageError(page))
> block_read_full_page(page, args->get_block);
> else
> unlock_page(page);
> --
> 2.19.1
Why is the !PageError() check needed here?
- Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 6/7] Add decryption support for sub-pagesized blocks
2019-06-16 16:08 ` [PATCH V3 6/7] Add decryption support for sub-pagesized blocks Chandan Rajendra
@ 2019-06-21 21:29 ` Eric Biggers
2019-06-25 6:22 ` Chandan Rajendra
0 siblings, 1 reply; 19+ messages in thread
From: Eric Biggers @ 2019-06-21 21:29 UTC (permalink / raw)
To: Chandan Rajendra
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Sun, Jun 16, 2019 at 09:38:12PM +0530, Chandan Rajendra wrote:
> To support decryption of sub-pagesized blocks this commit adds code to,
> 1. Track buffer head in "struct read_callbacks_ctx".
> 2. Pass buffer head argument to all read callbacks.
> 3. Add new fscrypt helper to decrypt the file data referred to by a
> buffer head.
>
> Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
> ---
> fs/buffer.c | 55 +++++++++------
> fs/crypto/bio.c | 21 +++++-
> fs/f2fs/data.c | 2 +-
> fs/mpage.c | 2 +-
> fs/read_callbacks.c | 118 +++++++++++++++++++++++++--------
> include/linux/buffer_head.h | 1 +
> include/linux/read_callbacks.h | 13 +++-
> 7 files changed, 158 insertions(+), 54 deletions(-)
>
This is another patch that unnecessarily changes way too many components at
once. My suggestions elsewhere would resolve this, though:
- This patch changes fs/f2fs/data.c and fs/mpage.c only to pass a NULL
buffer_head to read_callbacks_setup(). But as per my comments on patch 1,
read_callbacks_setup() should be split into read_callbacks_setup_bio() and
read_callbacks_end_bh().
- This patch changes fs/crypto/ only to add support for the buffer_head
decryption work. But as per my comments on patch 1, that should be in
read_callbacks.c instead.
And adding buffer_head support to fs/read_callbacks.c should be its own patch,
*or* should simply be folded into the patch that adds fs/read_callbacks.c.
Then the only thing remaining in this patch would be updating fs/buffer.c to
make it use the read_callbacks, which should be retitled to something like
"fs/buffer.c: add decryption support via read_callbacks".
- Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 3/7] fscrypt: remove struct fscrypt_ctx
2019-06-16 16:08 ` [PATCH V3 3/7] fscrypt: remove struct fscrypt_ctx Chandan Rajendra
@ 2019-06-21 22:00 ` Eric Biggers
0 siblings, 0 replies; 19+ messages in thread
From: Eric Biggers @ 2019-06-21 22:00 UTC (permalink / raw)
To: Chandan Rajendra
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Sun, Jun 16, 2019 at 09:38:09PM +0530, Chandan Rajendra wrote:
> -/**
> - * 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);
FS_CTX_REQUIRES_FREE_ENCRYPT_FL is no longer used, so should be removed.
- Eric
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 0/7] Consolidate FS read I/O callbacks code
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
` (6 preceding siblings ...)
2019-06-16 16:08 ` [PATCH V3 7/7] ext4: Enable encryption for subpage-sized blocks Chandan Rajendra
@ 2019-06-21 22:15 ` Eric Biggers
2019-06-25 6:24 ` Chandan Rajendra
7 siblings, 1 reply; 19+ messages in thread
From: Eric Biggers @ 2019-06-21 22:15 UTC (permalink / raw)
To: Chandan Rajendra
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Sun, Jun 16, 2019 at 09:38:06PM +0530, 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-v3.
>
FWIW: while doing my review I put together an (untested) incremental patch that
addresses my comments on the code, so I've provided it below in case you want to
start with it when addressing my comments.
This is just a single diff against your subpage-encryption-v3 branch, so of
course it would still need to be folded into the appropriate patches. Also see
my suggestions in reply to patch 2 about how to better organize the series. I
also left TODOs in kerneldoc comments that still need to be updated.
diff --git a/fs/Kconfig b/fs/Kconfig
index d869859c88da18..d95897a0f3d052 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -22,7 +22,7 @@ config FS_IOMAP
bool
config FS_READ_CALLBACKS
- bool
+ bool
source "fs/ext2/Kconfig"
source "fs/ext4/Kconfig"
diff --git a/fs/Makefile b/fs/Makefile
index a1a06f0db5c181..81854d8161dea7 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -21,8 +21,6 @@ else
obj-y += no-block.o
endif
-obj-$(CONFIG_FS_READ_CALLBACKS) += read_callbacks.o
-
obj-$(CONFIG_PROC_FS) += proc_namespace.o
obj-y += notify/
@@ -55,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/buffer.c b/fs/buffer.c
index dcb67525dac91d..bfda6af7ea1042 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -247,7 +247,7 @@ __find_get_block_slow(struct block_device *bdev, sector_t block)
return ret;
}
-void end_buffer_async_read(struct buffer_head *bh)
+void end_buffer_async_read(struct buffer_head *bh, int uptodate)
{
unsigned long flags;
struct buffer_head *first;
@@ -255,7 +255,17 @@ void end_buffer_async_read(struct buffer_head *bh)
struct page *page;
int page_uptodate = 1;
+ BUG_ON(!buffer_async_read(bh));
+
page = bh->b_page;
+ if (uptodate) {
+ set_buffer_uptodate(bh);
+ } else {
+ clear_buffer_uptodate(bh);
+ buffer_io_error(bh, ", async page read");
+ SetPageError(page);
+ }
+
/*
* Be _very_ careful from here on. Bad things can happen if
* two buffer heads end IO at almost the same time and both
@@ -298,25 +308,9 @@ void end_buffer_async_read(struct buffer_head *bh)
* 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)
+static void end_buffer_async_read_cbs(struct buffer_head *bh, int uptodate)
{
- struct page *page;
-
- BUG_ON(!buffer_async_read(bh));
-
- if (read_callbacks_end_bh(bh, uptodate))
- return;
-
- page = bh->b_page;
- if (uptodate) {
- set_buffer_uptodate(bh);
- } else {
- clear_buffer_uptodate(bh);
- buffer_io_error(bh, ", async page read");
- SetPageError(page);
- }
-
- end_buffer_async_read(bh);
+ read_callbacks_endio_bh(bh, uptodate);
}
/*
@@ -391,7 +385,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_cbs;
set_buffer_async_read(bh);
}
@@ -2307,10 +2301,10 @@ 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);
+ end_buffer_async_read(bh, 1);
} else {
- if (WARN_ON(read_callbacks_setup(inode, NULL, bh, NULL))) {
- __end_buffer_async_read(bh, 0);
+ if (read_callbacks_setup_bh(inode, bh)) {
+ end_buffer_async_read(bh, 0);
continue;
}
submit_bh(REQ_OP_READ, 0, bh);
diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 7752f99642808b..24ed99e2eca0b2 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -9,7 +9,6 @@ config FS_ENCRYPTION
select CRYPTO_CTS
select CRYPTO_SHA256
select KEYS
- select FS_READ_CALLBACKS if BLOCK
help
Enable encryption of files and directories. This
feature is similar to ecryptfs, but it is more memory
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index b836d648fd2713..f65cc3059e5c77 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -24,52 +24,8 @@
#include <linux/module.h>
#include <linux/bio.h>
#include <linux/namei.h>
-#include <linux/buffer_head.h>
-#include <linux/read_callbacks.h>
#include "fscrypt_private.h"
-static void fscrypt_decrypt_bio(struct bio *bio)
-{
- struct bio_vec *bv;
- struct bvec_iter_all iter_all;
-
- bio_for_each_segment_all(bv, bio, 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 fscrypt_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);
-}
-
-void fscrypt_decrypt_work(struct work_struct *work)
-{
- struct read_callbacks_ctx *ctx =
- container_of(work, struct read_callbacks_ctx, work);
-
- WARN_ON(!ctx->bh && !ctx->bio);
-
- if (ctx->bio)
- fscrypt_decrypt_bio(ctx->bio);
- else
- fscrypt_decrypt_bh(ctx->bh);
-
- read_callbacks(ctx);
-}
-
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 18c6b664b042fb..56697ba58f8676 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -46,7 +46,6 @@ struct kmem_cache *fscrypt_info_cachep;
void fscrypt_enqueue_decrypt_work(struct work_struct *work)
{
- INIT_WORK(work, fscrypt_decrypt_work);
queue_work(fscrypt_read_workqueue, work);
}
EXPORT_SYMBOL(fscrypt_enqueue_decrypt_work);
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 0bd65921efbf1c..702403dc761464 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -93,8 +93,6 @@ typedef enum {
FS_ENCRYPT,
} fscrypt_direction_t;
-#define FS_CTX_REQUIRES_FREE_ENCRYPT_FL 0x00000001
-
static inline bool fscrypt_valid_enc_modes(u32 contents_mode,
u32 filenames_mode)
{
@@ -113,9 +111,6 @@ static inline bool fscrypt_valid_enc_modes(u32 contents_mode,
return false;
}
-/* bio.c */
-void fscrypt_decrypt_work(struct work_struct *work);
-
/* crypto.c */
extern struct kmem_cache *fscrypt_info_cachep;
extern int fscrypt_initialize(unsigned int cop_flags);
diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
index cbb5ca830e57f3..c232aab7260ffd 100644
--- a/fs/ext4/Kconfig
+++ b/fs/ext4/Kconfig
@@ -39,6 +39,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 110a38ca5d53a7..c66072dee9bb5d 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -5,6 +5,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/f2fs/data.c b/fs/f2fs/data.c
index 1e8b1eb68a90a3..cf17b19b292557 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -65,20 +65,6 @@ static enum count_type __read_io_type(struct page *page)
return F2FS_RD_DATA;
}
-static void f2fs_end_page_op(struct bio *bio, struct page *page)
-{
- BUG_ON(!PageLocked(page));
-
- /* PG_error was set if any post_read step failed */
- if (bio->bi_status || PageError(page)) {
- ClearPageUptodate(page);
- /* will re-read again later */
- ClearPageError(page);
- }
-
- dec_page_count(F2FS_P_SB(page), __read_io_type(page));
-}
-
static void __read_end_io(struct bio *bio)
{
struct page *page;
@@ -88,14 +74,16 @@ static void __read_end_io(struct bio *bio)
bio_for_each_segment_all(bv, bio, iter_all) {
page = bv->bv_page;
- if (!bio->bi_status && !PageError(page))
+ if (bio->bi_status || read_callbacks_failed(page)) {
+ ClearPageUptodate(page);
+ /* will re-read again later */
+ ClearPageError(page);
+ } else {
SetPageUptodate(page);
-
- f2fs_end_page_op(bio, page);
-
+ }
+ dec_page_count(F2FS_P_SB(page), __read_io_type(page));
unlock_page(page);
}
-
bio_put(bio);
}
@@ -107,10 +95,7 @@ static void f2fs_read_end_io(struct bio *bio)
bio->bi_status = BLK_STS_IOERR;
}
- if (read_callbacks_end_bio(bio))
- return;
-
- __read_end_io(bio);
+ read_callbacks_endio_bio(bio, __read_end_io);
}
static void f2fs_write_end_io(struct bio *bio)
@@ -511,7 +496,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
{
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct bio *bio;
- int ret;
+ int err;
bio = f2fs_bio_alloc(sbi, min_t(int, nr_pages, BIO_MAX_PAGES), false);
if (!bio)
@@ -520,10 +505,10 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
bio->bi_end_io = f2fs_read_end_io;
bio_set_op_attrs(bio, REQ_OP_READ, op_flag);
- ret = read_callbacks_setup(inode, bio, NULL, f2fs_end_page_op);
- if (ret) {
+ err = read_callbacks_setup_bio(inode, bio);
+ if (err) {
bio_put(bio);
- return ERR_PTR(ret);
+ return ERR_PTR(err);
}
return bio;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 06b89a9862ab2b..182c1d91d150b9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3161,8 +3161,6 @@ void f2fs_destroy_checkpoint_caches(void);
/*
* data.c
*/
-int f2fs_init_post_read_processing(void);
-void f2fs_destroy_post_read_processing(void);
void f2fs_submit_merged_write(struct f2fs_sb_info *sbi, enum page_type type);
void f2fs_submit_merged_write_cond(struct f2fs_sb_info *sbi,
struct inode *inode, struct page *page,
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index e2e6a8c7236c7f..2c8123b1587166 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3627,9 +3627,7 @@ static int __init init_f2fs_fs(void)
err = register_filesystem(&f2fs_fs_type);
if (err)
goto free_shrinker;
-
f2fs_create_root_stats();
-
return 0;
free_shrinker:
diff --git a/fs/mpage.c b/fs/mpage.c
index 387c23b529eb90..bf0549d9ce656e 100644
--- a/fs/mpage.c
+++ b/fs/mpage.c
@@ -33,6 +33,25 @@
#include <linux/read_callbacks.h>
#include "internal.h"
+static void __mpage_end_io(struct bio *bio)
+{
+ struct bio_vec *bv;
+ struct bvec_iter_all iter_all;
+ bool is_write = (bio_op(bio) != REQ_OP_READ);
+ int err = blk_status_to_errno(bio->bi_status);
+
+ bio_for_each_segment_all(bv, bio, iter_all) {
+ struct page *page = bv->bv_page;
+ int this_err = err;
+
+ if (!err && !is_write && read_callbacks_failed(page))
+ this_err = -EIO;
+
+ page_endio(page, is_write, this_err);
+ }
+ bio_put(bio);
+}
+
/*
* I/O completion handler for multipage BIOs.
*
@@ -47,18 +66,7 @@
*/
static void mpage_end_io(struct bio *bio)
{
- struct bio_vec *bv;
- struct bvec_iter_all iter_all;
-
- if (read_callbacks_end_bio(bio))
- return;
- bio_for_each_segment_all(bv, bio, iter_all) {
- struct page *page = bv->bv_page;
- page_endio(page, bio_op(bio),
- blk_status_to_errno(bio->bi_status));
- }
-
- bio_put(bio);
+ read_callbacks_endio_bio(bio, __mpage_end_io);
}
static struct bio *mpage_bio_submit(int op, int op_flags, struct bio *bio)
@@ -313,7 +321,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
if (args->bio == NULL)
goto confused;
- if (read_callbacks_setup(inode, args->bio, NULL, NULL)) {
+ if (read_callbacks_setup_bio(inode, args->bio)) {
bio_put(args->bio);
args->bio = NULL;
goto confused;
@@ -339,7 +347,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
confused:
if (args->bio)
args->bio = mpage_bio_submit(REQ_OP_READ, op_flags, args->bio);
- if (!PageUptodate(page) && !PageError(page))
+ if (!PageUptodate(page))
block_read_full_page(page, args->get_block);
else
unlock_page(page);
diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
index 7b3ab11c1652f6..3c2baffebb6159 100644
--- a/fs/read_callbacks.c
+++ b/fs/read_callbacks.c
@@ -1,15 +1,23 @@
// 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.
+ * fs/read_callbacks.c
+ *
+ * Utility functions for filesystems to postprocess data after I/O completion.
+ * Currently this only supports decryption (fscrypt), but eventually it will
+ * support fs-verity too.
+ *
+ * Before submitting a read I/O, filesystems call read_callbacks_setup_bio() (or
+ * read_callbacks_setup_bh()) to allocate and store a read_callbacks_ctx in
+ * bio::bi_private (or buffer_head::b_private). After the I/O completes, they
+ * call read_callbacks_endio_bio() (or read_callbacks_endio_bh()) to execute the
+ * postprocessing steps and free the context if needed, then unlock the pages.
+ *
+ * All pages into which the I/O is being done are assumed to be pagecache pages.
*/
-#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/mm.h>
#include <linux/read_callbacks.h>
#define NUM_PREALLOC_READ_CALLBACK_CTXS 128
@@ -23,194 +31,182 @@ enum read_callbacks_step {
STEP_DECRYPT,
};
-static void put_read_callbacks_ctx(struct read_callbacks_ctx *ctx)
-{
- mempool_free(ctx, read_callbacks_ctx_pool);
-}
+struct read_callbacks_ctx {
+ unsigned int enabled_steps;
+ unsigned int cur_step;
+ struct bio *bio;
+ struct buffer_head *bh;
+ end_bio_func_t end_bio;
+ struct work_struct work;
+};
-static void end_read_callbacks_bio(struct bio *bio)
+static struct read_callbacks_ctx *get_read_callbacks_ctx(struct inode *inode)
{
- struct read_callbacks_ctx *ctx;
- struct page *page;
- struct bio_vec *bv;
- struct bvec_iter_all iter_all;
-
- ctx = bio->bi_private;
+ struct read_callbacks_ctx *ctx = NULL;
+ unsigned int enabled_steps = 0;
- bio_for_each_segment_all(bv, bio, iter_all) {
- page = bv->bv_page;
+ if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
+ enabled_steps |= 1 << STEP_DECRYPT;
- if (bio->bi_status || PageError(page)) {
- ClearPageUptodate(page);
- SetPageError(page);
- } else {
- SetPageUptodate(page);
- }
+ if (enabled_steps) {
+ ctx = mempool_alloc(read_callbacks_ctx_pool, GFP_NOFS);
+ if (!ctx)
+ return ERR_PTR(-ENOMEM);
+ ctx->enabled_steps = enabled_steps;
+ ctx->cur_step = STEP_INITIAL;
+ }
+ return ctx;
+}
- if (ctx->page_op)
- ctx->page_op(bio, page);
+/**
+ * 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
+ *
+ * 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; -errno on failure.
+ */
+int read_callbacks_setup_bio(struct inode *inode, struct bio *bio)
+{
+ struct read_callbacks_ctx *ctx = get_read_callbacks_ctx(inode);
- unlock_page(page);
+ 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);
- put_read_callbacks_ctx(ctx);
+/**
+ * read_callbacks_setup_bh() - TODO
+ */
+int read_callbacks_setup_bh(struct inode *inode, struct buffer_head *bh)
+{
+ struct read_callbacks_ctx *ctx = get_read_callbacks_ctx(inode);
- bio_put(bio);
+ if (ctx) {
+ if (IS_ERR(ctx))
+ return PTR_ERR(ctx);
+ ctx->bh = bh;
+ ctx->bio = NULL;
+ bh->b_private = ctx;
+ }
+ return 0;
}
+EXPORT_SYMBOL(read_callbacks_setup_bh);
-static void end_read_callbacks_bh(struct buffer_head *bh)
+static void free_read_callbacks_ctx(struct read_callbacks_ctx *ctx)
{
- struct read_callbacks_ctx *ctx;
-
- if (!PageError(bh->b_page))
- set_buffer_uptodate(bh);
+ mempool_free(ctx, read_callbacks_ctx_pool);
+}
- ctx = bh->b_private;
+static void read_callbacks(struct read_callbacks_ctx *ctx);
- end_buffer_async_read(bh);
+static void decrypt_bio(struct bio *bio)
+{
+ struct bio_vec *bv;
+ struct bvec_iter_all iter_all;
- put_read_callbacks_ctx(ctx);
+ bio_for_each_segment_all(bv, bio, 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 end_read_callbacks(struct bio *bio, struct buffer_head *bh)
+static void decrypt_bh(struct buffer_head *bh)
{
- if (bio)
- end_read_callbacks_bio(bio);
- else
- end_read_callbacks_bh(bh);
+ struct page *page = bh->b_page;
+
+ if (fscrypt_decrypt_pagecache_blocks(page, bh->b_size, bh_offset(bh)))
+ SetPageError(page);
}
-/**
- * read_callbacks_end_bio() - Initiate the read callbacks state machine.
- * @bio: bio on which read I/O operation has just been completed.
- *
- * Initiates the execution of the read callbacks state machine when the read
- * operation has been completed successfully. If there was an error associated
- * with the bio, this function frees the read callbacks context structure stored
- * in bio->bi_private (if any).
- *
- * Return: 1 to indicate that the bio has been handled (including unlocking the
- * pages); 0 otherwise.
- */
-int read_callbacks_end_bio(struct bio *bio)
+static void decrypt_work(struct work_struct *work)
{
- if (!bio->bi_status && bio->bi_private) {
- read_callbacks((struct read_callbacks_ctx *)(bio->bi_private));
- return 1;
- }
+ struct read_callbacks_ctx *ctx =
+ container_of(work, struct read_callbacks_ctx, work);
- if (bio->bi_private)
- put_read_callbacks_ctx((struct read_callbacks_ctx *)(bio->bi_private));
+ if (ctx->bio)
+ decrypt_bio(ctx->bio);
+ else
+ decrypt_bh(ctx->bh);
- return 0;
+ read_callbacks(ctx);
}
-EXPORT_SYMBOL(read_callbacks_end_bio);
-/**
- * read_callbacks_end_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.
- *
- * Initiates the execution of the read callbacks state machine when the read
- * operation has been completed successfully. 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).
- *
- * Return: 1 to indicate that the buffer head has been handled (including
- * unlocking the buffer head and the corresponding page if applicable); 0
- * otherwise.
- */
-int read_callbacks_end_bh(struct buffer_head *bh, int uptodate)
+static void end_read_callbacks(struct read_callbacks_ctx *ctx)
{
- if (uptodate && bh->b_private) {
- read_callbacks((struct read_callbacks_ctx *)(bh->b_private));
- return 1;
- }
-
- if (bh->b_private)
- put_read_callbacks_ctx((struct read_callbacks_ctx *)(bh->b_private));
-
- return 0;
+ if (ctx->bio)
+ ctx->end_bio(ctx->bio);
+ else
+ end_buffer_async_read(ctx->bh,
+ !read_callbacks_failed(ctx->bh->b_page));
+ free_read_callbacks_ctx(ctx);
}
-EXPORT_SYMBOL(read_callbacks_end_bh);
-/**
- * read_callbacks() - Execute the read callbacks state machine.
- * @ctx: The context structure tracking the current state.
- *
- * For each state, this function enqueues a work into appropriate subsystem's
- * work queue. After performing further processing of the data in the bio's
- * pages, the subsystem should invoke read_calbacks() to continue with the next
- * state in the state machine.
- */
-void read_callbacks(struct read_callbacks_ctx *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:
- end_read_callbacks(ctx->bio, ctx->bh);
+ end_read_callbacks(ctx);
}
}
-EXPORT_SYMBOL(read_callbacks);
/**
- * read_callbacks_setup() - 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.
+ * read_callbacks_endio_bio() - TODO
*/
-int read_callbacks_setup(struct inode *inode, struct bio *bio,
- struct buffer_head *bh, end_page_op_t page_op)
+void read_callbacks_endio_bio(struct bio *bio, end_bio_func_t end_bio)
{
- struct read_callbacks_ctx *ctx = NULL;
- unsigned int enabled_steps = 0;
-
- if (!bh && !bio)
- return -EINVAL;
+ struct read_callbacks_ctx *ctx = bio->bi_private;
- if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
- enabled_steps |= 1 << STEP_DECRYPT;
+ 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);
- if (enabled_steps) {
- ctx = mempool_alloc(read_callbacks_ctx_pool, GFP_NOFS);
- if (!ctx)
- return -ENOMEM;
+/**
+ * read_callbacks_endio_bh() - TODO
+ */
+void read_callbacks_endio_bh(struct buffer_head *bh, int uptodate)
+{
+ struct read_callbacks_ctx *ctx = bh->b_private;
- ctx->bh = bh;
- ctx->bio = bio;
- ctx->inode = inode;
- ctx->enabled_steps = enabled_steps;
- ctx->cur_step = STEP_INITIAL;
- ctx->page_op = page_op;
- if (bio)
- bio->bi_private = ctx;
- else
- bh->b_private = ctx;
+ if (ctx) {
+ if (uptodate) {
+ read_callbacks(ctx);
+ return;
+ }
+ free_read_callbacks_ctx(ctx);
}
-
- return 0;
+ end_buffer_async_read(bh, uptodate);
}
-EXPORT_SYMBOL(read_callbacks_setup);
+EXPORT_SYMBOL(read_callbacks_endio_bh);
static int __init init_read_callbacks(void)
{
@@ -229,5 +225,4 @@ static int __init init_read_callbacks(void)
fail:
return -ENOMEM;
}
-
fs_initcall(init_read_callbacks);
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 42d0d63c7a3b38..5176a7a7e22aab 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -165,7 +165,7 @@ void create_empty_buffers(struct page *, unsigned long,
void end_buffer_read_sync(struct buffer_head *bh, int uptodate);
void end_buffer_write_sync(struct buffer_head *bh, int uptodate);
void end_buffer_async_write(struct buffer_head *bh, int uptodate);
-void end_buffer_async_read(struct buffer_head *bh);
+void end_buffer_async_read(struct buffer_head *bh, int uptodate);
/* Things to do with buffers at mapping->private_list */
void mark_buffer_dirty_inode(struct buffer_head *bh, struct inode *inode);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 13012c589aa32e..6d2d16d79752bb 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -402,6 +402,7 @@ static inline bool fscrypt_match_name(const struct fscrypt_name *fname,
return !memcmp(de_name, fname->disk_name.name, fname->disk_name.len);
}
+/* bio.c */
static inline int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
sector_t pblk, unsigned int len)
{
diff --git a/include/linux/read_callbacks.h b/include/linux/read_callbacks.h
index 0b52d7961fb2c8..610324f362a87e 100644
--- a/include/linux/read_callbacks.h
+++ b/include/linux/read_callbacks.h
@@ -2,44 +2,58 @@
#ifndef _READ_CALLBACKS_H
#define _READ_CALLBACKS_H
-typedef void (*end_page_op_t)(struct bio *bio, struct page *page);
-
-struct read_callbacks_ctx {
- struct buffer_head *bh;
- struct bio *bio;
- struct inode *inode;
- struct work_struct work;
- unsigned int cur_step;
- unsigned int enabled_steps;
- end_page_op_t page_op;
-};
+#include <linux/buffer_head.h>
+
+struct bio;
+struct page;
+struct read_callbacks_ctx;
+
+typedef void (*end_bio_func_t)(struct bio *bio);
#ifdef CONFIG_FS_READ_CALLBACKS
-void read_callbacks(struct read_callbacks_ctx *ctx);
-int read_callbacks_end_bio(struct bio *bio);
-int read_callbacks_end_bh(struct buffer_head *bh, int uptodate);
-int read_callbacks_setup(struct inode *inode, struct bio *bio,
- struct buffer_head *bh, end_page_op_t page_op);
-#else
-static inline void read_callbacks(struct read_callbacks_ctx *ctx)
+
+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);
+
+static inline bool read_callbacks_failed(struct page *page)
{
+ return PageError(page);
}
-static inline int read_callbacks_end_bio(struct bio *bio)
+#else /* !CONFIG_FS_READ_CALLBACKS */
+
+static inline int read_callbacks_setup_bio(struct inode *inode, struct bio *bio)
{
- return -EOPNOTSUPP;
+ return 0;
}
-static inline int read_callbacks_end_bh(struct buffer_head *bh, int uptodate)
+static inline int read_callbacks_setup_bh(struct inode *inode,
+ struct buffer_head *bh)
{
- return -EOPNOTSUPP;
+ return 0;
}
-static inline int read_callbacks_setup(struct inode *inode, struct bio *bio,
- struct buffer_head *bh, end_page_op_t page_op)
+static inline void read_callbacks_endio_bio(struct bio *bio,
+ end_bio_func_t end_bio)
{
- return -EOPNOTSUPP;
+ end_bio(bio);
}
-#endif
+
+static inline void read_callbacks_endio_bh(struct buffer_head *bh, int uptodate)
+{
+ end_buffer_async_read(bh, uptodate);
+}
+
+static inline bool read_callbacks_failed(struct page *page)
+{
+ return false;
+}
+
+#endif /* !CONFIG_FS_READ_CALLBACKS */
#endif /* _READ_CALLBACKS_H */
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH V3 1/7] FS: Introduce read callbacks
2019-06-21 20:03 ` Eric Biggers
@ 2019-06-25 4:59 ` Chandan Rajendra
0 siblings, 0 replies; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-25 4:59 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Saturday, June 22, 2019 1:33:57 AM IST Eric Biggers wrote:
> Hi Chandan,
>
> On Sun, Jun 16, 2019 at 09:38:07PM +0530, Chandan Rajendra wrote:
> > 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 <chandan@linux.ibm.com>
> > ---
> > fs/Kconfig | 3 +
> > fs/Makefile | 2 +
> > fs/crypto/Kconfig | 1 +
> > fs/crypto/bio.c | 11 +++
> > fs/read_callbacks.c | 174 +++++++++++++++++++++++++++++++++
> > include/linux/fscrypt.h | 5 +
> > include/linux/read_callbacks.h | 38 +++++++
> > 7 files changed, 234 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 f1046cf6ad85..d869859c88da 100644
> > --- a/fs/Kconfig
> > +++ b/fs/Kconfig
> > @@ -21,6 +21,9 @@ if BLOCK
> > config FS_IOMAP
> > bool
> >
> > +config FS_READ_CALLBACKS
> > + bool
>
> This should be intended with a tab, not spaces.
>
> > +
> > source "fs/ext2/Kconfig"
> > source "fs/ext4/Kconfig"
> > source "fs/jbd2/Kconfig"
> > diff --git a/fs/Makefile b/fs/Makefile
> > index c9aea23aba56..a1a06f0db5c1 100644
> > --- a/fs/Makefile
> > +++ b/fs/Makefile
> > @@ -21,6 +21,8 @@ else
> > obj-y += no-block.o
> > endif
> >
> > +obj-$(CONFIG_FS_READ_CALLBACKS) += read_callbacks.o
> > +
> > obj-$(CONFIG_PROC_FS) += proc_namespace.o
>
> Nit: maybe move this to just below the line for iomap.o, to be consistent with
> where FS_READ_CALLBACKS is in the Kconfig file.
>
> >
> > obj-y += notify/
> > diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> > index 24ed99e2eca0..7752f9964280 100644
> > --- a/fs/crypto/Kconfig
> > +++ b/fs/crypto/Kconfig
> > @@ -9,6 +9,7 @@ config FS_ENCRYPTION
> > select CRYPTO_CTS
> > select CRYPTO_SHA256
> > select KEYS
> > + select FS_READ_CALLBACKS if BLOCK
> > help
> > Enable encryption of files and directories. This
> > feature is similar to ecryptfs, but it is more memory
> > diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> > index 82da2510721f..f677ff93d464 100644
> > --- a/fs/crypto/bio.c
> > +++ b/fs/crypto/bio.c
> > @@ -24,6 +24,7 @@
> > #include <linux/module.h>
> > #include <linux/bio.h>
> > #include <linux/namei.h>
> > +#include <linux/read_callbacks.h>
> > #include "fscrypt_private.h"
> >
> > static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> > @@ -68,6 +69,16 @@ void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
> > }
> > EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
> >
> > +void fscrypt_decrypt_work(struct work_struct *work)
> > +{
> > + struct read_callbacks_ctx *ctx =
> > + container_of(work, struct read_callbacks_ctx, work);
> > +
> > + fscrypt_decrypt_bio(ctx->bio);
> > +
> > + read_callbacks(ctx);
> > +}
> > +
>
> This seems like a layering violation, since it means that read_callbacks.c calls
> fs/crypto/ *and* fs/crypto/ calls read_callbacks.c. I don't think fs/crypto/
> should be aware of read_callbacks at all. Instead we should have a clear
> ordering between the components: the filesystem uses read_callbacks.c and
> fs/crypto/, and read_callbacks.c uses fs/crypto/. So how about:
>
> - Move fscrypt_decrypt_work(), fscrypt_decrypt_bh(), and fscrypt_decrypt_bio()
> into fs/read_callbacks.c and remove the "fscrypt_" prefix from them.
>
> - Instead of selecting FS_READ_CALLBACKS from FS_ENCRYPTION, select it from
> EXT4_FS and F2FS_FS (if FS_ENCRYPTION). I.e., it's really the *filesystems*
> themselves that use read_callbacks, not fs/crypto/.
>
> - Move the definition of read_callbacks_ctx into fs/read_callbacks.c, and make
> read_callbacks() static, so these are private to the read_callbacks component.
>
> > int fscrypt_zeroout_range(const struct inode *inode, pgoff_t lblk,
> > sector_t pblk, unsigned int len)
> > {
> > diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
> > new file mode 100644
> > index 000000000000..a4196e3de05f
> > --- /dev/null
> > +++ b/fs/read_callbacks.c
> > @@ -0,0 +1,174 @@
> > +// 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/fscrypt.h>
> > +#include <linux/read_callbacks.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,
> > +};
> > +
> > +static void put_read_callbacks_ctx(struct read_callbacks_ctx *ctx)
> > +{
> > + mempool_free(ctx, read_callbacks_ctx_pool);
> > +}
>
> Maybe call this free_read_callbacks_ctx(), so that it doesn't sound like it's
> doing reference counting.
>
> > +
> > +static void end_read_callbacks_bio(struct bio *bio)
> > +{
> > + struct read_callbacks_ctx *ctx;
> > + struct page *page;
> > + struct bio_vec *bv;
> > + struct bvec_iter_all iter_all;
> > +
> > + ctx = bio->bi_private;
> > +
> > + bio_for_each_segment_all(bv, bio, iter_all) {
> > + page = bv->bv_page;
> > +
> > + if (bio->bi_status || PageError(page)) {
> > + ClearPageUptodate(page);
> > + SetPageError(page);
> > + } else {
> > + SetPageUptodate(page);
> > + }
> > +
> > + if (ctx->page_op)
> > + ctx->page_op(bio, page);
> > +
> > + unlock_page(page);
> > + }
> > +
> > + put_read_callbacks_ctx(ctx);
> > +
> > + bio_put(bio);
> > +}
>
> The filesystem itself (or fs/mpage.c) actually has to implement almost all this
> logic as well anyway, because CONFIG_FS_READ_CALLBACKS may be unset. And the
> ->page_op() callback, which exists only because f2fs needs to update some
> counter, is very ugly.
>
> IMO, it would be simpler to just make this whole function filesystem-specific,
> as a 'typedef void (*end_bio_func_t)(struct bio *bio);' which gets passed to the
> read_callbacks endio hook.
>
> Of course, each end_bio_func_t would have to check PageError() to check whether
> any read_callbacks step failed. But to make that a bit easier and to make it
> get compiled out when CONFIG_FS_READ_CALLBACKS is unset, there could be a helper
> function in read_callbacks.h:
>
> #ifdef CONFIG_FS_READ_CALLBACKS
> static inline bool read_callbacks_failed(struct page *page)
> {
> return PageError(page);
> }
> #else
> static inline bool read_callbacks_failed(struct page *page)
> {
> return false;
> }
> #endif
>
> > +
> > +/**
> > + * read_callbacks() - Execute the read callbacks state machine.
> > + * @ctx: The context structure tracking the current state.
> > + *
> > + * For each state, this function enqueues a work into appropriate subsystem's
> > + * work queue. After performing further processing of the data in the bio's
> > + * pages, the subsystem should invoke read_calbacks() to continue with the next
> > + * state in the state machine.
> > + */
> > +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, fscrypt_decrypt_work);
> > + fscrypt_enqueue_decrypt_work(&ctx->work);
> > + return;
> > + }
> > + ctx->cur_step++;
> > + /* fall-through */
> > + default:
> > + end_read_callbacks_bio(ctx->bio);
> > + }
> > +}
> > +EXPORT_SYMBOL(read_callbacks);
>
> As I mentioned, I think the work functions should be defined in this file rather
> than in fs/crypto/ or fs/verity/, since they're specific to the read_callbacks.
> fs/crypto/ and fs/verity/ should not be aware of read_callbacks at all.
> Moreover, the 'read_callbacks()' function should be static.
>
> > +
> > +/**
> > + * read_callbacks_end_bio() - Initiate the read callbacks state machine.
> > + * @bio: bio on which read I/O operation has just been completed.
> > + *
> > + * Initiates the execution of the read callbacks state machine when the read
> > + * operation has been completed successfully. If there was an error associated
> > + * with the bio, this function frees the read callbacks context structure stored
> > + * in bio->bi_private (if any).
> > + *
> > + * Return: 1 to indicate that the bio has been handled (including unlocking the
> > + * pages); 0 otherwise.
> > + */
> > +int read_callbacks_end_bio(struct bio *bio)
> > +{
> > + if (!bio->bi_status && bio->bi_private) {
> > + read_callbacks((struct read_callbacks_ctx *)(bio->bi_private));
> > + return 1;
> > + }
> > +
> > + if (bio->bi_private)
> > + put_read_callbacks_ctx((struct read_callbacks_ctx *)(bio->bi_private));
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(read_callbacks_end_bio);
>
> Okay, several issues here...
>
> First, the naming of this is really confusing, since it's actually *beginning*
> the read callbacks, not ending them; and it's basically the same name as
> end_read_callbacks_bio(), which actually *is* for ending the read callbacks.
> Since this is the endio hook, how about calling it read_callbacks_endio_bio(),
> and likewise read_callbacks_endio_bh()?
>
> But more importantly, this doesn't need to have a return value, since the
> read_callbacks layer has to know how to end the bio (meaning unlock the pages
> and mark them uptodate or not) *anyway*, or at least know how to ask the
> filesystem to do it. So it should just do it if needed, rather than returning 0
> and making the caller do it.
>
> Also just assign 'ctx = bio->bi_private' at the start of the function; no need
> to access the field 4 times and have unnecessary casts.
>
> So IMO, the below would be much better:
>
> 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);
>
> And then the !CONFIG_FS_READ_CALLBACKS stub:
>
> static inline void read_callbacks_endio_bio(struct bio *bio,
> end_bio_func_t end_bio)
> {
> end_bio(bio);
> }
>
> Similarly for the buffer_head versions.
>
> > +
> > +/**
> > + * read_callbacks_setup() - 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.
> > + * @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(struct inode *inode, struct bio *bio,
> > + end_page_op_t page_op)
> > +{
> > + 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 -ENOMEM;
> > + ctx->bio = bio;
> > + ctx->inode = inode;
> > + ctx->enabled_steps = enabled_steps;
> > + ctx->cur_step = STEP_INITIAL;
> > + ctx->page_op = page_op;
> > + bio->bi_private = ctx;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(read_callbacks_setup);
>
> Please call it:
>
> read_callbacks_setup_bio()
>
> Then when you add buffer_head support later in the patchset, rather than adding
> a buffer_head argument to this function, add a new function:
>
> read_callbacks_setup_bh()
>
> So that all the users don't have to pass both the buffer_head and bio arguments.
>
> These can use a common function get_read_callbacks_ctx() that creates a
> read_callbacks_ctx for the inode. E.g., the bio version could look like:
>
> 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);
>
>
> Also, a nit: can you move the read_callbacks_setup_*() functions to near the top
> of the file, since they're called first (before the endio functions)? Likewise
> in read_callbacks.h. It would nice to keep the functions in a logical order.
>
> > diff --git a/include/linux/read_callbacks.h b/include/linux/read_callbacks.h
> > new file mode 100644
> > index 000000000000..aa1ec8ed7a6a
> > --- /dev/null
> > +++ b/include/linux/read_callbacks.h
> > @@ -0,0 +1,38 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _READ_CALLBACKS_H
> > +#define _READ_CALLBACKS_H
>
> Headers should be self-contained, but this is missing some prerequisite headers
> and forward declarations. Try compiling a .c file with this header included
> first.
>
> > +
> > +typedef void (*end_page_op_t)(struct bio *bio, struct page *page);
> > +
> > +struct read_callbacks_ctx {
> > + struct bio *bio;
> > + struct inode *inode;
> > + struct work_struct work;
> > + unsigned int cur_step;
> > + unsigned int enabled_steps;
> > + end_page_op_t page_op;
> > +};
>
> As I mentioned, I don't think read_callbacks_ctx should be exposed to
> filesystems like this. Instead just forward declare it here, and put the actual
> definition in fs/read_callbacks.c.
>
> > +
> > +#ifdef CONFIG_FS_READ_CALLBACKS
> > +void read_callbacks(struct read_callbacks_ctx *ctx);
> > +int read_callbacks_end_bio(struct bio *bio);
> > +int read_callbacks_setup(struct inode *inode, struct bio *bio,
> > + end_page_op_t page_op);
> > +#else
> > +static inline void read_callbacks(struct read_callbacks_ctx *ctx)
> > +{
> > +}
> > +
> > +static inline int read_callbacks_end_bio(struct bio *bio)
> > +{
> > + return -EOPNOTSUPP;
> > +}
>
> This stub needs to return 0, otherwise it breaks fs/mpage.c and f2fs for
> everyone when CONFIG_FS_READ_CALLBACKS is unset.
>
> But as I mentioned elsewhere, I think this should actually call an end_bio()
> callback itself and return void, which would also avoid this issue.
>
> > +
> > +static inline int read_callbacks_setup(struct inode *inode,
> > + struct bio *bio, end_page_op_t page_op)
> > +{
> > + return -EOPNOTSUPP;
> > +}
>
> Similarly here, this stub needs to return 0.
>
I agree with all your comments. I will fix them up and post the next version.
--
chandan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 2/7] Integrate read callbacks into Ext4 and F2FS
2019-06-21 21:08 ` Eric Biggers
@ 2019-06-25 6:05 ` Chandan Rajendra
0 siblings, 0 replies; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-25 6:05 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Saturday, June 22, 2019 2:38:01 AM IST Eric Biggers wrote:
> Hi Chandan,
>
> On Sun, Jun 16, 2019 at 09:38:08PM +0530, Chandan Rajendra wrote:
> > This commit gets Ext4 and F2FS to make use of read callbacks API to
> > perform decryption of file data read from the disk.
> > ---
> > fs/crypto/bio.c | 30 +--------
> > fs/crypto/crypto.c | 1 +
> > fs/crypto/fscrypt_private.h | 3 +
> > fs/ext4/readpage.c | 29 +++------
> > fs/f2fs/data.c | 124 +++++++-----------------------------
> > fs/f2fs/super.c | 9 +--
> > fs/read_callbacks.c | 1 -
> > include/linux/fscrypt.h | 18 ------
> > 8 files changed, 40 insertions(+), 175 deletions(-)
> >
>
> This patch changes many different components. It would be much easier to
> review, and might get more attention from the other ext4 and f2fs developers, if
> it were split into 3 patches:
>
> a. Convert ext4 to use read_callbacks.
> b. Convert f2fs to use read_callbacks.
> c. Remove the functions from fs/crypto/ that became unused as a result of
> patches (a) and (b). (Actually, this part probably should be merged with the
> patch that removes the fscrypt_ctx, and the patch renamed to something like
> "fscrypt: remove decryption I/O path helpers")
>
> Any reason why this wouldn't work? AFAICS, you couldn't do it only because you
> made this patch change fscrypt_enqueue_decrypt_work() to be responsible for
> initializing the work function. But as per my comments on patch 1, I don't
> think we should do that, since it would make much more sense to put the work
> function in read_callbacks.c.
Yes, you are right about that. I will make the changes suggested by you.
>
> However, since you're converting ext4 to use mpage_readpages() anyway, I don't
> think we should bother with the intermediate change to ext4_mpage_readpages().
> It's useless, and that intermediate state of the ext4 code inevitably won't get
> tested very well. So perhaps order the whole series as:
>
> - fs: introduce read_callbacks
> - fs/mpage.c: add decryption support via read_callbacks
> - fs/buffer.c: add decryption support via read_callbacks
> - f2fs: convert to use read_callbacks
> - ext4: convert to use mpage_readpages[s]
> - ext4: support encryption with subpage-sized blocks
> - fscrypt: remove decryption I/O path helpers
>
> That order would also give the flexibility to possibly apply the fs/ changes
> first, without having to update both ext4 and f2fs simultaneously with them.
>
> > @@ -557,8 +511,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
> > {
> > struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> > struct bio *bio;
> > - struct bio_post_read_ctx *ctx;
> > - unsigned int post_read_steps = 0;
> > + int ret;
>
> Nit: 'err' rather than 'ret', since this is 0 or a -errno value.
>
> > -int __init f2fs_init_post_read_processing(void)
> > -{
> > - bio_post_read_ctx_cache = KMEM_CACHE(bio_post_read_ctx, 0);
> > - if (!bio_post_read_ctx_cache)
> > - goto fail;
> > - bio_post_read_ctx_pool =
> > - mempool_create_slab_pool(NUM_PREALLOC_POST_READ_CTXS,
> > - bio_post_read_ctx_cache);
> > - if (!bio_post_read_ctx_pool)
> > - goto fail_free_cache;
> > - return 0;
> > -
> > -fail_free_cache:
> > - kmem_cache_destroy(bio_post_read_ctx_cache);
> > -fail:
> > - return -ENOMEM;
> > -}
> > -
> > -void __exit f2fs_destroy_post_read_processing(void)
> > -{
> > - mempool_destroy(bio_post_read_ctx_pool);
> > - kmem_cache_destroy(bio_post_read_ctx_cache);
> > -}
>
> Need to remove the declarations of these functions from fs/f2fs/f2fs.h to.
>
> > diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
> > index a4196e3de05f..4b7fc2a349cd 100644
> > --- a/fs/read_callbacks.c
> > +++ b/fs/read_callbacks.c
> > @@ -76,7 +76,6 @@ void read_callbacks(struct read_callbacks_ctx *ctx)
> > switch (++ctx->cur_step) {
> > case STEP_DECRYPT:
> > if (ctx->enabled_steps & (1 << STEP_DECRYPT)) {
> > - INIT_WORK(&ctx->work, fscrypt_decrypt_work);
> > fscrypt_enqueue_decrypt_work(&ctx->work);
> > return;
> > }
>
> Again, I think the work initialization should remain here as:
>
> INIT_WORK(&ctx->work, decrypt_work);
>
> rather than moving it to fs/crypto/.
>
> Thanks!
>
> - Eric
>
--
chandan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 4/7] fs/mpage.c: Integrate read callbacks
2019-06-21 21:14 ` Eric Biggers
@ 2019-06-25 6:21 ` Chandan Rajendra
0 siblings, 0 replies; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-25 6:21 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Saturday, June 22, 2019 2:44:55 AM IST Eric Biggers wrote:
> On Sun, Jun 16, 2019 at 09:38:10PM +0530, Chandan Rajendra wrote:
> > This commit adds code to make do_mpage_readpage() to be "read callbacks"
> > aware i.e. for files requiring decryption, do_mpage_readpage() now
> > sets up the read callbacks state machine when allocating a bio and later
> > starts execution of the state machine after file data is read from the
> > underlying disk.
> >
> > Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
> > ---
> > fs/mpage.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/mpage.c b/fs/mpage.c
> > index 436a85260394..611ad122fc92 100644
> > --- a/fs/mpage.c
> > +++ b/fs/mpage.c
> > @@ -30,6 +30,7 @@
> > #include <linux/backing-dev.h>
> > #include <linux/pagevec.h>
> > #include <linux/cleancache.h>
> > +#include <linux/read_callbacks.h>
> > #include "internal.h"
> >
> > /*
> > @@ -49,6 +50,8 @@ static void mpage_end_io(struct bio *bio)
> > struct bio_vec *bv;
> > struct bvec_iter_all iter_all;
> >
> > + if (read_callbacks_end_bio(bio))
> > + return;
> > bio_for_each_segment_all(bv, bio, iter_all) {
> > struct page *page = bv->bv_page;
> > page_endio(page, bio_op(bio),
> > @@ -309,6 +312,12 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
> > gfp);
> > if (args->bio == NULL)
> > goto confused;
> > +
> > + if (read_callbacks_setup(inode, args->bio, NULL)) {
> > + bio_put(args->bio);
> > + args->bio = NULL;
> > + goto confused;
> > + }
> > }
> >
> > length = first_hole << blkbits;
> > @@ -330,7 +339,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
> > confused:
> > if (args->bio)
> > args->bio = mpage_bio_submit(REQ_OP_READ, op_flags, args->bio);
> > - if (!PageUptodate(page))
> > + if (!PageUptodate(page) && !PageError(page))
> > block_read_full_page(page, args->get_block);
> > else
> > unlock_page(page);
>
> Why is the !PageError() check needed here?
>
Sorry, Its a remnant from when I was debugging the patchset. I will remove
this.
--
chandan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 6/7] Add decryption support for sub-pagesized blocks
2019-06-21 21:29 ` Eric Biggers
@ 2019-06-25 6:22 ` Chandan Rajendra
0 siblings, 0 replies; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-25 6:22 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Saturday, June 22, 2019 2:59:17 AM IST Eric Biggers wrote:
> On Sun, Jun 16, 2019 at 09:38:12PM +0530, Chandan Rajendra wrote:
> > To support decryption of sub-pagesized blocks this commit adds code to,
> > 1. Track buffer head in "struct read_callbacks_ctx".
> > 2. Pass buffer head argument to all read callbacks.
> > 3. Add new fscrypt helper to decrypt the file data referred to by a
> > buffer head.
> >
> > Signed-off-by: Chandan Rajendra <chandan@linux.ibm.com>
> > ---
> > fs/buffer.c | 55 +++++++++------
> > fs/crypto/bio.c | 21 +++++-
> > fs/f2fs/data.c | 2 +-
> > fs/mpage.c | 2 +-
> > fs/read_callbacks.c | 118 +++++++++++++++++++++++++--------
> > include/linux/buffer_head.h | 1 +
> > include/linux/read_callbacks.h | 13 +++-
> > 7 files changed, 158 insertions(+), 54 deletions(-)
> >
>
> This is another patch that unnecessarily changes way too many components at
> once. My suggestions elsewhere would resolve this, though:
>
> - This patch changes fs/f2fs/data.c and fs/mpage.c only to pass a NULL
> buffer_head to read_callbacks_setup(). But as per my comments on patch 1,
> read_callbacks_setup() should be split into read_callbacks_setup_bio() and
> read_callbacks_end_bh().
>
> - This patch changes fs/crypto/ only to add support for the buffer_head
> decryption work. But as per my comments on patch 1, that should be in
> read_callbacks.c instead.
>
> And adding buffer_head support to fs/read_callbacks.c should be its own patch,
> *or* should simply be folded into the patch that adds fs/read_callbacks.c.
>
> Then the only thing remaining in this patch would be updating fs/buffer.c to
> make it use the read_callbacks, which should be retitled to something like
> "fs/buffer.c: add decryption support via read_callbacks".
>
I agree.
--
chandan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 0/7] Consolidate FS read I/O callbacks code
2019-06-21 22:15 ` [PATCH V3 0/7] Consolidate FS read I/O callbacks code Eric Biggers
@ 2019-06-25 6:24 ` Chandan Rajendra
0 siblings, 0 replies; 19+ messages in thread
From: Chandan Rajendra @ 2019-06-25 6:24 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-fscrypt,
tytso, adilger.kernel, jaegeuk, yuchao0, hch
On Saturday, June 22, 2019 3:45:51 AM IST Eric Biggers wrote:
> On Sun, Jun 16, 2019 at 09:38:06PM +0530, 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-v3.
> >
>
> FWIW: while doing my review I put together an (untested) incremental patch that
> addresses my comments on the code, so I've provided it below in case you want to
> start with it when addressing my comments.
>
> This is just a single diff against your subpage-encryption-v3 branch, so of
> course it would still need to be folded into the appropriate patches. Also see
> my suggestions in reply to patch 2 about how to better organize the series. I
> also left TODOs in kerneldoc comments that still need to be updated.
>
Thanks for all your help. I will post the next version of the patchset
addressing all your review comments.
--
chandan
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-06-25 6:23 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-16 16:08 [PATCH V3 0/7] Consolidate FS read I/O callbacks code Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 1/7] FS: Introduce read callbacks Chandan Rajendra
2019-06-21 20:03 ` Eric Biggers
2019-06-25 4:59 ` Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 2/7] Integrate read callbacks into Ext4 and F2FS Chandan Rajendra
2019-06-21 21:08 ` Eric Biggers
2019-06-25 6:05 ` Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 3/7] fscrypt: remove struct fscrypt_ctx Chandan Rajendra
2019-06-21 22:00 ` Eric Biggers
2019-06-16 16:08 ` [PATCH V3 4/7] fs/mpage.c: Integrate read callbacks Chandan Rajendra
2019-06-21 21:14 ` Eric Biggers
2019-06-25 6:21 ` Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 5/7] ext4: Wire up ext4_readpage[s] to use mpage_readpage[s] Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 6/7] Add decryption support for sub-pagesized blocks Chandan Rajendra
2019-06-21 21:29 ` Eric Biggers
2019-06-25 6:22 ` Chandan Rajendra
2019-06-16 16:08 ` [PATCH V3 7/7] ext4: Enable encryption for subpage-sized blocks Chandan Rajendra
2019-06-21 22:15 ` [PATCH V3 0/7] Consolidate FS read I/O callbacks code Eric Biggers
2019-06-25 6:24 ` Chandan Rajendra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).