Re: [f2fs-dev] [PATCH V2 07/13] Add decryption support for sub-pagesized blocks
Hi Eric, On Tuesday, April 30, 2019 6:08:18 AM IST Eric Biggers wrote: > On Sun, Apr 28, 2019 at 10:01:15AM +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. In the corresponding endio, loop across all the blocks mapped by the > >page, decrypting each block in turn. > > > > Signed-off-by: Chandan Rajendra > > --- > > fs/buffer.c| 83 +- > > fs/crypto/bio.c| 50 +--- > > fs/crypto/crypto.c | 19 +++- > > fs/f2fs/data.c | 2 +- > > fs/mpage.c | 2 +- > > fs/read_callbacks.c| 53 ++ > > include/linux/buffer_head.h| 1 + > > include/linux/read_callbacks.h | 5 +- > > 8 files changed, 154 insertions(+), 61 deletions(-) > > > > diff --git a/fs/buffer.c b/fs/buffer.c > > index ce357602f471..f324727e24bb 100644 > > --- a/fs/buffer.c > > +++ b/fs/buffer.c > > @@ -45,6 +45,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > static int fsync_buffers_list(spinlock_t *lock, struct list_head *list); > > @@ -245,11 +246,7 @@ __find_get_block_slow(struct block_device *bdev, > > sector_t block) > > return ret; > > } > > > > -/* > > - * I/O completion handler for block_read_full_page() - pages > > - * which come unlocked at the end of I/O. > > - */ > > -static void end_buffer_async_read(struct buffer_head *bh, int uptodate) > > +void end_buffer_page_read(struct buffer_head *bh) > > I think __end_buffer_async_read() would be a better name, since the *page* > isn't > necessarily done yet. > > > { > > unsigned long flags; > > struct buffer_head *first; > > @@ -257,17 +254,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 > > @@ -305,6 +292,44 @@ static void end_buffer_async_read(struct buffer_head > > *bh, int uptodate) > > local_irq_restore(flags); > > return; > > } > > +EXPORT_SYMBOL(end_buffer_page_read); > > No need for EXPORT_SYMBOL() here, as this is only called by built-in code. > > > + > > +/* > > + * I/O completion handler for block_read_full_page() - pages > > + * which come unlocked at the end of I/O. > > + */ > > This comment is no longer correct. Change to something like: > > /* > * 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 defined(CONFIG_FS_ENCRYPTION) || defined(CONFIG_FS_VERITY) > > + if (uptodate && bh->b_private) { > > + struct read_callbacks_ctx *ctx = bh->b_private; > > + > > + read_callbacks(ctx); > > + return; > > + } > > + > > + if (bh->b_private) { > > + struct read_callbacks_ctx *ctx = bh->b_private; > > + > > + WARN_ON(uptodate); > > + put_read_callbacks_ctx(ctx); > > + } > > +#endif > > These details should be handled in read_callbacks code, not here. AFAICS, all > you need is a function read_callbacks_end_bh() that returns a bool indicating > whether it handled the buffer_head or not: > > static void end_buffer_async_read(struct buffer_head *bh, int uptodate) > { > BUG_ON(!buffer_async_read(bh)); > > if (read_callbacks_end_bh(bh, uptodate)) > return; > > page = bh->b_page; > ... > } > > Then read_callbacks_end_bh() would check ->b_private and uptodate, and call > read_callbacks() or put_read_callbacks_ctx() as appropriate. When > CONFIG_FS_READ_CALLBACKS=n it would be a stub that always returns false. > > > + page = bh->b_page; > [...] > > > } > > @@ -2292,11 +2323,21 @@ int block_read_full_page(struct page *page, > > get_block_t *get_block) > > * the underlying blockdev brought it uptodate (the sct fix). > > */ > > for (i = 0; i < nr; i++) { > > - bh = arr[i]; > > - if (buffer_uptodate(bh)) > > + bh = arr[i].bh; > > + if (buffer_uptodate(bh)) { > > end_buffer_async_read(bh, 1); > > - else > > + } else { > > +#if
Re: [f2fs-dev] [PATCH V2 07/13] Add decryption support for sub-pagesized blocks
On Sun, Apr 28, 2019 at 10:01:15AM +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. In the corresponding endio, loop across all the blocks mapped by the >page, decrypting each block in turn. > > Signed-off-by: Chandan Rajendra > --- > fs/buffer.c| 83 +- > fs/crypto/bio.c| 50 +--- > fs/crypto/crypto.c | 19 +++- > fs/f2fs/data.c | 2 +- > fs/mpage.c | 2 +- > fs/read_callbacks.c| 53 ++ > include/linux/buffer_head.h| 1 + > include/linux/read_callbacks.h | 5 +- > 8 files changed, 154 insertions(+), 61 deletions(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index ce357602f471..f324727e24bb 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -45,6 +45,7 @@ > #include > #include > #include > +#include > #include > > static int fsync_buffers_list(spinlock_t *lock, struct list_head *list); > @@ -245,11 +246,7 @@ __find_get_block_slow(struct block_device *bdev, > sector_t block) > return ret; > } > > -/* > - * I/O completion handler for block_read_full_page() - pages > - * which come unlocked at the end of I/O. > - */ > -static void end_buffer_async_read(struct buffer_head *bh, int uptodate) > +void end_buffer_page_read(struct buffer_head *bh) I think __end_buffer_async_read() would be a better name, since the *page* isn't necessarily done yet. > { > unsigned long flags; > struct buffer_head *first; > @@ -257,17 +254,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 > @@ -305,6 +292,44 @@ static void end_buffer_async_read(struct buffer_head > *bh, int uptodate) > local_irq_restore(flags); > return; > } > +EXPORT_SYMBOL(end_buffer_page_read); No need for EXPORT_SYMBOL() here, as this is only called by built-in code. > + > +/* > + * I/O completion handler for block_read_full_page() - pages > + * which come unlocked at the end of I/O. > + */ This comment is no longer correct. Change to something like: /* * 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 defined(CONFIG_FS_ENCRYPTION) || defined(CONFIG_FS_VERITY) > + if (uptodate && bh->b_private) { > + struct read_callbacks_ctx *ctx = bh->b_private; > + > + read_callbacks(ctx); > + return; > + } > + > + if (bh->b_private) { > + struct read_callbacks_ctx *ctx = bh->b_private; > + > + WARN_ON(uptodate); > + put_read_callbacks_ctx(ctx); > + } > +#endif These details should be handled in read_callbacks code, not here. AFAICS, all you need is a function read_callbacks_end_bh() that returns a bool indicating whether it handled the buffer_head or not: static void end_buffer_async_read(struct buffer_head *bh, int uptodate) { BUG_ON(!buffer_async_read(bh)); if (read_callbacks_end_bh(bh, uptodate)) return; page = bh->b_page; ... } Then read_callbacks_end_bh() would check ->b_private and uptodate, and call read_callbacks() or put_read_callbacks_ctx() as appropriate. When CONFIG_FS_READ_CALLBACKS=n it would be a stub that always returns false. > + page = bh->b_page; [...] > } > @@ -2292,11 +2323,21 @@ int block_read_full_page(struct page *page, > get_block_t *get_block) >* the underlying blockdev brought it uptodate (the sct fix). >*/ > for (i = 0; i < nr; i++) { > - bh = arr[i]; > - if (buffer_uptodate(bh)) > + bh = arr[i].bh; > + if (buffer_uptodate(bh)) { > end_buffer_async_read(bh, 1); > - else > + } else { > +#if defined(CONFIG_FS_ENCRYPTION) || defined(CONFIG_FS_VERITY) > + struct read_callbacks_ctx *ctx; > + > + ctx = get_read_callbacks_ctx(inode, NULL, bh, > arr[i].blk_nr); > + if (WARN_ON(IS_ERR(ctx))) { > +
[f2fs-dev] [PATCH V2 07/13] Add decryption support for sub-pagesized blocks
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. In the corresponding endio, loop across all the blocks mapped by the page, decrypting each block in turn. Signed-off-by: Chandan Rajendra --- fs/buffer.c| 83 +- fs/crypto/bio.c| 50 +--- fs/crypto/crypto.c | 19 +++- fs/f2fs/data.c | 2 +- fs/mpage.c | 2 +- fs/read_callbacks.c| 53 ++ include/linux/buffer_head.h| 1 + include/linux/read_callbacks.h | 5 +- 8 files changed, 154 insertions(+), 61 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index ce357602f471..f324727e24bb 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -45,6 +45,7 @@ #include #include #include +#include #include static int fsync_buffers_list(spinlock_t *lock, struct list_head *list); @@ -245,11 +246,7 @@ __find_get_block_slow(struct block_device *bdev, sector_t block) return ret; } -/* - * I/O completion handler for block_read_full_page() - pages - * which come unlocked at the end of I/O. - */ -static void end_buffer_async_read(struct buffer_head *bh, int uptodate) +void end_buffer_page_read(struct buffer_head *bh) { unsigned long flags; struct buffer_head *first; @@ -257,17 +254,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 @@ -305,6 +292,44 @@ static void end_buffer_async_read(struct buffer_head *bh, int uptodate) local_irq_restore(flags); return; } +EXPORT_SYMBOL(end_buffer_page_read); + +/* + * 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) +{ + struct page *page; + + BUG_ON(!buffer_async_read(bh)); + +#if defined(CONFIG_FS_ENCRYPTION) || defined(CONFIG_FS_VERITY) + if (uptodate && bh->b_private) { + struct read_callbacks_ctx *ctx = bh->b_private; + + read_callbacks(ctx); + return; + } + + if (bh->b_private) { + struct read_callbacks_ctx *ctx = bh->b_private; + + WARN_ON(uptodate); + put_read_callbacks_ctx(ctx); + } +#endif + 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_page_read(bh); +} /* * Completion handler for block_write_full_page() - pages which are unlocked @@ -2220,7 +2245,11 @@ int block_read_full_page(struct page *page, get_block_t *get_block) { struct inode *inode = page->mapping->host; sector_t iblock, lblock; - struct buffer_head *bh, *head, *arr[MAX_BUF_PER_PAGE]; + struct buffer_head *bh, *head; + struct { + sector_t blk_nr; + struct buffer_head *bh; + } arr[MAX_BUF_PER_PAGE]; unsigned int blocksize, bbits; int nr, i; int fully_mapped = 1; @@ -2262,7 +2291,9 @@ int block_read_full_page(struct page *page, get_block_t *get_block) if (buffer_uptodate(bh)) continue; } - arr[nr++] = bh; + arr[nr].blk_nr = iblock; + arr[nr].bh = bh; + ++nr; } while (i++, iblock++, (bh = bh->b_this_page) != head); if (fully_mapped) @@ -2281,7 +2312,7 @@ int block_read_full_page(struct page *page, get_block_t *get_block) /* Stage two: lock the buffers */ for (i = 0; i < nr; i++) { - bh = arr[i]; + bh = arr[i].bh; lock_buffer(bh); mark_buffer_async_read(bh); } @@ -2292,11 +2323,21 @@ int block_read_full_page(struct page *page, get_block_t *get_block) * the underlying blockdev brought it uptodate (the sct fix). */ for (i = 0; i < nr; i++) { - bh = arr[i]; - if (buffer_uptodate(bh)) + bh = arr[i].bh; + if (buffer_uptodate(bh)) { end_buffer_async_read(bh, 1); - else +