Re: [f2fs-dev] [PATCH V3 0/7] Consolidate FS read I/O callbacks code
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 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V3 0/7] Consolidate FS read I/O callbacks code
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);
+
[f2fs-dev] [PATCH V3 0/7] Consolidate FS read I/O callbacks code
This patchset moves the "FS read I/O callbacks" code into a file of its own (i.e. fs/read_callbacks.c) and modifies the generic do_mpage_readpge() to make use of the functionality provided. "FS read I/O callbacks" code implements the state machine that needs to be executed after reading data from files that are encrypted and/or have verity metadata associated with them. With these changes in place, the patchset changes Ext4 to use mpage_readpage[s] instead of its own custom ext4_readpage[s]() functions. This is done to reduce duplication of code across filesystems. Also, "FS read I/O callbacks" source files will be built only if CONFIG_FS_ENCRYPTION is enabled. The patchset also modifies fs/buffer.c to get file encryption/decryption to work with subpage-sized blocks. The patches can also be obtained from https://github.com/chandanr/linux.git at branch subpage-encryption-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 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
