Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Chandan Rajendra <chandan@linux.ibm.com>
Cc: ebiggers@kernel.org, linux-fsdevel@vger.kernel.org,
	linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-fscrypt@vger.kernel.org, chandanrmail@gmail.com,
	adilger.kernel@dilger.ca, jaegeuk@kernel.org, yuchao0@huawei.com,
	hch@infradead.org
Subject: Re: [PATCH V4 5/8] f2fs: Use read_callbacks for decrypting file data
Date: Tue, 20 Aug 2019 12:38:37 -0400
Message-ID: <20190820163837.GD10232@mit.edu> (raw)
In-Reply-To: <1652707.8YmLLlegLt@localhost.localdomain>

On Tue, Aug 20, 2019 at 10:35:29AM +0530, Chandan Rajendra wrote:
> Looks like F2FS requires a lot more flexiblity than what can be offered by
> read callbacks i.e.
> 
> 1. F2FS wants to make use of its own workqueue for decryption, verity and
>    decompression.
> 2. F2FS' decompression code is not an FS independent entity like fscrypt and
>    fsverity. Hence they would need Filesystem specific callback functions to
>    be invoked from "read callbacks". 
> 
> Hence I would suggest that we should drop F2FS changes made in this
> patchset. Please let me know your thoughts on this.

That's probably the best way to go for now.  My one concern is that it
means that only ext4 will be using your framework.  I could imagine
that some people might argue that should just move the callback scheme
into ext4 code as opposed to leaving it in fscrypt --- at least until
we can find other file systems where we can show that it will be
useful for those other file systems.

(Perhaps a useful experiment would be to have someone implement patches
to support fscrypt and fsverity in ext2 --- the patch might or might
not be accepted for upstream inclusion, but it would be useful to
demonstrate how easy it is to add fscrypt and fsverity.)

The other thing to consider is that there has been some discussion
about adding generalized support for I/O submission to the iomap
library.  It might be that if that work is accepted, support for
fscrypt and fsverity would be a requirement for ext4 to use that
portion of iomap's functionality.  So in that eventuality, it might be
that we'll want to move your read callbacks code into iomap, or we'll
need to rework the read callbacks code so it can work with iomap.

But this is all work for the future.  I'm a firm believe that the
perfect should not be the enemy of the good, and that none of this
should be a fundamental obstacle in having your code upstream.

Cheers,

					- Ted



  parent reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-16  6:17 [PATCH V4 0/8] Consolidate FS read I/O callbacks code Chandan Rajendra
2019-08-16  6:17 ` [PATCH V4 1/8] buffer_head: Introduce BH_Read_Cb flag Chandan Rajendra
2019-08-16  6:17 ` [PATCH V4 2/8] FS: Introduce read callbacks Chandan Rajendra
2019-08-16  6:17 ` [PATCH V4 3/8] fs/mpage.c: Integrate " Chandan Rajendra
2019-08-16  6:18 ` [PATCH V4 4/8] fs/buffer.c: add decryption support via read_callbacks Chandan Rajendra
2019-08-16  6:18 ` [PATCH V4 5/8] f2fs: Use read_callbacks for decrypting file data Chandan Rajendra
2019-08-18 13:45   ` [f2fs-dev] " Chao Yu
2019-08-19 13:33     ` Chandan Rajendra
2019-08-20  7:43       ` Chao Yu
2019-08-20  5:05   ` Chandan Rajendra
2019-08-20  5:12     ` Gao Xiang
2019-08-20  5:16       ` Gao Xiang
2019-08-20 16:25       ` Theodore Y. Ts'o
2019-08-20 17:07         ` Gao Xiang
2019-08-20 16:38     ` Theodore Y. Ts'o [this message]
2019-08-20 17:31       ` Jaegeuk Kim
2019-08-21  2:04         ` Chandan Rajendra
2019-08-16  6:18 ` [PATCH V4 6/8] ext4: Wire up ext4_readpage[s] to use mpage_readpage[s] Chandan Rajendra
2019-08-16  6:18 ` [PATCH V4 7/8] ext4: Enable encryption for subpage-sized blocks Chandan Rajendra
2019-08-16  6:18 ` [PATCH V4 8/8] fscrypt: remove struct fscrypt_ctx Chandan Rajendra

Reply instructions:

You may reply publically 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=20190820163837.GD10232@mit.edu \
    --to=tytso@mit.edu \
    --cc=adilger.kernel@dilger.ca \
    --cc=chandan@linux.ibm.com \
    --cc=chandanrmail@gmail.com \
    --cc=ebiggers@kernel.org \
    --cc=hch@infradead.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=yuchao0@huawei.com \
    /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

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org linux-fsdevel@archiver.kernel.org
	public-inbox-index linux-fsdevel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox