Consolidate Post read processing code

The post read processing code is used by both Ext4 and F2FS. Hence to remove duplicity, this commit moves the code into include/linux/post_read_process.h and fs/post_read_process.c.

The corresponding decrypt and verity "work" functions have been moved inside fscrypt and fsverity sources. With these in place, the post processing code now has to just invoke enqueue functions provided by fscrypt and fsverity.

Signed-off-by Chandan Rajendra <[email protected]>

---
 fs/Makefile                       |   3 +-
 fs/crypto/bio.c                   |  21 ++--
 fs/crypto/crypto.c                |   1 +
 fs/crypto/fscrypt_private.h       |   3 +
 fs/ext4/ext4.h                    |   2 -
 fs/ext4/readpage.c                | 153 +-----------------------------
 fs/ext4/super.c                   |   9 +-
 fs/post_read_process.c            | 127 +++++++++++++++++++++++++
 fs/verity/verify.c                |  12 +++
 include/linux/fscrypt.h           |  11 ---
 include/linux/post_read_process.h |  21 ++++
 11 files changed, 176 insertions(+), 187 deletions(-)
 create mode 100644 fs/post_read_process.c
 create mode 100644 include/linux/post_read_process.h

fs/Makefile +0 -0

index 10b37f651ffd..5f6c0cba102b 100644

fs/Makefile +0 -0

 ifeq ($(CONFIG_BLOCK),y)
 obj-y +=	buffer.o block_dev.o direct-io.o mpage.o

fs/crypto/bio.c +7 -14

index 0959044c5cee..a659a76c05e4 100644
@@ -24,6 +24,8 @@
 #include <linux/module.h>
 #include <linux/bio.h>
 #include <linux/namei.h>
+#include <linux/post_read_process.h>
+
 #include "fscrypt_private.h"
 
 static void __fscrypt_decrypt_bio(struct bio *bio, bool done)

@@ -53,24 +55,15 @@ void fscrypt_decrypt_bio(struct bio *bio)
 }
 EXPORT_SYMBOL(fscrypt_decrypt_bio);
 
-static void completion_pages(struct work_struct *work)
+void fscrypt_decrypt_work(struct work_struct *work)
 {
-	struct fscrypt_ctx *ctx =
-		container_of(work, struct fscrypt_ctx, r.work);
-	struct bio *bio = ctx->r.bio;
+	struct bio_post_read_ctx *ctx =
+		container_of(work, struct bio_post_read_ctx, work);
 
-	__fscrypt_decrypt_bio(bio, true);
-	fscrypt_release_ctx(ctx);
-	bio_put(bio);
-}
+	fscrypt_decrypt_bio(ctx->bio);
 
-void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
-{
-	INIT_WORK(&ctx->r.work, completion_pages);
-	ctx->r.bio = bio;
-	fscrypt_enqueue_decrypt_work(&ctx->r.work);
+	bio_post_read_processing(ctx);
 }
-EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
 
 void fscrypt_pullback_bio_page(struct page **page, bool restore)
 {

fs/crypto/crypto.c +1 -0

index 4dc788e3bc96..36d599784e5a 100644
@@ -53,6 +53,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);

fs/crypto/fscrypt_private.h +3 -0

index 7da276159593..412a3bcf9efd 100644
@@ -114,6 +114,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);

fs/ext4/ext4.h +0 -2

index 0ffa84772667..c0245820bafe 100644
@@ -3088,8 +3088,6 @@ static inline void ext4_set_de_type(struct super_block *sb,
 extern int ext4_mpage_readpages(struct address_space *mapping,
 				struct list_head *pages, struct page *page,
 				unsigned nr_pages, bool is_readahead);
-extern int __init ext4_init_post_read_processing(void);
-extern void ext4_exit_post_read_processing(void);
 
 /* symlink.c */
 extern const struct inode_operations ext4_encrypted_symlink_inode_operations;

fs/ext4/readpage.c +2 -151

index 93fbc15177a3..8943fc41fd33 100644
@@ -44,14 +44,10 @@
 #include <linux/backing-dev.h>
 #include <linux/pagevec.h>
 #include <linux/cleancache.h>
+#include <linux/post_read_process.h>
 
 #include "ext4.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 inline bool ext4_bio_encrypted(struct bio *bio)
 {
 #ifdef CONFIG_FS_ENCRYPTION

@@ -61,124 +57,6 @@ static inline bool ext4_bio_encrypted(struct bio *bio)
 #endif
 }
 
-/* postprocessing steps for read bios */
-enum bio_post_read_step {
-	STEP_INITIAL = 0,
-	STEP_DECRYPT,
-	STEP_VERITY,
-};
-
-struct bio_post_read_ctx {
-	struct bio *bio;
-	struct work_struct work;
-	unsigned int cur_step;
-	unsigned int enabled_steps;
-};
-
-static void __read_end_io(struct bio *bio)
-{
-	struct page *page;
-	struct bio_vec *bv;
-	int i;
-
-	bio_for_each_segment_all(bv, bio, i) {
-		page = bv->bv_page;
-
-		/* PG_error was set if any post_read step failed */
-		if (bio->bi_status || PageError(page)) {
-			ClearPageUptodate(page);
-			SetPageError(page);
-		} else {
-			SetPageUptodate(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);
-}
-
-static void verity_work(struct work_struct *work)
-{
-	struct bio_post_read_ctx *ctx =
-		container_of(work, struct bio_post_read_ctx, work);
-
-	fsverity_verify_bio(ctx->bio);
-
-	bio_post_read_processing(ctx);
-}
-
-static void bio_post_read_processing(struct bio_post_read_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 */
-	case STEP_VERITY:
-		if (ctx->enabled_steps & (1 << STEP_VERITY)) {
-			INIT_WORK(&ctx->work, verity_work);
-			fsverity_enqueue_verify_work(&ctx->work);
-			return;
-		}
-		ctx->cur_step++;
-		/* fall-through */
-	default:
-		__read_end_io(ctx->bio);
-	}
-}
-
-static struct bio_post_read_ctx *get_bio_post_read_ctx(struct inode *inode,
-						       struct bio *bio,
-						       pgoff_t index)
-{
-	unsigned int post_read_steps = 0;
-	struct bio_post_read_ctx *ctx = NULL;
-
-	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
-		post_read_steps |= 1 << STEP_DECRYPT;
-#ifdef CONFIG_FS_VERITY
-	if (inode->i_verity_info != NULL &&
-	    (index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT)))
-		post_read_steps |= 1 << STEP_VERITY;
-#endif
-	if (post_read_steps) {
-		ctx = mempool_alloc(bio_post_read_ctx_pool, GFP_NOFS);
-		if (!ctx)
-			return ERR_PTR(-ENOMEM);
-		ctx->bio = bio;
-		ctx->enabled_steps = post_read_steps;
-		bio->bi_private = ctx;
-	}
-	return ctx;
-}
-
-static bool bio_post_read_required(struct bio *bio)
-{
-	return bio->bi_private && !bio->bi_status;
-}
-
 /*
  * I/O completion handler for multipage BIOs.
  *

@@ -196,11 +74,10 @@ static void mpage_end_io(struct bio *bio)
 	if (bio_post_read_required(bio)) {
 		struct bio_post_read_ctx *ctx = bio->bi_private;
 
-		ctx->cur_step = STEP_INITIAL;
 		bio_post_read_processing(ctx);
 		return;
 	}
-	__read_end_io(bio);
+	end_bio_post_read_processing(bio);
 }
 
 static inline loff_t ext4_readpage_limit(struct inode *inode)

@@ -416,29 +293,3 @@ int ext4_mpage_readpages(struct address_space *mapping,
 		submit_bio(bio);
 	return 0;
 }
-
-int __init ext4_init_post_read_processing(void)
-{
-	bio_post_read_ctx_cache =
-		kmem_cache_create("ext4_bio_post_read_ctx",
-				  sizeof(struct bio_post_read_ctx), 0, 0, NULL);
-	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 ext4_exit_post_read_processing(void)
-{
-	mempool_destroy(bio_post_read_ctx_pool);
-	kmem_cache_destroy(bio_post_read_ctx_cache);
-}

fs/ext4/super.c +1 -8

index 95a5d9fbbb9f..9314dddfbf34 100644
@@ -6102,10 +6102,6 @@ static int __init ext4_init_fs(void)
 		return err;
 
 	err = ext4_init_pending();
-	if (err)
-		goto out7;
-
-	err = ext4_init_post_read_processing();
 	if (err)
 		goto out6;
 

@@ -6147,10 +6143,8 @@ static int __init ext4_init_fs(void)
 out4:
 	ext4_exit_pageio();
 out5:
-	ext4_exit_post_read_processing();
-out6:
 	ext4_exit_pending();
-out7:
+out6:
 	ext4_exit_es();
 
 	return err;

@@ -6167,7 +6161,6 @@ static void __exit ext4_exit_fs(void)
 	ext4_exit_sysfs();
 	ext4_exit_system_zone();
 	ext4_exit_pageio();
-	ext4_exit_post_read_processing();
 	ext4_exit_es();
 	ext4_exit_pending();
 }

fs/post_read_process.c new file +0 -0

new file mode 100644
index 000000000000..9720eeff0160

fs/post_read_process.c new file +119 -0

+
+#define NUM_PREALLOC_POST_READ_CTXS	128
+
+static struct kmem_cache *bio_post_read_ctx_cache;
+static mempool_t *bio_post_read_ctx_pool;
+
+/* postprocessing steps for read bios */
+enum bio_post_read_step {
+	STEP_INITIAL = 0,
+	STEP_DECRYPT,
+	STEP_VERITY,
+};
+
+void end_bio_post_read_processing(struct bio *bio)
+{
+	struct page *page;
+	struct bio_vec *bv;
+	int i;
+
+	bio_for_each_segment_all(bv, bio, i) {
+		page = bv->bv_page;
+
+		/* PG_error was set if any post_read step failed */
+		if (bio->bi_status || PageError(page)) {
+			ClearPageUptodate(page);
+			SetPageError(page);
+		} else {
+			SetPageUptodate(page);
+		}
+		unlock_page(page);
+	}
+	if (bio->bi_private)
+		put_bio_post_read_ctx(bio->bi_private);
+	bio_put(bio);
+}
+
+void bio_post_read_processing(struct bio_post_read_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 */
+	case STEP_VERITY:
+		if (ctx->enabled_steps & (1 << STEP_VERITY)) {
+			fsverity_enqueue_verify_work(&ctx->work);
+			return;
+		}
+		ctx->cur_step++;
+		/* fall-through */
+	default:
+		end_bio_post_read_processing(ctx->bio);
+	}
+}
+
+struct bio_post_read_ctx *get_bio_post_read_ctx(struct inode *inode,
+						struct bio *bio,
+						pgoff_t index)
+{
+	unsigned int post_read_steps = 0;
+	struct bio_post_read_ctx *ctx = NULL;
+
+	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
+		post_read_steps |= 1 << STEP_DECRYPT;
+#ifdef CONFIG_FS_VERITY
+	if (inode->i_verity_info != NULL &&
+		(index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT)))
+		post_read_steps |= 1 << STEP_VERITY;
+#endif
+	if (post_read_steps) {
+		ctx = mempool_alloc(bio_post_read_ctx_pool, GFP_NOFS);
+		if (!ctx)
+			return ERR_PTR(-ENOMEM);
+		ctx->bio = bio;
+		ctx->inode = inode;
+		ctx->enabled_steps = post_read_steps;
+		ctx->cur_step = STEP_INITIAL;
+		bio->bi_private = ctx;
+	}
+	return ctx;
+}
+
+void put_bio_post_read_ctx(struct bio_post_read_ctx *ctx)
+{
+	mempool_free(ctx, bio_post_read_ctx_pool);
+}
+
+bool bio_post_read_required(struct bio *bio)
+{
+	return bio->bi_private && !bio->bi_status;
+}
+
+static int __init bio_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;
+}
+
+fs_initcall(bio_init_post_read_processing);

fs/verity/verify.c +12 -0

index fbfb68eff358..4f7cd2269e83 100644
@@ -13,6 +13,7 @@
 #include <linux/pagemap.h>
 #include <linux/ratelimit.h>
 #include <linux/scatterlist.h>
+#include <linux/post_read_process.h>
 
 struct workqueue_struct *fsverity_read_workqueue;
 

@@ -283,6 +284,16 @@ void fsverity_verify_bio(struct bio *bio)
 EXPORT_SYMBOL_GPL(fsverity_verify_bio);
 #endif /* CONFIG_BLOCK */
 
+static void fsverity_verify_work(struct work_struct *work)
+{
+	struct bio_post_read_ctx *ctx =
+		container_of(work, struct bio_post_read_ctx, work);
+
+	fsverity_verify_bio(ctx->bio);
+
+	bio_post_read_processing(ctx);
+}
+
 /**
  * fsverity_enqueue_verify_work - enqueue work on the fs-verity workqueue
  *

@@ -290,6 +301,7 @@ EXPORT_SYMBOL_GPL(fsverity_verify_bio);
  */
 void fsverity_enqueue_verify_work(struct work_struct *work)
 {
+	INIT_WORK(work, fsverity_verify_work);
 	queue_work(fsverity_read_workqueue, work);
 }
 EXPORT_SYMBOL_GPL(fsverity_enqueue_verify_work);

