Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: Chandan Rajendra <chandan@linux.vnet.ibm.com>
Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org, tytso@mit.edu
Subject: Re: [RFC PATCH 2/8] fs/buffer.c: make some functions non-static
Date: Tue, 16 Jan 2018 18:14:10 -0800	[thread overview]
Message-ID: <20180117021410.GB4477@zzz.localdomain> (raw)
In-Reply-To: <20180112141129.27507-3-chandan@linux.vnet.ibm.com>

Hi Chandan,

On Fri, Jan 12, 2018 at 07:41:23PM +0530, Chandan Rajendra wrote:
> The functions end_buffer_async_read(), block_size_bits() and
> create_page_buffers() will be needed by ext4 to implement encryption for
> blocksize < pagesize scenario.
> 

These all need to have EXPORT_SYMBOL() added, otherwise they won't be usable
when ext4 is built as a module.

block_size_bits() isn't actually needed as you can just use ->i_blkbits.

For the remaining two functions being exposed you need to explain why they are
*really* needed -- to create a version of block_read_full_page() that decrypts
the data after reading it.  Which is very ugly, and involves copy-and-pasting
quite a bit of code.

It's true that we've already effectively copy+pasted mpage_readpages() into ext4
(fs/ext4/readpage.c) for essentially the same reason, so your approach isn't
really any worse.  But did you consider instead cleaning things up by making the
generic code support encryption?  We now have the IS_ENCRYPTED() macro which
allows generic code to efficiently tell whether the file is encrypted or not.  I
think the only problem is that the code in fs/crypto/ can be built as a module,
so currently it can't be called directly by core code.  So we could either
change that and start requiring that fscrypt be built-in, or we could have the
filesystem pass an (optional) decryption callback to the generic code.

Or at the very least we could put the "encryption-aware" versions of
mpages_readpages() and block_read_full_page() into fs/crypto/ rather than in
fs/ext4/, so that they could be used by other filesystems in the future
(although currently f2fs and ubifs won't need them).

Eric

  parent reply	other threads:[~2018-01-17  2:14 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-12 14:11 [RFC PATCH 0/8] Ext4 encryption support for blocksize < pagesize Chandan Rajendra
2018-01-12 14:11 ` [RFC PATCH 1/8] ext4: use EXT4_INODE_ENCRYPT flag to detect encrypted bio Chandan Rajendra
2018-01-12 19:04   ` Randy Dunlap
2018-01-13  5:22     ` Chandan Rajendra
2018-01-12 14:11 ` [RFC PATCH 2/8] fs/buffer.c: make some functions non-static Chandan Rajendra
2018-01-12 14:38   ` Matthew Wilcox
2018-01-13  5:25     ` Chandan Rajendra
2018-01-17  2:14   ` Eric Biggers [this message]
2018-01-12 14:11 ` [RFC PATCH 3/8] ext4: decrypt all contiguous blocks in a page Chandan Rajendra
2018-01-17  2:18   ` Eric Biggers
2018-01-12 14:11 ` [RFC PATCH 4/8] ext4: decrypt all boundary blocks when doing buffered write Chandan Rajendra
2018-01-17  2:22   ` Eric Biggers
2018-01-12 14:11 ` [RFC PATCH 5/8] ext4: decrypt the block that needs to be partially zeroed Chandan Rajendra
2018-01-17  2:23   ` Eric Biggers
2018-01-12 14:11 ` [RFC PATCH 6/8] ext4: encrypt blocks whose size is less than page size Chandan Rajendra
2018-01-17  2:33   ` Eric Biggers
2018-01-12 14:11 ` [RFC PATCH 7/8] ext4: decrypt " Chandan Rajendra
2018-01-17  2:39   ` Eric Biggers
2018-01-12 14:11 ` [RFC PATCH 8/8] ext4: enable encryption for blocksize " Chandan Rajendra
2018-01-17  2:40   ` Eric Biggers
2018-01-17 13:42     ` Chandan Rajendra
2018-01-12 19:07 ` [RFC PATCH 0/8] Ext4 encryption support for blocksize < pagesize Randy Dunlap
2018-01-13  5:28   ` Chandan Rajendra
2018-01-13 12:48     ` Matthew Wilcox
2018-01-13 18:20       ` Randy Dunlap
2018-01-17  2:10 ` Eric Biggers
2018-01-17 13:43   ` Chandan Rajendra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180117021410.GB4477@zzz.localdomain \
    --to=ebiggers3@gmail.com \
    --cc=chandan@linux.vnet.ibm.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tytso@mit.edu \
    --subject='Re: [RFC PATCH 2/8] fs/buffer.c: make some functions non-static' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).