Re: [f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
On Tuesday, April 30, 2019 7:07:28 AM IST Chao Yu wrote:
> On 2019/4/28 12:31, Chandan Rajendra wrote:
> > The "read callbacks" code is used by both Ext4 and F2FS. Hence to
> > remove duplicity, this commit moves the code into
> > include/linux/read_callbacks.h and fs/read_callbacks.c.
> >
> > The corresponding decrypt and verity "work" functions have been moved
> > inside fscrypt and fsverity sources. With these in place, the read
> > callbacks code now has to just invoke enqueue functions provided by
> > fscrypt and fsverity.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/Kconfig | 4 +
> > fs/Makefile| 4 +
> > fs/crypto/Kconfig | 1 +
> > fs/crypto/bio.c| 23 ++---
> > fs/crypto/crypto.c | 17 +--
> > fs/crypto/fscrypt_private.h| 3 +
> > fs/ext4/ext4.h | 2 -
> > fs/ext4/readpage.c | 183 +
> > fs/ext4/super.c| 9 +-
> > fs/f2fs/data.c | 148 --
> > fs/f2fs/super.c| 9 +-
> > fs/read_callbacks.c| 136
> > fs/verity/Kconfig | 1 +
> > fs/verity/verify.c | 12 +++
> > include/linux/fscrypt.h| 20 +---
> > include/linux/read_callbacks.h | 21
> > 16 files changed, 251 insertions(+), 342 deletions(-)
> > create mode 100644 fs/read_callbacks.c
> > create mode 100644 include/linux/read_callbacks.h
> >
> > diff --git a/fs/Kconfig b/fs/Kconfig
> > index 97f9eb8df713..03084f2dbeaf 100644
> > --- a/fs/Kconfig
> > +++ b/fs/Kconfig
> > @@ -308,6 +308,10 @@ config NFS_COMMON
> > depends on NFSD || NFS_FS || LOCKD
> > default y
> >
> > +config FS_READ_CALLBACKS
> > + bool
> > + default n
> > +
> > source "net/sunrpc/Kconfig"
> > source "fs/ceph/Kconfig"
> > source "fs/cifs/Kconfig"
> > diff --git a/fs/Makefile b/fs/Makefile
> > index 9dd2186e74b5..e0c0fce8cf40 100644
> > --- a/fs/Makefile
> > +++ b/fs/Makefile
> > @@ -21,6 +21,10 @@ else
> > obj-y += no-block.o
> > endif
> >
> > +ifeq ($(CONFIG_FS_READ_CALLBACKS),y)
> > +obj-y += read_callbacks.o
> > +endif
> > +
> > obj-$(CONFIG_PROC_FS) += proc_namespace.o
> >
> > obj-y += notify/
> > diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> > index f0de238000c0..163c328bcbd4 100644
> > --- a/fs/crypto/Kconfig
> > +++ b/fs/crypto/Kconfig
> > @@ -8,6 +8,7 @@ config FS_ENCRYPTION
> > select CRYPTO_CTS
> > select CRYPTO_SHA256
> > select KEYS
> > + select FS_READ_CALLBACKS
> > 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 5759bcd018cd..27f5618174f2 100644
> > --- a/fs/crypto/bio.c
> > +++ b/fs/crypto/bio.c
> > @@ -24,6 +24,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +
> > #include "fscrypt_private.h"
> >
> > static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> > @@ -54,24 +56,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 read_callbacks_ctx *ctx =
> > + container_of(work, struct read_callbacks_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(>r.work, completion_pages);
> > - ctx->r.bio = bio;
> > - fscrypt_enqueue_decrypt_work(>r.work);
> > + read_callbacks(ctx);
> > }
> > -EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
> >
> > void fscrypt_pullback_bio_page(struct page **page, bool restore)
> > {
> > @@ -87,7 +80,7 @@ void fscrypt_pullback_bio_page(struct page **page, bool
> > restore)
> > ctx = (struct fscrypt_ctx *)page_private(bounce_page);
> >
> > /* restore control page */
> > - *page = ctx->w.control_page;
> > + *page = ctx->control_page;
> >
> > if (restore)
> > fscrypt_restore_control_page(bounce_page);
> > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > index 3fc84bf2b1e5..ffa9302a7351 100644
> > --- a/fs/crypto/crypto.c
> > +++ b/fs/crypto/crypto.c
> > @@ -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);
> > @@ -70,11 +71,11 @@ void
Re: [f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
On Tuesday, April 30, 2019 5:30:28 AM IST Eric Biggers wrote:
> Hi Chandan,
>
> On Sun, Apr 28, 2019 at 10:01:10AM +0530, Chandan Rajendra wrote:
> > The "read callbacks" code is used by both Ext4 and F2FS. Hence to
> > remove duplicity, this commit moves the code into
> > include/linux/read_callbacks.h and fs/read_callbacks.c.
> >
> > The corresponding decrypt and verity "work" functions have been moved
> > inside fscrypt and fsverity sources. With these in place, the read
> > callbacks code now has to just invoke enqueue functions provided by
> > fscrypt and fsverity.
> >
> > Signed-off-by: Chandan Rajendra
> > ---
> > fs/Kconfig | 4 +
> > fs/Makefile| 4 +
> > fs/crypto/Kconfig | 1 +
> > fs/crypto/bio.c| 23 ++---
> > fs/crypto/crypto.c | 17 +--
> > fs/crypto/fscrypt_private.h| 3 +
> > fs/ext4/ext4.h | 2 -
> > fs/ext4/readpage.c | 183 +
> > fs/ext4/super.c| 9 +-
> > fs/f2fs/data.c | 148 --
> > fs/f2fs/super.c| 9 +-
> > fs/read_callbacks.c| 136
> > fs/verity/Kconfig | 1 +
> > fs/verity/verify.c | 12 +++
> > include/linux/fscrypt.h| 20 +---
> > include/linux/read_callbacks.h | 21
> > 16 files changed, 251 insertions(+), 342 deletions(-)
> > create mode 100644 fs/read_callbacks.c
> > create mode 100644 include/linux/read_callbacks.h
> >
>
> For easier review, can you split this into multiple patches? Ideally the ext4
> and f2fs patches would be separate, but if that's truly not possible due to
> interdependencies it seems you could at least do:
>
> 1. Introduce the read_callbacks.
> 2. Convert encryption to use the read_callbacks.
> 3. Remove union from struct fscrypt_context.
>
> Also: just FYI, fs-verity isn't upstream yet, and in the past few months I
> haven't had much time to work on it. So you might consider arranging your
> series so that initially just fscrypt is supported. That will be useful on
> its
> own, for block_size < PAGE_SIZE support. Then fsverity can be added later.
>
> > diff --git a/fs/Kconfig b/fs/Kconfig
> > index 97f9eb8df713..03084f2dbeaf 100644
> > --- a/fs/Kconfig
> > +++ b/fs/Kconfig
> > @@ -308,6 +308,10 @@ config NFS_COMMON
> > depends on NFSD || NFS_FS || LOCKD
> > default y
> >
> > +config FS_READ_CALLBACKS
> > + bool
> > + default n
>
> 'default n' is unnecesary, since 'n' is already the default.
>
> > +
> > source "net/sunrpc/Kconfig"
> > source "fs/ceph/Kconfig"
> > source "fs/cifs/Kconfig"
> > diff --git a/fs/Makefile b/fs/Makefile
> > index 9dd2186e74b5..e0c0fce8cf40 100644
> > --- a/fs/Makefile
> > +++ b/fs/Makefile
> > @@ -21,6 +21,10 @@ else
> > obj-y += no-block.o
> > endif
> >
> > +ifeq ($(CONFIG_FS_READ_CALLBACKS),y)
> > +obj-y += read_callbacks.o
> > +endif
> > +
>
> This can be simplified to:
>
> obj-$(CONFIG_FS_READ_CALLBACKS) += read_callbacks.o
>
> > diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
> > new file mode 100644
> > index ..b6d5b95e67d7
> > --- /dev/null
> > +++ b/fs/read_callbacks.c
> > @@ -0,0 +1,136 @@
> > +// 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
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define NUM_PREALLOC_POST_READ_CTXS128
> > +
> > +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,
> > + STEP_VERITY,
> > +};
> > +
> > +void end_read_callbacks(struct bio *bio)
> > +{
> > + struct page *page;
> > + struct bio_vec *bv;
> > + int i;
> > + struct bvec_iter_all iter_all;
> > +
> > + bio_for_each_segment_all(bv, bio, i, iter_all) {
> > + page = bv->bv_page;
> > +
> > + BUG_ON(bio->bi_status);
> > +
> > + if (!PageError(page))
> > + SetPageUptodate(page);
> > +
> > + unlock_page(page);
> > + }
> > + if (bio->bi_private)
> > + put_read_callbacks_ctx(bio->bi_private);
> > + bio_put(bio);
> > +}
> > +EXPORT_SYMBOL(end_read_callbacks);
>
> end_read_callbacks() is only called by read_callbacks() just below, so it
> should
> be 'static'.
>
> > +
> > +struct read_callbacks_ctx *get_read_callbacks_ctx(struct inode *inode,
> > + struct bio *bio,
> > + pgoff_t index)
> > +{
> > + unsigned int read_callbacks_steps = 0;
>
> Rename
Re: [f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
On Tuesday, April 30, 2019 11:35:08 PM IST Eric Biggers wrote: > On Sun, Apr 28, 2019 at 10:01:10AM +0530, Chandan Rajendra wrote: > > The "read callbacks" code is used by both Ext4 and F2FS. Hence to > > remove duplicity, this commit moves the code into > > include/linux/read_callbacks.h and fs/read_callbacks.c. > > > > The corresponding decrypt and verity "work" functions have been moved > > inside fscrypt and fsverity sources. With these in place, the read > > callbacks code now has to just invoke enqueue functions provided by > > fscrypt and fsverity. > > > > Signed-off-by: Chandan Rajendra > > --- > > fs/Kconfig | 4 + > > fs/Makefile| 4 + > > fs/crypto/Kconfig | 1 + > > fs/crypto/bio.c| 23 ++--- > > fs/crypto/crypto.c | 17 +-- > > fs/crypto/fscrypt_private.h| 3 + > > fs/ext4/ext4.h | 2 - > > fs/ext4/readpage.c | 183 + > > fs/ext4/super.c| 9 +- > > fs/f2fs/data.c | 148 -- > > fs/f2fs/super.c| 9 +- > > fs/read_callbacks.c| 136 > > fs/verity/Kconfig | 1 + > > fs/verity/verify.c | 12 +++ > > include/linux/fscrypt.h| 20 +--- > > include/linux/read_callbacks.h | 21 > > 16 files changed, 251 insertions(+), 342 deletions(-) > > create mode 100644 fs/read_callbacks.c > > create mode 100644 include/linux/read_callbacks.h > > > > diff --git a/fs/Kconfig b/fs/Kconfig > > index 97f9eb8df713..03084f2dbeaf 100644 > > --- a/fs/Kconfig > > +++ b/fs/Kconfig > > @@ -308,6 +308,10 @@ config NFS_COMMON > > depends on NFSD || NFS_FS || LOCKD > > default y > > > > +config FS_READ_CALLBACKS > > + bool > > + default n > > + > > source "net/sunrpc/Kconfig" > > source "fs/ceph/Kconfig" > > source "fs/cifs/Kconfig" > > This shouldn't be under the 'if NETWORK_FILESYSTEMS' block, since it has > nothing > to do with network filesystems. When trying to compile this I got: > > WARNING: unmet direct dependencies detected for FS_READ_CALLBACKS > Depends on [n]: NETWORK_FILESYSTEMS [=n] > Selected by [y]: > - FS_ENCRYPTION [=y] > - FS_VERITY [=y] > > Perhaps put it just below FS_IOMAP? > > > diff --git a/fs/Makefile b/fs/Makefile > > index 9dd2186e74b5..e0c0fce8cf40 100644 > > --- a/fs/Makefile > > +++ b/fs/Makefile > > @@ -21,6 +21,10 @@ else > > obj-y += no-block.o > > endif > > > > +ifeq ($(CONFIG_FS_READ_CALLBACKS),y) > > +obj-y += read_callbacks.o > > +endif > > + > > obj-$(CONFIG_PROC_FS) += proc_namespace.o > > > > obj-y += notify/ > > diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig > > index f0de238000c0..163c328bcbd4 100644 > > --- a/fs/crypto/Kconfig > > +++ b/fs/crypto/Kconfig > > @@ -8,6 +8,7 @@ config FS_ENCRYPTION > > select CRYPTO_CTS > > select CRYPTO_SHA256 > > select KEYS > > + select FS_READ_CALLBACKS > > help > > Enable encryption of files and directories. This > > feature is similar to ecryptfs, but it is more memory > > This selection needs to be conditional on BLOCK. > > select FS_READ_CALLBACKS if BLOCK > > Otherwise, building without BLOCK and with UBIFS encryption support fails. > > fs/read_callbacks.c: In function ‘end_read_callbacks’: > fs/read_callbacks.c:34:23: error: storage size of ‘iter_all’ isn’t known > struct bvec_iter_all iter_all; > ^~~~ > fs/read_callbacks.c:37:20: error: dereferencing pointer to incomplete > type ‘struct buffer_head’ > if (!PageError(bh->b_page)) > > [...] > I will fix this in the next version of this patchset. -- 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 V2 02/13] Consolidate "read callbacks" into a new file
On Sun, Apr 28, 2019 at 10:01:10AM +0530, Chandan Rajendra wrote: > The "read callbacks" code is used by both Ext4 and F2FS. Hence to > remove duplicity, this commit moves the code into > include/linux/read_callbacks.h and fs/read_callbacks.c. > > The corresponding decrypt and verity "work" functions have been moved > inside fscrypt and fsverity sources. With these in place, the read > callbacks code now has to just invoke enqueue functions provided by > fscrypt and fsverity. > > Signed-off-by: Chandan Rajendra > --- > fs/Kconfig | 4 + > fs/Makefile| 4 + > fs/crypto/Kconfig | 1 + > fs/crypto/bio.c| 23 ++--- > fs/crypto/crypto.c | 17 +-- > fs/crypto/fscrypt_private.h| 3 + > fs/ext4/ext4.h | 2 - > fs/ext4/readpage.c | 183 + > fs/ext4/super.c| 9 +- > fs/f2fs/data.c | 148 -- > fs/f2fs/super.c| 9 +- > fs/read_callbacks.c| 136 > fs/verity/Kconfig | 1 + > fs/verity/verify.c | 12 +++ > include/linux/fscrypt.h| 20 +--- > include/linux/read_callbacks.h | 21 > 16 files changed, 251 insertions(+), 342 deletions(-) > create mode 100644 fs/read_callbacks.c > create mode 100644 include/linux/read_callbacks.h > > diff --git a/fs/Kconfig b/fs/Kconfig > index 97f9eb8df713..03084f2dbeaf 100644 > --- a/fs/Kconfig > +++ b/fs/Kconfig > @@ -308,6 +308,10 @@ config NFS_COMMON > depends on NFSD || NFS_FS || LOCKD > default y > > +config FS_READ_CALLBACKS > + bool > + default n > + > source "net/sunrpc/Kconfig" > source "fs/ceph/Kconfig" > source "fs/cifs/Kconfig" This shouldn't be under the 'if NETWORK_FILESYSTEMS' block, since it has nothing to do with network filesystems. When trying to compile this I got: WARNING: unmet direct dependencies detected for FS_READ_CALLBACKS Depends on [n]: NETWORK_FILESYSTEMS [=n] Selected by [y]: - FS_ENCRYPTION [=y] - FS_VERITY [=y] Perhaps put it just below FS_IOMAP? > diff --git a/fs/Makefile b/fs/Makefile > index 9dd2186e74b5..e0c0fce8cf40 100644 > --- a/fs/Makefile > +++ b/fs/Makefile > @@ -21,6 +21,10 @@ else > obj-y += no-block.o > endif > > +ifeq ($(CONFIG_FS_READ_CALLBACKS),y) > +obj-y += read_callbacks.o > +endif > + > obj-$(CONFIG_PROC_FS) += proc_namespace.o > > obj-y+= notify/ > diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig > index f0de238000c0..163c328bcbd4 100644 > --- a/fs/crypto/Kconfig > +++ b/fs/crypto/Kconfig > @@ -8,6 +8,7 @@ config FS_ENCRYPTION > select CRYPTO_CTS > select CRYPTO_SHA256 > select KEYS > + select FS_READ_CALLBACKS > help > Enable encryption of files and directories. This > feature is similar to ecryptfs, but it is more memory This selection needs to be conditional on BLOCK. select FS_READ_CALLBACKS if BLOCK Otherwise, building without BLOCK and with UBIFS encryption support fails. fs/read_callbacks.c: In function ‘end_read_callbacks’: fs/read_callbacks.c:34:23: error: storage size of ‘iter_all’ isn’t known struct bvec_iter_all iter_all; ^~~~ fs/read_callbacks.c:37:20: error: dereferencing pointer to incomplete type ‘struct buffer_head’ if (!PageError(bh->b_page)) [...] - Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
On 2019/4/28 12:31, Chandan Rajendra wrote:
> The "read callbacks" code is used by both Ext4 and F2FS. Hence to
> remove duplicity, this commit moves the code into
> include/linux/read_callbacks.h and fs/read_callbacks.c.
>
> The corresponding decrypt and verity "work" functions have been moved
> inside fscrypt and fsverity sources. With these in place, the read
> callbacks code now has to just invoke enqueue functions provided by
> fscrypt and fsverity.
>
> Signed-off-by: Chandan Rajendra
> ---
> fs/Kconfig | 4 +
> fs/Makefile| 4 +
> fs/crypto/Kconfig | 1 +
> fs/crypto/bio.c| 23 ++---
> fs/crypto/crypto.c | 17 +--
> fs/crypto/fscrypt_private.h| 3 +
> fs/ext4/ext4.h | 2 -
> fs/ext4/readpage.c | 183 +
> fs/ext4/super.c| 9 +-
> fs/f2fs/data.c | 148 --
> fs/f2fs/super.c| 9 +-
> fs/read_callbacks.c| 136
> fs/verity/Kconfig | 1 +
> fs/verity/verify.c | 12 +++
> include/linux/fscrypt.h| 20 +---
> include/linux/read_callbacks.h | 21
> 16 files changed, 251 insertions(+), 342 deletions(-)
> create mode 100644 fs/read_callbacks.c
> create mode 100644 include/linux/read_callbacks.h
>
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 97f9eb8df713..03084f2dbeaf 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -308,6 +308,10 @@ config NFS_COMMON
> depends on NFSD || NFS_FS || LOCKD
> default y
>
> +config FS_READ_CALLBACKS
> + bool
> + default n
> +
> source "net/sunrpc/Kconfig"
> source "fs/ceph/Kconfig"
> source "fs/cifs/Kconfig"
> diff --git a/fs/Makefile b/fs/Makefile
> index 9dd2186e74b5..e0c0fce8cf40 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -21,6 +21,10 @@ else
> obj-y += no-block.o
> endif
>
> +ifeq ($(CONFIG_FS_READ_CALLBACKS),y)
> +obj-y += read_callbacks.o
> +endif
> +
> obj-$(CONFIG_PROC_FS) += proc_namespace.o
>
> obj-y+= notify/
> diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> index f0de238000c0..163c328bcbd4 100644
> --- a/fs/crypto/Kconfig
> +++ b/fs/crypto/Kconfig
> @@ -8,6 +8,7 @@ config FS_ENCRYPTION
> select CRYPTO_CTS
> select CRYPTO_SHA256
> select KEYS
> + select FS_READ_CALLBACKS
> 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 5759bcd018cd..27f5618174f2 100644
> --- a/fs/crypto/bio.c
> +++ b/fs/crypto/bio.c
> @@ -24,6 +24,8 @@
> #include
> #include
> #include
> +#include
> +
> #include "fscrypt_private.h"
>
> static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
> @@ -54,24 +56,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 read_callbacks_ctx *ctx =
> + container_of(work, struct read_callbacks_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(>r.work, completion_pages);
> - ctx->r.bio = bio;
> - fscrypt_enqueue_decrypt_work(>r.work);
> + read_callbacks(ctx);
> }
> -EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
>
> void fscrypt_pullback_bio_page(struct page **page, bool restore)
> {
> @@ -87,7 +80,7 @@ void fscrypt_pullback_bio_page(struct page **page, bool
> restore)
> ctx = (struct fscrypt_ctx *)page_private(bounce_page);
>
> /* restore control page */
> - *page = ctx->w.control_page;
> + *page = ctx->control_page;
>
> if (restore)
> fscrypt_restore_control_page(bounce_page);
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 3fc84bf2b1e5..ffa9302a7351 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -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);
> @@ -70,11 +71,11 @@ void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
> {
> unsigned long flags;
>
> - if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->w.bounce_page) {
> - mempool_free(ctx->w.bounce_page, fscrypt_bounce_page_pool);
> - ctx->w.bounce_page =
Re: [f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
Hi Chandan,
On Sun, Apr 28, 2019 at 10:01:10AM +0530, Chandan Rajendra wrote:
> The "read callbacks" code is used by both Ext4 and F2FS. Hence to
> remove duplicity, this commit moves the code into
> include/linux/read_callbacks.h and fs/read_callbacks.c.
>
> The corresponding decrypt and verity "work" functions have been moved
> inside fscrypt and fsverity sources. With these in place, the read
> callbacks code now has to just invoke enqueue functions provided by
> fscrypt and fsverity.
>
> Signed-off-by: Chandan Rajendra
> ---
> fs/Kconfig | 4 +
> fs/Makefile| 4 +
> fs/crypto/Kconfig | 1 +
> fs/crypto/bio.c| 23 ++---
> fs/crypto/crypto.c | 17 +--
> fs/crypto/fscrypt_private.h| 3 +
> fs/ext4/ext4.h | 2 -
> fs/ext4/readpage.c | 183 +
> fs/ext4/super.c| 9 +-
> fs/f2fs/data.c | 148 --
> fs/f2fs/super.c| 9 +-
> fs/read_callbacks.c| 136
> fs/verity/Kconfig | 1 +
> fs/verity/verify.c | 12 +++
> include/linux/fscrypt.h| 20 +---
> include/linux/read_callbacks.h | 21
> 16 files changed, 251 insertions(+), 342 deletions(-)
> create mode 100644 fs/read_callbacks.c
> create mode 100644 include/linux/read_callbacks.h
>
For easier review, can you split this into multiple patches? Ideally the ext4
and f2fs patches would be separate, but if that's truly not possible due to
interdependencies it seems you could at least do:
1. Introduce the read_callbacks.
2. Convert encryption to use the read_callbacks.
3. Remove union from struct fscrypt_context.
Also: just FYI, fs-verity isn't upstream yet, and in the past few months I
haven't had much time to work on it. So you might consider arranging your
series so that initially just fscrypt is supported. That will be useful on its
own, for block_size < PAGE_SIZE support. Then fsverity can be added later.
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 97f9eb8df713..03084f2dbeaf 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -308,6 +308,10 @@ config NFS_COMMON
> depends on NFSD || NFS_FS || LOCKD
> default y
>
> +config FS_READ_CALLBACKS
> + bool
> + default n
'default n' is unnecesary, since 'n' is already the default.
> +
> source "net/sunrpc/Kconfig"
> source "fs/ceph/Kconfig"
> source "fs/cifs/Kconfig"
> diff --git a/fs/Makefile b/fs/Makefile
> index 9dd2186e74b5..e0c0fce8cf40 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -21,6 +21,10 @@ else
> obj-y += no-block.o
> endif
>
> +ifeq ($(CONFIG_FS_READ_CALLBACKS),y)
> +obj-y += read_callbacks.o
> +endif
> +
This can be simplified to:
obj-$(CONFIG_FS_READ_CALLBACKS) += read_callbacks.o
> diff --git a/fs/read_callbacks.c b/fs/read_callbacks.c
> new file mode 100644
> index ..b6d5b95e67d7
> --- /dev/null
> +++ b/fs/read_callbacks.c
> @@ -0,0 +1,136 @@
> +// 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
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define NUM_PREALLOC_POST_READ_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,
> + STEP_VERITY,
> +};
> +
> +void end_read_callbacks(struct bio *bio)
> +{
> + struct page *page;
> + struct bio_vec *bv;
> + int i;
> + struct bvec_iter_all iter_all;
> +
> + bio_for_each_segment_all(bv, bio, i, iter_all) {
> + page = bv->bv_page;
> +
> + BUG_ON(bio->bi_status);
> +
> + if (!PageError(page))
> + SetPageUptodate(page);
> +
> + unlock_page(page);
> + }
> + if (bio->bi_private)
> + put_read_callbacks_ctx(bio->bi_private);
> + bio_put(bio);
> +}
> +EXPORT_SYMBOL(end_read_callbacks);
end_read_callbacks() is only called by read_callbacks() just below, so it should
be 'static'.
> +
> +struct read_callbacks_ctx *get_read_callbacks_ctx(struct inode *inode,
> + struct bio *bio,
> + pgoff_t index)
> +{
> + unsigned int read_callbacks_steps = 0;
Rename 'read_callbacks_steps' => 'enabled_steps', since it's clear from context.
> + struct read_callbacks_ctx *ctx = NULL;
> +
> + if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode))
> + read_callbacks_steps |= 1 << STEP_DECRYPT;
> +#ifdef CONFIG_FS_VERITY
> + if (inode->i_verity_info != NULL &&
> +
[f2fs-dev] [PATCH V2 02/13] Consolidate "read callbacks" into a new file
The "read callbacks" code is used by both Ext4 and F2FS. Hence to
remove duplicity, this commit moves the code into
include/linux/read_callbacks.h and fs/read_callbacks.c.
The corresponding decrypt and verity "work" functions have been moved
inside fscrypt and fsverity sources. With these in place, the read
callbacks code now has to just invoke enqueue functions provided by
fscrypt and fsverity.
Signed-off-by: Chandan Rajendra
---
fs/Kconfig | 4 +
fs/Makefile| 4 +
fs/crypto/Kconfig | 1 +
fs/crypto/bio.c| 23 ++---
fs/crypto/crypto.c | 17 +--
fs/crypto/fscrypt_private.h| 3 +
fs/ext4/ext4.h | 2 -
fs/ext4/readpage.c | 183 +
fs/ext4/super.c| 9 +-
fs/f2fs/data.c | 148 --
fs/f2fs/super.c| 9 +-
fs/read_callbacks.c| 136
fs/verity/Kconfig | 1 +
fs/verity/verify.c | 12 +++
include/linux/fscrypt.h| 20 +---
include/linux/read_callbacks.h | 21
16 files changed, 251 insertions(+), 342 deletions(-)
create mode 100644 fs/read_callbacks.c
create mode 100644 include/linux/read_callbacks.h
diff --git a/fs/Kconfig b/fs/Kconfig
index 97f9eb8df713..03084f2dbeaf 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -308,6 +308,10 @@ config NFS_COMMON
depends on NFSD || NFS_FS || LOCKD
default y
+config FS_READ_CALLBACKS
+ bool
+ default n
+
source "net/sunrpc/Kconfig"
source "fs/ceph/Kconfig"
source "fs/cifs/Kconfig"
diff --git a/fs/Makefile b/fs/Makefile
index 9dd2186e74b5..e0c0fce8cf40 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -21,6 +21,10 @@ else
obj-y += no-block.o
endif
+ifeq ($(CONFIG_FS_READ_CALLBACKS),y)
+obj-y += read_callbacks.o
+endif
+
obj-$(CONFIG_PROC_FS) += proc_namespace.o
obj-y += notify/
diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index f0de238000c0..163c328bcbd4 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -8,6 +8,7 @@ config FS_ENCRYPTION
select CRYPTO_CTS
select CRYPTO_SHA256
select KEYS
+ select FS_READ_CALLBACKS
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 5759bcd018cd..27f5618174f2 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -24,6 +24,8 @@
#include
#include
#include
+#include
+
#include "fscrypt_private.h"
static void __fscrypt_decrypt_bio(struct bio *bio, bool done)
@@ -54,24 +56,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 read_callbacks_ctx *ctx =
+ container_of(work, struct read_callbacks_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(>r.work, completion_pages);
- ctx->r.bio = bio;
- fscrypt_enqueue_decrypt_work(>r.work);
+ read_callbacks(ctx);
}
-EXPORT_SYMBOL(fscrypt_enqueue_decrypt_bio);
void fscrypt_pullback_bio_page(struct page **page, bool restore)
{
@@ -87,7 +80,7 @@ void fscrypt_pullback_bio_page(struct page **page, bool
restore)
ctx = (struct fscrypt_ctx *)page_private(bounce_page);
/* restore control page */
- *page = ctx->w.control_page;
+ *page = ctx->control_page;
if (restore)
fscrypt_restore_control_page(bounce_page);
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 3fc84bf2b1e5..ffa9302a7351 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -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);
@@ -70,11 +71,11 @@ void fscrypt_release_ctx(struct fscrypt_ctx *ctx)
{
unsigned long flags;
- if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->w.bounce_page) {
- mempool_free(ctx->w.bounce_page, fscrypt_bounce_page_pool);
- ctx->w.bounce_page = NULL;
+ if (ctx->flags & FS_CTX_HAS_BOUNCE_BUFFER_FL && ctx->bounce_page) {
+ mempool_free(ctx->bounce_page, fscrypt_bounce_page_pool);
+ ctx->bounce_page = NULL;
}
- ctx->w.control_page = NULL;
+ ctx->control_page =