include/linux/fscrypt.h +0 -0

index 6ba193c23f37..13f70e22aff2 100644

include/linux/fscrypt.h +0 -7

 		struct list_head free_list;	/* Free list */
 	};
 	u8 flags;				/* Flags */

@@ -206,8 +202,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_pullback_bio_page(struct page **, bool);
 extern int fscrypt_zeroout_range(const struct inode *, pgoff_t, sector_t,
 				 unsigned int);

@@ -376,11 +370,6 @@ static inline void fscrypt_decrypt_bio(struct bio *bio)
 {
 }
 
-static inline void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx,
-					       struct bio *bio)
-{
-}
-
 static inline void fscrypt_pullback_bio_page(struct page **page, bool restore)
 {
 	return;

include/linux/post_read_process.h new file +21 -0

new file mode 100644
index 000000000000..09e52928f861
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _POST_READ_PROCESS_H
+#define _POST_READ_PROCESS_H
+
+struct bio_post_read_ctx {
+	struct bio *bio;
+	struct inode *inode;
+	struct work_struct work;
+	unsigned int cur_step;
+	unsigned int enabled_steps;
+};
+
+void end_bio_post_read_processing(struct bio *bio);
+void bio_post_read_processing(struct bio_post_read_ctx *ctx);
+struct bio_post_read_ctx *get_bio_post_read_ctx(struct inode *inode,
+						struct bio *bio,
+						pgoff_t index);
+void put_bio_post_read_ctx(struct bio_post_read_ctx *ctx);
+bool bio_post_read_required(struct bio *bio);
+
+#endif	/* _POST_READ_PROCESS_H */

Signature · 1 line

2.19.1

Hi Chandan,

Quoted 26 lines · Chandan Rajendra

The post read processing code is used by both Ext4 and F2FS. Hence to remove duplicity, this commit moves the code into include/linux/post_read_process.h and fs/post_read_process.c.

The corresponding decrypt and verity "work" functions have been moved inside fscrypt and fsverity sources. With these in place, the post processing code now has to just invoke enqueue functions provided by fscrypt and fsverity.

Signed-off-by Chandan Rajendra <[email protected]>

---
 fs/Makefile                       |   3 +-
 fs/crypto/bio.c                   |  21 ++--
 fs/crypto/crypto.c                |   1 +
 fs/crypto/fscrypt_private.h       |   3 +
 fs/ext4/ext4.h                    |   2 -
 fs/ext4/readpage.c                | 153 +-----------------------------
 fs/ext4/super.c                   |   9 +-
 fs/post_read_process.c            | 127 +++++++++++++++++++++++++
 fs/verity/verify.c                |  12 +++
 include/linux/fscrypt.h           |  11 ---
 include/linux/post_read_process.h |  21 ++++
 11 files changed, 176 insertions(+), 187 deletions(-)
 create mode 100644 fs/post_read_process.c
 create mode 100644 include/linux/post_read_process.h

fs/Makefile obj-y := open.o read_write.o file_table.o super.o \ · line 17

Quoted 12 lines

fs/Makefile +2 -1

index 10b37f651ffd..5f6c0cba102b 100644
@@ -12,7 +12,8 @@ obj-y :=	open.o read_write.o file_table.o super.o \
 		attr.o bad_inode.o file.o filesystems.o namespace.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o splice.o sync.o utimes.o d_path.o \
-		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o
+		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o \
+		post_read_process.o
 

To avoid bloating every Linux kernel in existence, post_read_process.c should only be compiled if CONFIG_FS_ENCRYPTION || CONFIG_FS_VERITY.

Quoted 316 lines

fs/Makefile +0 -0

 ifeq ($(CONFIG_BLOCK),y)
 obj-y +=	buffer.o block_dev.o direct-io.o mpage.o

fs/crypto/bio.c +7 -14

index 0959044c5cee..a659a76c05e4 100644
@@ -24,6 +24,8 @@
 #include <linux/module.h>
 #include <linux/bio.h>
 #include <linux/namei.h>
+#include <linux/post_read_process.h>
+
 #include "fscrypt_private.h"
 
 static void __fscrypt_decrypt_bio(struct bio *bio, bool done)

@@ -53,24 +55,15 @@ void fscrypt_decrypt_bio(struct bio *bio)
 }
 EXPORT_SYMBOL(fscrypt_decrypt_bio);
 
-static void completion_pages(struct work_struct *work)
+void fscrypt_decrypt_work(struct work_struct *work)
 {
-	struct fscrypt_ctx *ctx =
-		container_of(work, struct fscrypt_ctx, r.work);
-	struct bio *bio = ctx->r.bio;
+	struct bio_post_read_ctx *ctx =
+		container_of(work, struct bio_post_read_ctx, work);
 
-	__fscrypt_decrypt_bio(bio, true);
-	fscrypt_release_ctx(ctx);
-	bio_put(bio);
-}
+	fscrypt_decrypt_bio(ctx->bio);
 
-void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx, struct bio *bio)
-{
-	INIT_WORK(&ctx->r.work, completion_pages);
-	ctx->r.bio = bio;
-	fscrypt_enqueue_decrypt_work(&ctx->r.work);
+	bio_post_read_processing(ctx);
 }
-EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
 
 void fscrypt_pullback_bio_page(struct page **page, bool restore)
 {

fs/crypto/crypto.c +1 -0

index 4dc788e3bc96..36d599784e5a 100644
@@ -53,6 +53,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);

fs/crypto/fscrypt_private.h +3 -0

index 7da276159593..412a3bcf9efd 100644
@@ -114,6 +114,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);

fs/ext4/ext4.h +0 -2

