From: Christoph Hellwig <hch@infradead.org> To: Christoph Hellwig <hch@infradead.org>, "Theodore Y. Ts'o" <tytso@mit.edu>, linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, Kim Boojin <boojin.kim@samsung.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Satya Tangirala <satyat@google.com>, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [PATCH v5 3/9] block: blk-crypto for Inline Encryption Date: Tue, 5 Nov 2019 07:39:57 -0800 [thread overview] Message-ID: <20191105153957.GA29320@infradead.org> (raw) In-Reply-To: <20191105015411.GB692@sol.localdomain> On Mon, Nov 04, 2019 at 06:01:17PM -0800, Eric Biggers wrote: > I think that "Severely bloating the per-I/O data structure" is an exaggeration, > since that it's only 32 bytes, and it isn't in struct bio directly but rather in > struct bio_crypt_ctx... Yes, and none of that is needed for the real inline crypto. And I think we can further reduce the overhead of bio_crypt_ctx once we have the basiscs sorted out. If we want to gain more traction we need to reduce the I/O to a minimum. > In any case, Satya, it might be a good idea to reorganize this patchset so that > it first adds all logic that's needed for "real" inline encryption support > (including the needed parts of blk-crypto.c), then adds the crypto API fallback > as a separate patch. That would separate the concerns more cleanly and make the > patchset easier to review, and make it easier to make the fallback > de-configurable or even remove it entirely if that turns out to be needed. Yes, that is a good idea. Not just in terms of patch, but also in terms of code organization. The current structure is pretty weird with 3 files that are mostly tighly integrated, except that one also has the software implementations. So what I think we need at a minimum is: - reoranizize that we have say block/blk-crypt.c for all the inline crypto infrastructure, and block/blk-crypy-sw.c for the actual software crypto implementation. - remove all the fields only needed for software crypto from bio_crypt_ctx, and instead clone the bio into a bioset with the additional fields only when we use the software implementation, so that there is no overhead for the hardware path.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org> To: Christoph Hellwig <hch@infradead.org>, "Theodore Y. Ts'o" <tytso@mit.edu>, linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, Kim Boojin <boojin.kim@samsung.com>, Kuohong Wang <kuohong.wang@mediatek.com>, Barani Muthukumaran <bmuthuku@qti.qualcomm.com>, Satya Tangirala <satyat@google.com>, linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH v5 3/9] block: blk-crypto for Inline Encryption Date: Tue, 5 Nov 2019 07:39:57 -0800 [thread overview] Message-ID: <20191105153957.GA29320@infradead.org> (raw) In-Reply-To: <20191105015411.GB692@sol.localdomain> On Mon, Nov 04, 2019 at 06:01:17PM -0800, Eric Biggers wrote: > I think that "Severely bloating the per-I/O data structure" is an exaggeration, > since that it's only 32 bytes, and it isn't in struct bio directly but rather in > struct bio_crypt_ctx... Yes, and none of that is needed for the real inline crypto. And I think we can further reduce the overhead of bio_crypt_ctx once we have the basiscs sorted out. If we want to gain more traction we need to reduce the I/O to a minimum. > In any case, Satya, it might be a good idea to reorganize this patchset so that > it first adds all logic that's needed for "real" inline encryption support > (including the needed parts of blk-crypto.c), then adds the crypto API fallback > as a separate patch. That would separate the concerns more cleanly and make the > patchset easier to review, and make it easier to make the fallback > de-configurable or even remove it entirely if that turns out to be needed. Yes, that is a good idea. Not just in terms of patch, but also in terms of code organization. The current structure is pretty weird with 3 files that are mostly tighly integrated, except that one also has the software implementations. So what I think we need at a minimum is: - reoranizize that we have say block/blk-crypt.c for all the inline crypto infrastructure, and block/blk-crypy-sw.c for the actual software crypto implementation. - remove all the fields only needed for software crypto from bio_crypt_ctx, and instead clone the bio into a bioset with the additional fields only when we use the software implementation, so that there is no overhead for the hardware path. _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2019-11-05 15:39 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-28 7:20 [PATCH v5 0/9] Inline Encryption Support Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-28 7:20 ` [PATCH v5 1/9] block: Keyslot Manager for Inline Encryption Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-31 18:04 ` Christoph Hellwig 2019-10-31 18:04 ` [f2fs-dev] " Christoph Hellwig 2019-10-28 7:20 ` [PATCH v5 2/9] block: Add encryption context to struct bio Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-31 18:16 ` Christoph Hellwig 2019-10-31 18:16 ` [f2fs-dev] " Christoph Hellwig 2019-10-28 7:20 ` [PATCH v5 3/9] block: blk-crypto for Inline Encryption Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-31 17:57 ` Christoph Hellwig 2019-10-31 17:57 ` [f2fs-dev] " Christoph Hellwig 2019-10-31 20:50 ` Theodore Y. Ts'o 2019-10-31 20:50 ` [f2fs-dev] " Theodore Y. Ts'o 2019-10-31 21:22 ` Christoph Hellwig 2019-10-31 21:22 ` [f2fs-dev] " Christoph Hellwig 2019-11-05 2:01 ` Eric Biggers 2019-11-05 2:01 ` [f2fs-dev] " Eric Biggers 2019-11-05 15:39 ` Christoph Hellwig [this message] 2019-11-05 15:39 ` Christoph Hellwig 2019-10-28 7:20 ` [PATCH v5 4/9] scsi: ufs: UFS driver v2.1 spec crypto additions Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-28 7:20 ` [PATCH v5 5/9] scsi: ufs: UFS crypto API Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-31 18:23 ` Christoph Hellwig 2019-10-31 18:23 ` [f2fs-dev] " Christoph Hellwig 2019-10-28 7:20 ` [PATCH v5 6/9] scsi: ufs: Add inline encryption support to UFS Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-31 18:26 ` Christoph Hellwig 2019-10-31 18:26 ` [f2fs-dev] " Christoph Hellwig 2019-10-28 7:20 ` [PATCH v5 7/9] fscrypt: add inline encryption support Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-31 18:32 ` Christoph Hellwig 2019-10-31 18:32 ` [f2fs-dev] " Christoph Hellwig 2019-10-31 20:21 ` Eric Biggers 2019-10-31 20:21 ` [f2fs-dev] " Eric Biggers 2019-10-31 21:21 ` Christoph Hellwig 2019-10-31 21:21 ` [f2fs-dev] " Christoph Hellwig 2019-10-31 22:25 ` Eric Biggers 2019-10-31 22:25 ` [f2fs-dev] " Eric Biggers 2019-11-05 0:15 ` Christoph Hellwig 2019-11-05 0:15 ` [f2fs-dev] " Christoph Hellwig 2019-11-05 1:03 ` Eric Biggers 2019-11-05 1:03 ` [f2fs-dev] " Eric Biggers 2019-11-05 3:12 ` Eric Biggers 2019-11-05 3:12 ` [f2fs-dev] " Eric Biggers 2019-10-28 7:20 ` [PATCH v5 8/9] f2fs: " Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel 2019-10-31 17:14 ` Jaegeuk Kim 2019-10-31 17:14 ` [f2fs-dev] " Jaegeuk Kim 2019-10-28 7:20 ` [PATCH v5 9/9] ext4: " Satya Tangirala 2019-10-28 7:20 ` [f2fs-dev] " Satya Tangirala via Linux-f2fs-devel
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=20191105153957.GA29320@infradead.org \ --to=hch@infradead.org \ --cc=bmuthuku@qti.qualcomm.com \ --cc=boojin.kim@samsung.com \ --cc=kuohong.wang@mediatek.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-fscrypt@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-scsi@vger.kernel.org \ --cc=satyat@google.com \ --cc=tytso@mit.edu \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.