index 0ffa84772667..c0245820bafe 100644
@@ -3088,8 +3088,6 @@ static inline void ext4_set_de_type(struct super_block *sb,
 extern int ext4_mpage_readpages(struct address_space *mapping,
 				struct list_head *pages, struct page *page,
 				unsigned nr_pages, bool is_readahead);
-extern int __init ext4_init_post_read_processing(void);
-extern void ext4_exit_post_read_processing(void);
 
 /* symlink.c */
 extern const struct inode_operations ext4_encrypted_symlink_inode_operations;

fs/ext4/readpage.c +2 -151

index 93fbc15177a3..8943fc41fd33 100644
@@ -44,14 +44,10 @@
 #include <linux/backing-dev.h>
 #include <linux/pagevec.h>
 #include <linux/cleancache.h>
+#include <linux/post_read_process.h>
 
 #include "ext4.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 inline bool ext4_bio_encrypted(struct bio *bio)
 {
 #ifdef CONFIG_FS_ENCRYPTION

@@ -61,124 +57,6 @@ static inline bool ext4_bio_encrypted(struct bio *bio)
 #endif
 }
 
-/* postprocessing steps for read bios */
-enum bio_post_read_step {
-	STEP_INITIAL = 0,
-	STEP_DECRYPT,
-	STEP_VERITY,
-};
-
-struct bio_post_read_ctx {
-	struct bio *bio;
-	struct work_struct work;
-	unsigned int cur_step;
-	unsigned int enabled_steps;
-};
-
-static void __read_end_io(struct bio *bio)
-{
-	struct page *page;
-	struct bio_vec *bv;
-	int i;
-
-	bio_for_each_segment_all(bv, bio, i) {
-		page = bv->bv_page;
-
-		/* PG_error was set if any post_read step failed */
-		if (bio->bi_status || PageError(page)) {
-			ClearPageUptodate(page);
-			SetPageError(page);
-		} else {
-			SetPageUptodate(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);
-}
-
-static void verity_work(struct work_struct *work)
-{
-	struct bio_post_read_ctx *ctx =
-		container_of(work, struct bio_post_read_ctx, work);
-
-	fsverity_verify_bio(ctx->bio);
-
-	bio_post_read_processing(ctx);
-}
-
-static void bio_post_read_processing(struct bio_post_read_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 */
-	case STEP_VERITY:
-		if (ctx->enabled_steps & (1 << STEP_VERITY)) {
-			INIT_WORK(&ctx->work, verity_work);
-			fsverity_enqueue_verify_work(&ctx->work);
-			return;
-		}
-		ctx->cur_step++;
-		/* fall-through */
-	default:
-		__read_end_io(ctx->bio);
-	}
-}
-
-static struct bio_post_read_ctx *get_bio_post_read_ctx(struct inode *inode,
-						       struct bio *bio,
-						       pgoff_t index)
-{
-	unsigned int post_read_steps = 0;
-	struct bio_post_read_ctx *ctx = NULL;
-
-	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
-		post_read_steps |= 1 << STEP_DECRYPT;
-#ifdef CONFIG_FS_VERITY
-	if (inode->i_verity_info != NULL &&
-	    (index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT)))
-		post_read_steps |= 1 << STEP_VERITY;
-#endif
-	if (post_read_steps) {
-		ctx = mempool_alloc(bio_post_read_ctx_pool, GFP_NOFS);
-		if (!ctx)
-			return ERR_PTR(-ENOMEM);
-		ctx->bio = bio;
-		ctx->enabled_steps = post_read_steps;
-		bio->bi_private = ctx;
-	}
-	return ctx;
-}
-
-static bool bio_post_read_required(struct bio *bio)
-{
-	return bio->bi_private && !bio->bi_status;
-}
-
 /*
  * I/O completion handler for multipage BIOs.
  *

@@ -196,11 +74,10 @@ static void mpage_end_io(struct bio *bio)
 	if (bio_post_read_required(bio)) {
 		struct bio_post_read_ctx *ctx = bio->bi_private;
 
-		ctx->cur_step = STEP_INITIAL;
 		bio_post_read_processing(ctx);
 		return;
 	}
-	__read_end_io(bio);
+	end_bio_post_read_processing(bio);
 }
 
 static inline loff_t ext4_readpage_limit(struct inode *inode)

@@ -416,29 +293,3 @@ int ext4_mpage_readpages(struct address_space *mapping,
 		submit_bio(bio);
 	return 0;
 }
-
-int __init ext4_init_post_read_processing(void)
-{
-	bio_post_read_ctx_cache =
-		kmem_cache_create("ext4_bio_post_read_ctx",
-				  sizeof(struct bio_post_read_ctx), 0, 0, NULL);
-	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 ext4_exit_post_read_processing(void)
-{
-	mempool_destroy(bio_post_read_ctx_pool);
-	kmem_cache_destroy(bio_post_read_ctx_cache);
-}

fs/ext4/super.c +1 -8

index 95a5d9fbbb9f..9314dddfbf34 100644
@@ -6102,10 +6102,6 @@ static int __init ext4_init_fs(void)
 		return err;
 
 	err = ext4_init_pending();
-	if (err)
-		goto out7;
-
-	err = ext4_init_post_read_processing();
 	if (err)
 		goto out6;
 

@@ -6147,10 +6143,8 @@ static int __init ext4_init_fs(void)
 out4:
 	ext4_exit_pageio();
 out5:
-	ext4_exit_post_read_processing();
-out6:
 	ext4_exit_pending();
-out7:
+out6:
 	ext4_exit_es();
 
 	return err;

@@ -6167,7 +6161,6 @@ static void __exit ext4_exit_fs(void)
 	ext4_exit_sysfs();
 	ext4_exit_system_zone();
 	ext4_exit_pageio();
-	ext4_exit_post_read_processing();
 	ext4_exit_es();
 	ext4_exit_pending();
 }

fs/post_read_process.c new file +3 -0

new file mode 100644
index 000000000000..9720eeff0160
@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/module.h>
+#include <linux/mm.h>

fs/post_read_process.c line 8

Quoted 5 lines

fs/post_read_process.c new file +5 -0

+#include <linux/pagemap.h>
+#include <linux/bio.h>
+#include <linux/fscrypt.h>
+#include <linux/fsverity.h>
+#include <linux/post_read_process.h>

To help people who come across this code, please add a comment at the top of this file that briefly describes what it is.

Quoted 163 lines

fs/post_read_process.c new file +119 -0

+
+#define NUM_PREALLOC_POST_READ_CTXS	128
+
+static struct kmem_cache *bio_post_read_ctx_cache;
+static mempool_t *bio_post_read_ctx_pool;
+
+/* postprocessing steps for read bios */
+enum bio_post_read_step {
+	STEP_INITIAL = 0,
+	STEP_DECRYPT,
+	STEP_VERITY,
+};
+
+void end_bio_post_read_processing(struct bio *bio)
+{
+	struct page *page;
+	struct bio_vec *bv;
+	int i;
+
+	bio_for_each_segment_all(bv, bio, i) {
+		page = bv->bv_page;
+
+		/* PG_error was set if any post_read step failed */
+		if (bio->bi_status || PageError(page)) {
+			ClearPageUptodate(page);
+			SetPageError(page);
+		} else {
+			SetPageUptodate(page);
+		}
+		unlock_page(page);
+	}
+	if (bio->bi_private)
+		put_bio_post_read_ctx(bio->bi_private);
+	bio_put(bio);
+}
+
+void bio_post_read_processing(struct bio_post_read_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 */
+	case STEP_VERITY:
+		if (ctx->enabled_steps & (1 << STEP_VERITY)) {
+			fsverity_enqueue_verify_work(&ctx->work);
+			return;
+		}
+		ctx->cur_step++;
+		/* fall-through */
+	default:
+		end_bio_post_read_processing(ctx->bio);
+	}
+}
+
+struct bio_post_read_ctx *get_bio_post_read_ctx(struct inode *inode,
+						struct bio *bio,
+						pgoff_t index)
+{
+	unsigned int post_read_steps = 0;
+	struct bio_post_read_ctx *ctx = NULL;
+
+	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
+		post_read_steps |= 1 << STEP_DECRYPT;
+#ifdef CONFIG_FS_VERITY
+	if (inode->i_verity_info != NULL &&
+		(index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT)))
+		post_read_steps |= 1 << STEP_VERITY;
+#endif
+	if (post_read_steps) {
+		ctx = mempool_alloc(bio_post_read_ctx_pool, GFP_NOFS);
+		if (!ctx)
+			return ERR_PTR(-ENOMEM);
+		ctx->bio = bio;
+		ctx->inode = inode;
+		ctx->enabled_steps = post_read_steps;
+		ctx->cur_step = STEP_INITIAL;
+		bio->bi_private = ctx;
+	}
+	return ctx;
+}
+
+void put_bio_post_read_ctx(struct bio_post_read_ctx *ctx)
+{
+	mempool_free(ctx, bio_post_read_ctx_pool);
+}
+
+bool bio_post_read_required(struct bio *bio)
+{
+	return bio->bi_private && !bio->bi_status;
+}
+
+static int __init bio_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;
+}
+
+fs_initcall(bio_init_post_read_processing);

fs/verity/verify.c +12 -0

index fbfb68eff358..4f7cd2269e83 100644
@@ -13,6 +13,7 @@
 #include <linux/pagemap.h>
 #include <linux/ratelimit.h>
 #include <linux/scatterlist.h>
+#include <linux/post_read_process.h>
 
 struct workqueue_struct *fsverity_read_workqueue;
 

@@ -283,6 +284,16 @@ void fsverity_verify_bio(struct bio *bio)
 EXPORT_SYMBOL_GPL(fsverity_verify_bio);
 #endif /* CONFIG_BLOCK */
 
+static void fsverity_verify_work(struct work_struct *work)
+{
+	struct bio_post_read_ctx *ctx =
+		container_of(work, struct bio_post_read_ctx, work);
+
+	fsverity_verify_bio(ctx->bio);
+
+	bio_post_read_processing(ctx);
+}
+
 /**
  * fsverity_enqueue_verify_work - enqueue work on the fs-verity workqueue
  *

@@ -290,6 +301,7 @@ EXPORT_SYMBOL_GPL(fsverity_verify_bio);
  */
 void fsverity_enqueue_verify_work(struct work_struct *work)
 {
+	INIT_WORK(work, fsverity_verify_work);
 	queue_work(fsverity_read_workqueue, work);
 }
 EXPORT_SYMBOL_GPL(fsverity_enqueue_verify_work);

include/linux/fscrypt.h +0 -0

index 6ba193c23f37..13f70e22aff2 100644
@@ -68,10 +68,6 @@ struct fscrypt_ctx {
 			struct page *bounce_page;	/* Ciphertext page */
 			struct page *control_page;	/* Original page  */

include/linux/fscrypt.h struct fscrypt_ctx { · line 74

Quoted 5 lines

include/linux/fscrypt.h +0 -4

 		} w;
-		struct {
-			struct bio *bio;
-			struct work_struct work;
-		} r;

Now that 'struct fscrypt_ctx' is only used for writes, the 'w' part should be changed to an anonymous struct.

Quoted 42 lines

include/linux/fscrypt.h +0 -7

 		struct list_head free_list;	/* Free list */
 	};
 	u8 flags;				/* Flags */

@@ -206,8 +202,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_pullback_bio_page(struct page **, bool);
 extern int fscrypt_zeroout_range(const struct inode *, pgoff_t, sector_t,
 				 unsigned int);

@@ -376,11 +370,6 @@ static inline void fscrypt_decrypt_bio(struct bio *bio)
 {
 }
 
-static inline void fscrypt_enqueue_decrypt_bio(struct fscrypt_ctx *ctx,
-					       struct bio *bio)
-{
-}
-
 static inline void fscrypt_pullback_bio_page(struct page **page, bool restore)
 {
 	return;

include/linux/post_read_process.h new file +11 -0

new file mode 100644
index 000000000000..09e52928f861
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _POST_READ_PROCESS_H
+#define _POST_READ_PROCESS_H
+
+struct bio_post_read_ctx {
+	struct bio *bio;
+	struct inode *inode;
+	struct work_struct work;
+	unsigned int cur_step;
+	unsigned int enabled_steps;
+};

include/linux/post_read_process.h line 21

Quoted 10 lines

include/linux/post_read_process.h new file +10 -0

+
+void end_bio_post_read_processing(struct bio *bio);
+void bio_post_read_processing(struct bio_post_read_ctx *ctx);
+struct bio_post_read_ctx *get_bio_post_read_ctx(struct inode *inode,
+						struct bio *bio,
+						pgoff_t index);
+void put_bio_post_read_ctx(struct bio_post_read_ctx *ctx);
+bool bio_post_read_required(struct bio *bio);
+
+#endif	/* _POST_READ_PROCESS_H */

- Eric

Ext4 and F2FS store verity metadata in data extents (beyond inode->i_size) associated with a file. But other filesystems might choose alternative means to store verity metadata. Hence this commit adds a callback function pointer to 'struct fsverity_operations' to help in deciding if verity operation needs to performed against a page-cache page holding file data.

Signed-off-by Chandan Rajendra <[email protected]>

---
 fs/ext4/super.c          | 9 +++++++++
 fs/post_read_process.c   | 5 +++--
 include/linux/fsverity.h | 1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

fs/ext4/super.c +0 -0

index 9314dddfbf34..2d7781ab6824 100644

fs/post_read_process.c +0 -0

index 9720eeff0160..1f8663d70247 100644

fs/post_read_process.c +0 -0

 #endif
 	if (post_read_steps) {

include/linux/fsverity.h +1 -0

index 7c33b42abf1b..b83712d6c79a 100644
@@ -18,6 +18,7 @@ struct fsverity_operations {
 	int (*set_verity)(struct inode *inode, loff_t data_i_size);
 	int (*get_metadata_end)(struct inode *inode, loff_t *metadata_end_ret);
 	struct page *(*read_metadata_page)(struct inode *inode, pgoff_t index);
+	bool (*verity_required)(struct inode *inode, pgoff_t index);
 };
 
 #ifdef CONFIG_FS_VERITY

Signature · 1 line

2.19.1

Hi Chandan,

Quoted 19 lines · Chandan Rajendra

Ext4 and F2FS store verity metadata in data extents (beyond inode->i_size) associated with a file. But other filesystems might choose alternative means to store verity metadata. Hence this commit adds a callback function pointer to 'struct fsverity_operations' to help in deciding if verity operation needs to performed against a page-cache page holding file data.

Signed-off-by Chandan Rajendra <[email protected]>

---
 fs/ext4/super.c          | 9 +++++++++
 fs/post_read_process.c   | 5 +++--
 include/linux/fsverity.h | 1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

fs/ext4/super.c +0 -0

index 9314dddfbf34..2d7781ab6824 100644

fs/ext4/super.c static struct page *ext4_read_verity_metadata_page(struct inode *inode, · line 1437

Quoted 10 lines

fs/ext4/super.c +7 -0

@@ -1428,10 +1428,19 @@ static struct page *ext4_read_verity_metadata_page(struct inode *inode,
 	return read_mapping_page(inode->i_mapping, index, NULL);
 }
 
+static bool ext4_verity_required(struct inode *inode, pgoff_t index)
+{
+	if (index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT))
+		return true;
+	else
+		return false;
+}

This can be simplified to:

	return index < (i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT;

fs/ext4/super.c static struct page *ext4_read_verity_metadata_page(struct inode *inode, · line 1445

Quoted 8 lines

fs/ext4/super.c +2 -0

+
 static const struct fsverity_operations ext4_verityops = {
 	.set_verity		= ext4_set_verity,
 	.get_metadata_end	= ext4_get_verity_metadata_end,
 	.read_metadata_page	= ext4_read_verity_metadata_page,
+	.verity_required	= ext4_verity_required,
 };
 #endif /* CONFIG_FS_VERITY */

Doesn't f2fs need this too? This patch only changes ext4.

Quoted 11 lines

fs/post_read_process.c +0 -1

index 9720eeff0160..1f8663d70247 100644
@@ -79,8 +79,9 @@ struct bio_post_read_ctx *get_bio_post_read_ctx(struct inode *inode,
 	if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
 		post_read_steps |= 1 << STEP_DECRYPT;
 #ifdef CONFIG_FS_VERITY
-	if (inode->i_verity_info != NULL &&

fs/post_read_process.c struct bio_post_read_ctx *get_bio_post_read_ctx(struct inode *inode, · line 85

Quoted 5 lines

fs/post_read_process.c +3 -1

-		(index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT)))
+	if (inode->i_verity_info != NULL
+		&& inode->i_sb->s_vop->verity_required
+		&& inode->i_sb->s_vop->verity_required(inode, index))
 		post_read_steps |= 1 << STEP_VERITY;

If ->verity_required is NULL, shouldn't that be equivalent to ->verity_required() returning true?

Quoted 7 lines

fs/post_read_process.c +0 -0

 #endif
 	if (post_read_steps) {

include/linux/fsverity.h +0 -0

index 7c33b42abf1b..b83712d6c79a 100644

include/linux/fsverity.h struct fsverity_operations { · line 24

Quoted 7 lines

include/linux/fsverity.h +1 -0

@@ -18,6 +18,7 @@ struct fsverity_operations {
 	int (*set_verity)(struct inode *inode, loff_t data_i_size);
 	int (*get_metadata_end)(struct inode *inode, loff_t *metadata_end_ret);
 	struct page *(*read_metadata_page)(struct inode *inode, pgoff_t index);
+	bool (*verity_required)(struct inode *inode, pgoff_t index);
 };
 
 #ifdef CONFIG_FS_VERITY

- Eric

Ext4 and F2FS currently use a non-NULL value stored at bio->bi_private to determine if the contents of the bio need to be "post processed" i.e. whether its contents need to be decrypted and/or verified. For block size < page size scenario, bio->bi_private would hold a pointer to buffer_head. Hence, this commit adds the new flag REQ_POST_READ_PROC to be able to decisively check for post process requirement for a bio.

Signed-off-by Chandan Rajendra <[email protected]>

---
 fs/ext4/readpage.c        | 11 +++++++++--
 fs/post_read_process.c    |  2 +-
 include/linux/blk_types.h |  2 ++
 3 files changed, 12 insertions(+), 3 deletions(-)

fs/ext4/readpage.c +9 -2

index 8943fc41fd33..c7dbab35deaa 100644
@@ -245,6 +245,7 @@ int ext4_mpage_readpages(struct address_space *mapping,
 		}
 		if (bio == NULL) {
 			struct bio_post_read_ctx *ctx;
+			unsigned int op_flags = 0;
 
 			bio = bio_alloc(GFP_KERNEL,
 				min_t(int, nr_pages, BIO_MAX_PAGES));

@@ -259,8 +260,14 @@ int ext4_mpage_readpages(struct address_space *mapping,
 			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);
+
+			if (is_readahead)
+				op_flags |= REQ_RAHEAD;
+
+			if (ctx)
+				op_flags |= REQ_POST_READ_PROC;
+
+			bio_set_op_attrs(bio, REQ_OP_READ, op_flags);
 		}
 
 		length = first_hole << blkbits;

fs/post_read_process.c +1 -1

index 1f8663d70247..66c1c6e57e70 100644
@@ -104,7 +104,7 @@ void put_bio_post_read_ctx(struct bio_post_read_ctx *ctx)
 
 bool bio_post_read_required(struct bio *bio)
 {
-	return bio->bi_private && !bio->bi_status;
+	return bio->bi_opf & REQ_POST_READ_PROC;
 }
 
 static int __init bio_init_post_read_processing(void)

include/linux/blk_types.h +2 -0

index 5c7e7f859a24..6904945c8c40 100644
@@ -320,6 +320,7 @@ enum req_flag_bits {
 	__REQ_RAHEAD,		/* read ahead, can fail anytime */
 	__REQ_BACKGROUND,	/* background IO */
 	__REQ_NOWAIT,           /* Don't wait if request will block */
+	__REQ_POST_READ_PROC,
 
 	/* command specific flags for REQ_OP_WRITE_ZEROES: */
 	__REQ_NOUNMAP,		/* do not free blocks when zeroing */

@@ -346,6 +347,7 @@ enum req_flag_bits {
 #define REQ_RAHEAD		(1ULL << __REQ_RAHEAD)
 #define REQ_BACKGROUND		(1ULL << __REQ_BACKGROUND)
 #define REQ_NOWAIT		(1ULL << __REQ_NOWAIT)
+#define REQ_POST_READ_PROC	(1ULL << __REQ_POST_READ_PROC)
 #define REQ_NOUNMAP		(1ULL << __REQ_NOUNMAP)
 #define REQ_HIPRI		(1ULL << __REQ_HIPRI)
 

Signature · 1 line

2.19.1

Hi Chandan,

Quoted 69 lines · Chandan Rajendra

Ext4 and F2FS currently use a non-NULL value stored at bio->bi_private to determine if the contents of the bio need to be "post processed" i.e. whether its contents need to be decrypted and/or verified. For block size < page size scenario, bio->bi_private would hold a pointer to buffer_head. Hence, this commit adds the new flag REQ_POST_READ_PROC to be able to decisively check for post process requirement for a bio.

Signed-off-by Chandan Rajendra <[email protected]>

---
 fs/ext4/readpage.c        | 11 +++++++++--
 fs/post_read_process.c    |  2 +-
 include/linux/blk_types.h |  2 ++
 3 files changed, 12 insertions(+), 3 deletions(-)

fs/ext4/readpage.c +9 -2

index 8943fc41fd33..c7dbab35deaa 100644
@@ -245,6 +245,7 @@ int ext4_mpage_readpages(struct address_space *mapping,
 		}
 		if (bio == NULL) {
 			struct bio_post_read_ctx *ctx;
+			unsigned int op_flags = 0;
 
 			bio = bio_alloc(GFP_KERNEL,
 				min_t(int, nr_pages, BIO_MAX_PAGES));

@@ -259,8 +260,14 @@ int ext4_mpage_readpages(struct address_space *mapping,
 			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);
+
+			if (is_readahead)
+				op_flags |= REQ_RAHEAD;
+
+			if (ctx)
+				op_flags |= REQ_POST_READ_PROC;
+
+			bio_set_op_attrs(bio, REQ_OP_READ, op_flags);
 		}
 
 		length = first_hole << blkbits;

fs/post_read_process.c +1 -1

index 1f8663d70247..66c1c6e57e70 100644
@@ -104,7 +104,7 @@ void put_bio_post_read_ctx(struct bio_post_read_ctx *ctx)
 
 bool bio_post_read_required(struct bio *bio)
 {
-	return bio->bi_private && !bio->bi_status;
+	return bio->bi_opf & REQ_POST_READ_PROC;
 }
 
 static int __init bio_init_post_read_processing(void)

include/linux/blk_types.h +1 -0

index 5c7e7f859a24..6904945c8c40 100644
@@ -320,6 +320,7 @@ enum req_flag_bits {
 	__REQ_RAHEAD,		/* read ahead, can fail anytime */
 	__REQ_BACKGROUND,	/* background IO */
 	__REQ_NOWAIT,           /* Don't wait if request will block */
+	__REQ_POST_READ_PROC,
 
 	/* command specific flags for REQ_OP_WRITE_ZEROES: */
 	__REQ_NOUNMAP,		/* do not free blocks when zeroing */

include/linux/blk_types.h enum req_flag_bits { · line 353

Quoted 7 lines

include/linux/blk_types.h +1 -0

@@ -346,6 +347,7 @@ enum req_flag_bits {
 #define REQ_RAHEAD		(1ULL << __REQ_RAHEAD)
 #define REQ_BACKGROUND		(1ULL << __REQ_BACKGROUND)
 #define REQ_NOWAIT		(1ULL << __REQ_NOWAIT)
+#define REQ_POST_READ_PROC	(1ULL << __REQ_POST_READ_PROC)
 #define REQ_NOUNMAP		(1ULL << __REQ_NOUNMAP)
 #define REQ_HIPRI		(1ULL << __REQ_HIPRI)
 

I don't think this is an appropriate use of a request flag, as request flags are meant for the block layer.

Also doesn't the bio still need a pointer to the bio_post_read_ctx anyway? So I don't see how this would solve the problem, if ->bi_private is already used.

readpage() implmentations zero out part of the page-cache page mapping a file hole. This commit adds a call back to "struct fsverity_operations" which readpage() can invoke to verify the file hole.

Signed-off-by Chandan Rajendra <[email protected]>

---
 fs/ext4/super.c          | 1 +
 include/linux/fsverity.h | 1 +
 2 files changed, 2 insertions(+)

fs/ext4/super.c +1 -0

index 4493ddc357c6..94e0d4b56653 100644
@@ -1459,6 +1459,7 @@ static const struct fsverity_operations ext4_verityops = {
 	.read_metadata_page	= ext4_read_verity_metadata_page,
 	.verity_required	= ext4_verity_required,
 	.readpage_limit		= ext4_readpage_limit,
+	.check_hole		= fsverity_check_hole,
 };
 #endif /* CONFIG_FS_VERITY */
 

include/linux/fsverity.h +1 -0

index fc8113acbbfe..8bc28b07d550 100644
@@ -20,6 +20,7 @@ struct fsverity_operations {
 	struct page *(*read_metadata_page)(struct inode *inode, pgoff_t index);
 	bool (*verity_required)(struct inode *inode, pgoff_t index);
 	loff_t (*readpage_limit)(struct inode *inode);
+	bool (*check_hole)(struct inode *inode, struct page *page);
 };
 
 #ifdef CONFIG_FS_VERITY

Signature · 1 line

2.19.1

Quoted 3 lines · Chandan Rajendra

readpage() implmentations zero out part of the page-cache page mapping a file hole. This commit adds a call back to "struct fsverity_operations" which readpage() can invoke to verify the file hole.

I don't see why this is needed. Since now CONFIG_FS_VERITY will be a bool, can't you just call fsverity_verify_hole() directly?

This patchset moves the "post read processing" code into a file of its own and gets the generic do_mpage_readpge() to make use of the functionality provided. With these changes in place, the patchset changes Ext4 to use mpage_readpage[s] instead of its own custom ext4_readpages() function. This is done to reduce duplicity of code across filesystems. Based on the reviews provided for this patchset, I will change F2FS to use mpage_readpage[s] and post the next version of this patchset to linux-fsdevel mailing list.

The patchset also includes patches from previous postings i.e. patches to replace per-filesystem encryption config options with a single config option that affects all filesystems making use of fscrypt code.

Chandan Rajendra (10):
ext4: use IS_ENCRYPTED() to check encryption status
f2fs: use IS_ENCRYPTED() to check encryption status
fscrypt: remove filesystem specific build config option Consolidate "post read processing" into a new file
fsverity: Add call back to decide if verity check has to be performed Introduce REQ_POST_READ_PROC bio flag
fsverity: Add call back to determine readpage limit
fsverity: Add call back to verify file holes
fs/mpage.c: Integrate post read processing
ext4: Wire up ext4_readpage[s] to use mpage_readpage[s]

 Documentation/filesystems/fscrypt.rst   |   4 +-
 arch/mips/configs/generic_defconfig     |   2 +-
 arch/nds32/configs/defconfig            |   2 +-
 arch/s390/configs/debug_defconfig       |   2 +-
 arch/s390/configs/performance_defconfig |   2 +-
 fs/Makefile                             |   3 +-
 fs/crypto/Kconfig                       |   5 +-
 fs/crypto/bio.c                         |  21 +-
 fs/crypto/crypto.c                      |   1 +
 fs/crypto/fscrypt_private.h             |   4 +-
 fs/ext4/Kconfig                         |  15 -
 fs/ext4/Makefile                        |   2 +-
 fs/ext4/dir.c                           |  10 +-
 fs/ext4/ext4.h                          |  14 +-
 fs/ext4/ext4_jbd2.h                     |   2 +-
 fs/ext4/extents.c                       |   4 +-
 fs/ext4/ialloc.c                        |   2 +-
 fs/ext4/inode.c                         |  29 +-
 fs/ext4/ioctl.c                         |   4 +-
 fs/ext4/move_extent.c                   |   3 +-
 fs/ext4/namei.c                         |  18 +-
 fs/ext4/page-io.c                       |   9 +-
 fs/ext4/readpage.c                      | 444 ------------------------
 fs/ext4/super.c                         |  43 ++-
 fs/ext4/sysfs.c                         |   4 +-
 fs/f2fs/Kconfig                         |  12 +-
 fs/f2fs/data.c                          |   4 +-
 fs/f2fs/dir.c                           |  10 +-
 fs/f2fs/f2fs.h                          |  14 +-
 fs/f2fs/file.c                          |  10 +-
 fs/f2fs/inode.c                         |   4 +-
 fs/f2fs/namei.c                         |   6 +-
 fs/f2fs/super.c                         |   8 +-
 fs/f2fs/sysfs.c                         |   4 +-
 fs/mpage.c                              |  77 +++-
 fs/post_read_process.c                  | 128 +++++++
 fs/ubifs/Kconfig                        |  12 +-
 fs/ubifs/Makefile                       |   2 +-
 fs/ubifs/ioctl.c                        |   4 +-
 fs/ubifs/sb.c                           |   2 +-
 fs/ubifs/super.c                        |   2 +-
 fs/ubifs/ubifs.h                        |   5 +-
 fs/verity/verify.c                      |  12 +
 include/linux/blk_types.h               |   2 +
 include/linux/fs.h                      |   4 +-
 include/linux/fscrypt.h                 | 405 ++++++++++++++++++++-
 include/linux/fscrypt_notsupp.h         | 231 ------------
 include/linux/fscrypt_supp.h            | 204 -----------
 include/linux/fsverity.h                |   3 +
 include/linux/post_read_process.h       |  21 ++
 50 files changed, 757 insertions(+), 1078 deletions(-)
 delete mode 100644 fs/ext4/readpage.c
 create mode 100644 fs/post_read_process.c
 delete mode 100644 include/linux/fscrypt_notsupp.h
 delete mode 100644 include/linux/fscrypt_supp.h
 create mode 100644 include/linux/post_read_process.h

This patchset moves the "post read processing" code into a file of its own and gets the generic do_mpage_readpge() to make use of the functionality provided. With these changes in place, the patchset changes Ext4 to use mpage_readpage[s] instead of its own custom ext4_readpages() function. This is done to reduce duplicity of code across filesystems. Based on the reviews provided for this patchset, I will change F2FS to use mpage_readpage[s] and post the next version of this patchset to linux-fsdevel mailing list.

The patchset also includes patches from previous postings i.e. patches to replace per-filesystem encryption config options with a single config option that affects all filesystems making use of fscrypt code.

Chandan Rajendra (10):
ext4: use IS_ENCRYPTED() to check encryption status
f2fs: use IS_ENCRYPTED() to check encryption status
fscrypt: remove filesystem specific build config option Consolidate "post read processing" into a new file
fsverity: Add call back to decide if verity check has to be performed Introduce REQ_POST_READ_PROC bio flag
fsverity: Add call back to determine readpage limit
fsverity: Add call back to verify file holes
fs/mpage.c: Integrate post read processing
ext4: Wire up ext4_readpage[s] to use mpage_readpage[s]

Hi Chandan,

Quoted 14 lines · Chandan Rajendra

This patchset moves the "post read processing" code into a file of its own and gets the generic do_mpage_readpge() to make use of the functionality provided. With these changes in place, the patchset changes Ext4 to use mpage_readpage[s] instead of its own custom ext4_readpages() function. This is done to reduce duplicity of code across filesystems. Based on the reviews provided for this patchset, I will change F2FS to use mpage_readpage[s] and post the next version of this patchset to linux-fsdevel mailing list.

The patchset also includes patches from previous postings i.e. patches to replace per-filesystem encryption config options with a single config option that affects all filesystems making use of fscrypt code.

Quoted 12 lines

Chandan Rajendra (10):
ext4: use IS_ENCRYPTED() to check encryption status
f2fs: use IS_ENCRYPTED() to check encryption status
fscrypt: remove filesystem specific build config option Consolidate "post read processing" into a new file
fsverity: Add call back to decide if verity check has to be performed Introduce REQ_POST_READ_PROC bio flag
fsverity: Add call back to determine readpage limit
fsverity: Add call back to verify file holes
fs/mpage.c: Integrate post read processing
ext4: Wire up ext4_readpage[s] to use mpage_readpage[s]

Thanks for working on this! This will also make it much easier to support block_size != PAGE_SIZE in ext4 encryption, right? I think this is the best path forward, but I'll take a closer look at your new patches.

FYI regarding practical matters, merging fs-verity was delayed due to disagreement about the API. See https://lwn.net/Articles/775872/.

We don't have to wait for fs-verity for your initial fscrypt changes, though:

ext4: use IS_ENCRYPTED() to check encryption status
f2fs: use IS_ENCRYPTED() to check encryption status
fscrypt: remove filesystem specific build config option

So, a couple weeks ago Ted and I already queued those three patches in fscrypt.git (https://git.kernel.org/pub/scm/linux/kernel/git/tytso/fscrypt.git
branch "master", though we plan to change the repo soon) for the upcoming merge window, based on upstream rather than fs-verity. Are you fine with that?

I also suggest adding linux-fsdevel to the Cc given the fs/*.c changes.

Thanks!

- Eric

Based on the reviews provided for this patchset, I
will change F2FS to use mpage_readpage[s] and post the next version of this patchset to linux-fsdevel mailing list.

Quoted 3 lines · Chandan Rajendra

Based on the reviews provided for this patchset, I
will change F2FS to use mpage_readpage[s] and post the next version of this patchset to linux-fsdevel mailing list.

Is that really possible? F2FS has other functionality in its ->readpages(), such as support for filesystems that use multiple block devices.