All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 0/5] add support for direct I/O with fscrypt using blk-crypto
@ 2022-01-28 23:39 ` Eric Biggers
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner

[Note: I'm planning to send a patchset adding STATX_DIRECTIO as was
discussed on v10, but that will be a separate patchset.]

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

This series adds support for this.  There are multiple use cases for DIO
on encrypted files, but avoiding double caching on loopback devices
located in an encrypted directory is the main one currently.

v1 through v9 of this series were sent out by Satya Tangirala.  I've
cleaned up a few things since Satya's last version
(https://lore.kernel.org/all/20210604210908.2105870-1-satyat@google.com/T/#u).
But more notably, I've made a couple simplifications.

First, since f2fs has now been converted to use iomap for DIO, I've
dropped the patch which added fscrypt support to fs/direct-io.c.

Second, I've returned to the original design where DIO requests must be
fully aligned to the FS block size in terms of file position, length,
and memory buffers.  Satya previously was pursuing a slightly different
design, where the memory buffers (but not the file position and length)
were allowed to be aligned to just the block device logical block size.
This was at the request of Dave Chinner on v4 and v6 of the patchset
(https://lore.kernel.org/linux-fscrypt/20200720233739.824943-1-satyat@google.com/T/#u
and
https://lore.kernel.org/linux-fscrypt/20200724184501.1651378-1-satyat@google.com/T/#u).

I believe that approach is a dead end, for two reasons.  First, it
necessarily causes it to be possible that crypto data units span bvecs.
Splits cannot occur at such locations; however the block layer currently
assumes that bios can be split at any bvec boundary.  Changing that is
quite difficult, as Satya's v9 patchset demonstrated.  This is not an
issue if we require FS block aligned buffers instead.  Second, it
doesn't change the fact that FS block alignment is still required for
the file position and I/O length; this is unavoidable due to the
granularity of encryption being the FS block size.  So, it seems that
relaxing the memory buffer alignment requirement wouldn't make things
meaningfully easier for applications, which raises the question of why
we would bother with it in the first place.

Christoph Hellwig also said that he much prefers that fscrypt DIO be
supported without sector-only alignment to start:
https://lore.kernel.org/r/YPu+88KReGlt94o3@infradead.org

Given the above, as far as I know the only remaining objection to this
patchset would be that DIO constraints aren't sufficiently discoverable
by userspace.  Now, to put this in context, this is a longstanding issue
with all Linux filesystems, except XFS which has XFS_IOC_DIOINFO.  It's
not specific to this feature, and it doesn't actually seem to be too
important in practice; many other filesystem features place constraints
on DIO, and f2fs even *only* allows fully FS block size aligned DIO.
(And for better or worse, many systems using fscrypt already have
out-of-tree patches that enable DIO support, and people don't seem to
have trouble with the FS block size alignment requirement.)

To address the issue of DIO constraints being insufficiently
discoverable, I plan to make statx() expose this information.

This series applies to v5.17-rc1.

Changed v10 => v11:
  * Changed fscrypt_dio_unsupported() back to fscrypt_dio_supported().
  * Removed a mention of f2fs from fscrypt_dio_supported().
  * Added Reviewed-by and Acked-by tags, including a couple from earlier
    I had dropped due to the renaming of fscrypt_dio_supported().
  * In fscrypt_limit_io_blocks(), don't load i_crypt_info until it's
    known to be valid, to avoid confusion as is done elsewhere.

Eric Biggers (5):
  fscrypt: add functions for direct I/O support
  iomap: support direct I/O with fscrypt using blk-crypto
  ext4: support direct I/O with fscrypt using blk-crypto
  f2fs: support direct I/O with fscrypt using blk-crypto
  fscrypt: update documentation for direct I/O support

 Documentation/filesystems/fscrypt.rst | 25 ++++++-
 fs/crypto/crypto.c                    |  8 +++
 fs/crypto/inline_crypt.c              | 93 +++++++++++++++++++++++++++
 fs/ext4/file.c                        | 10 +--
 fs/ext4/inode.c                       |  7 ++
 fs/f2fs/data.c                        |  7 ++
 fs/f2fs/f2fs.h                        |  6 +-
 fs/iomap/direct-io.c                  |  6 ++
 include/linux/fscrypt.h               | 18 ++++++
 9 files changed, 173 insertions(+), 7 deletions(-)


base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07
-- 
2.35.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [f2fs-dev] [PATCH v11 0/5] add support for direct I/O with fscrypt using blk-crypto
@ 2022-01-28 23:39 ` Eric Biggers
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: Theodore Ts'o, Darrick J . Wong, Dave Chinner,
	linux-f2fs-devel, linux-xfs, linux-fsdevel, Jaegeuk Kim,
	linux-ext4, Christoph Hellwig

[Note: I'm planning to send a patchset adding STATX_DIRECTIO as was
discussed on v10, but that will be a separate patchset.]

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

This series adds support for this.  There are multiple use cases for DIO
on encrypted files, but avoiding double caching on loopback devices
located in an encrypted directory is the main one currently.

v1 through v9 of this series were sent out by Satya Tangirala.  I've
cleaned up a few things since Satya's last version
(https://lore.kernel.org/all/20210604210908.2105870-1-satyat@google.com/T/#u).
But more notably, I've made a couple simplifications.

First, since f2fs has now been converted to use iomap for DIO, I've
dropped the patch which added fscrypt support to fs/direct-io.c.

Second, I've returned to the original design where DIO requests must be
fully aligned to the FS block size in terms of file position, length,
and memory buffers.  Satya previously was pursuing a slightly different
design, where the memory buffers (but not the file position and length)
were allowed to be aligned to just the block device logical block size.
This was at the request of Dave Chinner on v4 and v6 of the patchset
(https://lore.kernel.org/linux-fscrypt/20200720233739.824943-1-satyat@google.com/T/#u
and
https://lore.kernel.org/linux-fscrypt/20200724184501.1651378-1-satyat@google.com/T/#u).

I believe that approach is a dead end, for two reasons.  First, it
necessarily causes it to be possible that crypto data units span bvecs.
Splits cannot occur at such locations; however the block layer currently
assumes that bios can be split at any bvec boundary.  Changing that is
quite difficult, as Satya's v9 patchset demonstrated.  This is not an
issue if we require FS block aligned buffers instead.  Second, it
doesn't change the fact that FS block alignment is still required for
the file position and I/O length; this is unavoidable due to the
granularity of encryption being the FS block size.  So, it seems that
relaxing the memory buffer alignment requirement wouldn't make things
meaningfully easier for applications, which raises the question of why
we would bother with it in the first place.

Christoph Hellwig also said that he much prefers that fscrypt DIO be
supported without sector-only alignment to start:
https://lore.kernel.org/r/YPu+88KReGlt94o3@infradead.org

Given the above, as far as I know the only remaining objection to this
patchset would be that DIO constraints aren't sufficiently discoverable
by userspace.  Now, to put this in context, this is a longstanding issue
with all Linux filesystems, except XFS which has XFS_IOC_DIOINFO.  It's
not specific to this feature, and it doesn't actually seem to be too
important in practice; many other filesystem features place constraints
on DIO, and f2fs even *only* allows fully FS block size aligned DIO.
(And for better or worse, many systems using fscrypt already have
out-of-tree patches that enable DIO support, and people don't seem to
have trouble with the FS block size alignment requirement.)

To address the issue of DIO constraints being insufficiently
discoverable, I plan to make statx() expose this information.

This series applies to v5.17-rc1.

Changed v10 => v11:
  * Changed fscrypt_dio_unsupported() back to fscrypt_dio_supported().
  * Removed a mention of f2fs from fscrypt_dio_supported().
  * Added Reviewed-by and Acked-by tags, including a couple from earlier
    I had dropped due to the renaming of fscrypt_dio_supported().
  * In fscrypt_limit_io_blocks(), don't load i_crypt_info until it's
    known to be valid, to avoid confusion as is done elsewhere.

Eric Biggers (5):
  fscrypt: add functions for direct I/O support
  iomap: support direct I/O with fscrypt using blk-crypto
  ext4: support direct I/O with fscrypt using blk-crypto
  f2fs: support direct I/O with fscrypt using blk-crypto
  fscrypt: update documentation for direct I/O support

 Documentation/filesystems/fscrypt.rst | 25 ++++++-
 fs/crypto/crypto.c                    |  8 +++
 fs/crypto/inline_crypt.c              | 93 +++++++++++++++++++++++++++
 fs/ext4/file.c                        | 10 +--
 fs/ext4/inode.c                       |  7 ++
 fs/f2fs/data.c                        |  7 ++
 fs/f2fs/f2fs.h                        |  6 +-
 fs/iomap/direct-io.c                  |  6 ++
 include/linux/fscrypt.h               | 18 ++++++
 9 files changed, 173 insertions(+), 7 deletions(-)


base-commit: e783362eb54cd99b2cac8b3a9aeac942e6f6ac07
-- 
2.35.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v11 1/5] fscrypt: add functions for direct I/O support
  2022-01-28 23:39 ` [f2fs-dev] " Eric Biggers
@ 2022-01-28 23:39   ` Eric Biggers
  -1 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Satya Tangirala

From: Eric Biggers <ebiggers@google.com>

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

In preparation for supporting this, add the following functions:

- fscrypt_dio_supported() checks whether a DIO request is supported as
  far as encryption is concerned.  Encrypted files will only support DIO
  when inline encryption is used and the I/O request is properly
  aligned; this function checks these preconditions.

- fscrypt_limit_io_blocks() limits the length of a bio to avoid crossing
  a place in the file that a bio with an encryption context cannot
  cross due to a DUN discontiguity.  This function is needed by
  filesystems that use the iomap DIO implementation (which operates
  directly on logical ranges, so it won't use fscrypt_mergeable_bio())
  and that support FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32.

Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/crypto.c       |  8 ++++
 fs/crypto/inline_crypt.c | 93 ++++++++++++++++++++++++++++++++++++++++
 include/linux/fscrypt.h  | 18 ++++++++
 3 files changed, 119 insertions(+)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 4ef3f714046aa..4fcca79f39aeb 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -69,6 +69,14 @@ void fscrypt_free_bounce_page(struct page *bounce_page)
 }
 EXPORT_SYMBOL(fscrypt_free_bounce_page);
 
+/*
+ * Generate the IV for the given logical block number within the given file.
+ * For filenames encryption, lblk_num == 0.
+ *
+ * Keep this in sync with fscrypt_limit_io_blocks().  fscrypt_limit_io_blocks()
+ * needs to know about any IV generation methods where the low bits of IV don't
+ * simply contain the lblk_num (e.g., IV_INO_LBLK_32).
+ */
 void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
 			 const struct fscrypt_info *ci)
 {
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index c57bebfa48fea..93c2ca8580923 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -17,6 +17,7 @@
 #include <linux/buffer_head.h>
 #include <linux/sched/mm.h>
 #include <linux/slab.h>
+#include <linux/uio.h>
 
 #include "fscrypt_private.h"
 
@@ -315,6 +316,10 @@ EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx_bh);
  *
  * fscrypt_set_bio_crypt_ctx() must have already been called on the bio.
  *
+ * This function isn't required in cases where crypto-mergeability is ensured in
+ * another way, such as I/O targeting only a single file (and thus a single key)
+ * combined with fscrypt_limit_io_blocks() to ensure DUN contiguity.
+ *
  * Return: true iff the I/O is mergeable
  */
 bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
@@ -363,3 +368,91 @@ bool fscrypt_mergeable_bio_bh(struct bio *bio,
 	return fscrypt_mergeable_bio(bio, inode, next_lblk);
 }
 EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio_bh);
+
+/**
+ * fscrypt_dio_supported() - check whether a DIO (direct I/O) request is
+ *			     supported as far as encryption is concerned
+ * @iocb: the file and position the I/O is targeting
+ * @iter: the I/O data segment(s)
+ *
+ * Return: %true if there are no encryption constraints that prevent DIO from
+ *	   being supported; %false if DIO is unsupported.  (Note that in the
+ *	   %true case, the filesystem might have other, non-encryption-related
+ *	   constraints that prevent DIO from actually being supported.)
+ */
+bool fscrypt_dio_supported(struct kiocb *iocb, struct iov_iter *iter)
+{
+	const struct inode *inode = file_inode(iocb->ki_filp);
+	const unsigned int blocksize = i_blocksize(inode);
+
+	/* If the file is unencrypted, no veto from us. */
+	if (!fscrypt_needs_contents_encryption(inode))
+		return true;
+
+	/* We only support DIO with inline crypto, not fs-layer crypto. */
+	if (!fscrypt_inode_uses_inline_crypto(inode))
+		return false;
+
+	/*
+	 * Since the granularity of encryption is filesystem blocks, the file
+	 * position and total I/O length must be aligned to the filesystem block
+	 * size -- not just to the block device's logical block size as is
+	 * traditionally the case for DIO on many filesystems.
+	 *
+	 * We require that the user-provided memory buffers be filesystem block
+	 * aligned too.  It is simpler to have a single alignment value required
+	 * for all properties of the I/O, as is normally the case for DIO.
+	 * Also, allowing less aligned buffers would imply that data units could
+	 * cross bvecs, which would greatly complicate the I/O stack, which
+	 * assumes that bios can be split at any bvec boundary.
+	 */
+	if (!IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(iter), blocksize))
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(fscrypt_dio_supported);
+
+/**
+ * fscrypt_limit_io_blocks() - limit I/O blocks to avoid discontiguous DUNs
+ * @inode: the file on which I/O is being done
+ * @lblk: the block at which the I/O is being started from
+ * @nr_blocks: the number of blocks we want to submit starting at @lblk
+ *
+ * Determine the limit to the number of blocks that can be submitted in a bio
+ * targeting @lblk without causing a data unit number (DUN) discontiguity.
+ *
+ * This is normally just @nr_blocks, as normally the DUNs just increment along
+ * with the logical blocks.  (Or the file is not encrypted.)
+ *
+ * In rare cases, fscrypt can be using an IV generation method that allows the
+ * DUN to wrap around within logically contiguous blocks, and that wraparound
+ * will occur.  If this happens, a value less than @nr_blocks will be returned
+ * so that the wraparound doesn't occur in the middle of a bio, which would
+ * cause encryption/decryption to produce wrong results.
+ *
+ * Return: the actual number of blocks that can be submitted
+ */
+u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
+{
+	const struct fscrypt_info *ci;
+	u32 dun;
+
+	if (!fscrypt_inode_uses_inline_crypto(inode))
+		return nr_blocks;
+
+	if (nr_blocks <= 1)
+		return nr_blocks;
+
+	ci = inode->i_crypt_info;
+	if (!(fscrypt_policy_flags(&ci->ci_policy) &
+	      FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
+		return nr_blocks;
+
+	/* With IV_INO_LBLK_32, the DUN can wrap around from U32_MAX to 0. */
+
+	dun = ci->ci_hashed_ino + lblk;
+
+	return min_t(u64, nr_blocks, (u64)U32_MAX + 1 - dun);
+}
+EXPORT_SYMBOL_GPL(fscrypt_limit_io_blocks);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 91ea9477e9bd2..50d92d805bd8c 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -714,6 +714,10 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 bool fscrypt_mergeable_bio_bh(struct bio *bio,
 			      const struct buffer_head *next_bh);
 
+bool fscrypt_dio_supported(struct kiocb *iocb, struct iov_iter *iter);
+
+u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks);
+
 #else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
 
 static inline bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
@@ -742,6 +746,20 @@ static inline bool fscrypt_mergeable_bio_bh(struct bio *bio,
 {
 	return true;
 }
+
+static inline bool fscrypt_dio_supported(struct kiocb *iocb,
+					 struct iov_iter *iter)
+{
+	const struct inode *inode = file_inode(iocb->ki_filp);
+
+	return !fscrypt_needs_contents_encryption(inode);
+}
+
+static inline u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk,
+					  u64 nr_blocks)
+{
+	return nr_blocks;
+}
 #endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
 
 /**
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [f2fs-dev] [PATCH v11 1/5] fscrypt: add functions for direct I/O support
@ 2022-01-28 23:39   ` Eric Biggers
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: Satya Tangirala, Theodore Ts'o, Darrick J . Wong,
	Dave Chinner, linux-f2fs-devel, linux-xfs, linux-fsdevel,
	Jaegeuk Kim, linux-ext4, Christoph Hellwig

From: Eric Biggers <ebiggers@google.com>

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

In preparation for supporting this, add the following functions:

- fscrypt_dio_supported() checks whether a DIO request is supported as
  far as encryption is concerned.  Encrypted files will only support DIO
  when inline encryption is used and the I/O request is properly
  aligned; this function checks these preconditions.

- fscrypt_limit_io_blocks() limits the length of a bio to avoid crossing
  a place in the file that a bio with an encryption context cannot
  cross due to a DUN discontiguity.  This function is needed by
  filesystems that use the iomap DIO implementation (which operates
  directly on logical ranges, so it won't use fscrypt_mergeable_bio())
  and that support FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32.

Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/crypto.c       |  8 ++++
 fs/crypto/inline_crypt.c | 93 ++++++++++++++++++++++++++++++++++++++++
 include/linux/fscrypt.h  | 18 ++++++++
 3 files changed, 119 insertions(+)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 4ef3f714046aa..4fcca79f39aeb 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -69,6 +69,14 @@ void fscrypt_free_bounce_page(struct page *bounce_page)
 }
 EXPORT_SYMBOL(fscrypt_free_bounce_page);
 
+/*
+ * Generate the IV for the given logical block number within the given file.
+ * For filenames encryption, lblk_num == 0.
+ *
+ * Keep this in sync with fscrypt_limit_io_blocks().  fscrypt_limit_io_blocks()
+ * needs to know about any IV generation methods where the low bits of IV don't
+ * simply contain the lblk_num (e.g., IV_INO_LBLK_32).
+ */
 void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
 			 const struct fscrypt_info *ci)
 {
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index c57bebfa48fea..93c2ca8580923 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -17,6 +17,7 @@
 #include <linux/buffer_head.h>
 #include <linux/sched/mm.h>
 #include <linux/slab.h>
+#include <linux/uio.h>
 
 #include "fscrypt_private.h"
 
@@ -315,6 +316,10 @@ EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx_bh);
  *
  * fscrypt_set_bio_crypt_ctx() must have already been called on the bio.
  *
+ * This function isn't required in cases where crypto-mergeability is ensured in
+ * another way, such as I/O targeting only a single file (and thus a single key)
+ * combined with fscrypt_limit_io_blocks() to ensure DUN contiguity.
+ *
  * Return: true iff the I/O is mergeable
  */
 bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
@@ -363,3 +368,91 @@ bool fscrypt_mergeable_bio_bh(struct bio *bio,
 	return fscrypt_mergeable_bio(bio, inode, next_lblk);
 }
 EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio_bh);
+
+/**
+ * fscrypt_dio_supported() - check whether a DIO (direct I/O) request is
+ *			     supported as far as encryption is concerned
+ * @iocb: the file and position the I/O is targeting
+ * @iter: the I/O data segment(s)
+ *
+ * Return: %true if there are no encryption constraints that prevent DIO from
+ *	   being supported; %false if DIO is unsupported.  (Note that in the
+ *	   %true case, the filesystem might have other, non-encryption-related
+ *	   constraints that prevent DIO from actually being supported.)
+ */
+bool fscrypt_dio_supported(struct kiocb *iocb, struct iov_iter *iter)
+{
+	const struct inode *inode = file_inode(iocb->ki_filp);
+	const unsigned int blocksize = i_blocksize(inode);
+
+	/* If the file is unencrypted, no veto from us. */
+	if (!fscrypt_needs_contents_encryption(inode))
+		return true;
+
+	/* We only support DIO with inline crypto, not fs-layer crypto. */
+	if (!fscrypt_inode_uses_inline_crypto(inode))
+		return false;
+
+	/*
+	 * Since the granularity of encryption is filesystem blocks, the file
+	 * position and total I/O length must be aligned to the filesystem block
+	 * size -- not just to the block device's logical block size as is
+	 * traditionally the case for DIO on many filesystems.
+	 *
+	 * We require that the user-provided memory buffers be filesystem block
+	 * aligned too.  It is simpler to have a single alignment value required
+	 * for all properties of the I/O, as is normally the case for DIO.
+	 * Also, allowing less aligned buffers would imply that data units could
+	 * cross bvecs, which would greatly complicate the I/O stack, which
+	 * assumes that bios can be split at any bvec boundary.
+	 */
+	if (!IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(iter), blocksize))
+		return false;
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(fscrypt_dio_supported);
+
+/**
+ * fscrypt_limit_io_blocks() - limit I/O blocks to avoid discontiguous DUNs
+ * @inode: the file on which I/O is being done
+ * @lblk: the block at which the I/O is being started from
+ * @nr_blocks: the number of blocks we want to submit starting at @lblk
+ *
+ * Determine the limit to the number of blocks that can be submitted in a bio
+ * targeting @lblk without causing a data unit number (DUN) discontiguity.
+ *
+ * This is normally just @nr_blocks, as normally the DUNs just increment along
+ * with the logical blocks.  (Or the file is not encrypted.)
+ *
+ * In rare cases, fscrypt can be using an IV generation method that allows the
+ * DUN to wrap around within logically contiguous blocks, and that wraparound
+ * will occur.  If this happens, a value less than @nr_blocks will be returned
+ * so that the wraparound doesn't occur in the middle of a bio, which would
+ * cause encryption/decryption to produce wrong results.
+ *
+ * Return: the actual number of blocks that can be submitted
+ */
+u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
+{
+	const struct fscrypt_info *ci;
+	u32 dun;
+
+	if (!fscrypt_inode_uses_inline_crypto(inode))
+		return nr_blocks;
+
+	if (nr_blocks <= 1)
+		return nr_blocks;
+
+	ci = inode->i_crypt_info;
+	if (!(fscrypt_policy_flags(&ci->ci_policy) &
+	      FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
+		return nr_blocks;
+
+	/* With IV_INO_LBLK_32, the DUN can wrap around from U32_MAX to 0. */
+
+	dun = ci->ci_hashed_ino + lblk;
+
+	return min_t(u64, nr_blocks, (u64)U32_MAX + 1 - dun);
+}
+EXPORT_SYMBOL_GPL(fscrypt_limit_io_blocks);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 91ea9477e9bd2..50d92d805bd8c 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -714,6 +714,10 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 bool fscrypt_mergeable_bio_bh(struct bio *bio,
 			      const struct buffer_head *next_bh);
 
+bool fscrypt_dio_supported(struct kiocb *iocb, struct iov_iter *iter);
+
+u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks);
+
 #else /* CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
 
 static inline bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
@@ -742,6 +746,20 @@ static inline bool fscrypt_mergeable_bio_bh(struct bio *bio,
 {
 	return true;
 }
+
+static inline bool fscrypt_dio_supported(struct kiocb *iocb,
+					 struct iov_iter *iter)
+{
+	const struct inode *inode = file_inode(iocb->ki_filp);
+
+	return !fscrypt_needs_contents_encryption(inode);
+}
+
+static inline u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk,
+					  u64 nr_blocks)
+{
+	return nr_blocks;
+}
 #endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
 
 /**
-- 
2.35.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v11 2/5] iomap: support direct I/O with fscrypt using blk-crypto
  2022-01-28 23:39 ` [f2fs-dev] " Eric Biggers
@ 2022-01-28 23:39   ` Eric Biggers
  -1 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Satya Tangirala

From: Eric Biggers <ebiggers@google.com>

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

Add support for this to the iomap DIO implementation by calling
fscrypt_set_bio_crypt_ctx() to set encryption contexts on the bios.

Don't check for the rare case where a DUN (crypto data unit number)
discontiguity creates a boundary that bios must not cross.  Instead,
filesystems are expected to handle this in ->iomap_begin() by limiting
the length of the mapping so that iomap doesn't have to worry about it.

Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Acked-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/iomap/direct-io.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 03ea367df19a4..20325b3926fa3 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -6,6 +6,7 @@
 #include <linux/module.h>
 #include <linux/compiler.h>
 #include <linux/fs.h>
+#include <linux/fscrypt.h>
 #include <linux/pagemap.h>
 #include <linux/iomap.h>
 #include <linux/backing-dev.h>
@@ -179,11 +180,14 @@ static void iomap_dio_bio_end_io(struct bio *bio)
 static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
 		loff_t pos, unsigned len)
 {
+	struct inode *inode = file_inode(dio->iocb->ki_filp);
 	struct page *page = ZERO_PAGE(0);
 	int flags = REQ_SYNC | REQ_IDLE;
 	struct bio *bio;
 
 	bio = bio_alloc(GFP_KERNEL, 1);
+	fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
+				  GFP_KERNEL);
 	bio_set_dev(bio, iter->iomap.bdev);
 	bio->bi_iter.bi_sector = iomap_sector(&iter->iomap, pos);
 	bio->bi_private = dio;
@@ -310,6 +314,8 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
 		}
 
 		bio = bio_alloc(GFP_KERNEL, nr_pages);
+		fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
+					  GFP_KERNEL);
 		bio_set_dev(bio, iomap->bdev);
 		bio->bi_iter.bi_sector = iomap_sector(iomap, pos);
 		bio->bi_write_hint = dio->iocb->ki_hint;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [f2fs-dev] [PATCH v11 2/5] iomap: support direct I/O with fscrypt using blk-crypto
@ 2022-01-28 23:39   ` Eric Biggers
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: Satya Tangirala, Theodore Ts'o, Darrick J . Wong,
	Dave Chinner, linux-f2fs-devel, linux-xfs, linux-fsdevel,
	Jaegeuk Kim, linux-ext4, Christoph Hellwig

From: Eric Biggers <ebiggers@google.com>

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

Add support for this to the iomap DIO implementation by calling
fscrypt_set_bio_crypt_ctx() to set encryption contexts on the bios.

Don't check for the rare case where a DUN (crypto data unit number)
discontiguity creates a boundary that bios must not cross.  Instead,
filesystems are expected to handle this in ->iomap_begin() by limiting
the length of the mapping so that iomap doesn't have to worry about it.

Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Acked-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/iomap/direct-io.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 03ea367df19a4..20325b3926fa3 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -6,6 +6,7 @@
 #include <linux/module.h>
 #include <linux/compiler.h>
 #include <linux/fs.h>
+#include <linux/fscrypt.h>
 #include <linux/pagemap.h>
 #include <linux/iomap.h>
 #include <linux/backing-dev.h>
@@ -179,11 +180,14 @@ static void iomap_dio_bio_end_io(struct bio *bio)
 static void iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
 		loff_t pos, unsigned len)
 {
+	struct inode *inode = file_inode(dio->iocb->ki_filp);
 	struct page *page = ZERO_PAGE(0);
 	int flags = REQ_SYNC | REQ_IDLE;
 	struct bio *bio;
 
 	bio = bio_alloc(GFP_KERNEL, 1);
+	fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
+				  GFP_KERNEL);
 	bio_set_dev(bio, iter->iomap.bdev);
 	bio->bi_iter.bi_sector = iomap_sector(&iter->iomap, pos);
 	bio->bi_private = dio;
@@ -310,6 +314,8 @@ static loff_t iomap_dio_bio_iter(const struct iomap_iter *iter,
 		}
 
 		bio = bio_alloc(GFP_KERNEL, nr_pages);
+		fscrypt_set_bio_crypt_ctx(bio, inode, pos >> inode->i_blkbits,
+					  GFP_KERNEL);
 		bio_set_dev(bio, iomap->bdev);
 		bio->bi_iter.bi_sector = iomap_sector(iomap, pos);
 		bio->bi_write_hint = dio->iocb->ki_hint;
-- 
2.35.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v11 3/5] ext4: support direct I/O with fscrypt using blk-crypto
  2022-01-28 23:39 ` [f2fs-dev] " Eric Biggers
@ 2022-01-28 23:39   ` Eric Biggers
  -1 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Satya Tangirala

From: Eric Biggers <ebiggers@google.com>

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

Therefore, make ext4 support DIO on files that are using inline
encryption.  Since ext4 uses iomap for DIO, and fscrypt support was
already added to iomap DIO, this just requires two small changes:

- Let DIO proceed when supported, by checking fscrypt_dio_supported()
  instead of assuming that encrypted files never support DIO.

- In ext4_iomap_begin(), use fscrypt_limit_io_blocks() to limit the
  length of the mapping in the rare case where a DUN discontiguity
  occurs in the middle of an extent.  The iomap DIO implementation
  requires this, since it assumes that it can submit a bio covering (up
  to) the whole mapping, without checking fscrypt constraints itself.

Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Reviewed-by: Jaegeuk Kim <jaegeuk@kernel.org>
Acked-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/file.c  | 10 ++++++----
 fs/ext4/inode.c |  7 +++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 8cc11715518ac..8bd66cdc41be2 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -36,9 +36,11 @@
 #include "acl.h"
 #include "truncate.h"
 
-static bool ext4_dio_supported(struct inode *inode)
+static bool ext4_dio_supported(struct kiocb *iocb, struct iov_iter *iter)
 {
-	if (IS_ENABLED(CONFIG_FS_ENCRYPTION) && IS_ENCRYPTED(inode))
+	struct inode *inode = file_inode(iocb->ki_filp);
+
+	if (!fscrypt_dio_supported(iocb, iter))
 		return false;
 	if (fsverity_active(inode))
 		return false;
@@ -61,7 +63,7 @@ static ssize_t ext4_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
 		inode_lock_shared(inode);
 	}
 
-	if (!ext4_dio_supported(inode)) {
+	if (!ext4_dio_supported(iocb, to)) {
 		inode_unlock_shared(inode);
 		/*
 		 * Fallback to buffered I/O if the operation being performed on
@@ -509,7 +511,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	}
 
 	/* Fallback to buffered I/O if the inode does not support direct I/O. */
-	if (!ext4_dio_supported(inode)) {
+	if (!ext4_dio_supported(iocb, from)) {
 		if (ilock_shared)
 			inode_unlock_shared(inode);
 		else
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5f79d265d06a0..7af1bba34b8b8 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3409,6 +3409,13 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 	if (ret < 0)
 		return ret;
 out:
+	/*
+	 * When inline encryption is enabled, sometimes I/O to an encrypted file
+	 * has to be broken up to guarantee DUN contiguity.  Handle this by
+	 * limiting the length of the mapping returned.
+	 */
+	map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
+
 	ext4_set_iomap(inode, iomap, &map, offset, length, flags);
 
 	return 0;
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [f2fs-dev] [PATCH v11 3/5] ext4: support direct I/O with fscrypt using blk-crypto
@ 2022-01-28 23:39   ` Eric Biggers
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: Satya Tangirala, Theodore Ts'o, Darrick J . Wong,
	Dave Chinner, linux-f2fs-devel, linux-xfs, linux-fsdevel,
	Jaegeuk Kim, linux-ext4, Christoph Hellwig

From: Eric Biggers <ebiggers@google.com>

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

Therefore, make ext4 support DIO on files that are using inline
encryption.  Since ext4 uses iomap for DIO, and fscrypt support was
already added to iomap DIO, this just requires two small changes:

- Let DIO proceed when supported, by checking fscrypt_dio_supported()
  instead of assuming that encrypted files never support DIO.

- In ext4_iomap_begin(), use fscrypt_limit_io_blocks() to limit the
  length of the mapping in the rare case where a DUN discontiguity
  occurs in the middle of an extent.  The iomap DIO implementation
  requires this, since it assumes that it can submit a bio covering (up
  to) the whole mapping, without checking fscrypt constraints itself.

Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Reviewed-by: Jaegeuk Kim <jaegeuk@kernel.org>
Acked-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/file.c  | 10 ++++++----
 fs/ext4/inode.c |  7 +++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 8cc11715518ac..8bd66cdc41be2 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -36,9 +36,11 @@
 #include "acl.h"
 #include "truncate.h"
 
-static bool ext4_dio_supported(struct inode *inode)
+static bool ext4_dio_supported(struct kiocb *iocb, struct iov_iter *iter)
 {
-	if (IS_ENABLED(CONFIG_FS_ENCRYPTION) && IS_ENCRYPTED(inode))
+	struct inode *inode = file_inode(iocb->ki_filp);
+
+	if (!fscrypt_dio_supported(iocb, iter))
 		return false;
 	if (fsverity_active(inode))
 		return false;
@@ -61,7 +63,7 @@ static ssize_t ext4_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
 		inode_lock_shared(inode);
 	}
 
-	if (!ext4_dio_supported(inode)) {
+	if (!ext4_dio_supported(iocb, to)) {
 		inode_unlock_shared(inode);
 		/*
 		 * Fallback to buffered I/O if the operation being performed on
@@ -509,7 +511,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	}
 
 	/* Fallback to buffered I/O if the inode does not support direct I/O. */
-	if (!ext4_dio_supported(inode)) {
+	if (!ext4_dio_supported(iocb, from)) {
 		if (ilock_shared)
 			inode_unlock_shared(inode);
 		else
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5f79d265d06a0..7af1bba34b8b8 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3409,6 +3409,13 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 	if (ret < 0)
 		return ret;
 out:
+	/*
+	 * When inline encryption is enabled, sometimes I/O to an encrypted file
+	 * has to be broken up to guarantee DUN contiguity.  Handle this by
+	 * limiting the length of the mapping returned.
+	 */
+	map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
+
 	ext4_set_iomap(inode, iomap, &map, offset, length, flags);
 
 	return 0;
-- 
2.35.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v11 4/5] f2fs: support direct I/O with fscrypt using blk-crypto
  2022-01-28 23:39 ` [f2fs-dev] " Eric Biggers
@ 2022-01-28 23:39   ` Eric Biggers
  -1 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Satya Tangirala

From: Eric Biggers <ebiggers@google.com>

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

Therefore, make f2fs support DIO on files that are using inline
encryption.  Since f2fs uses iomap for DIO, and fscrypt support was
already added to iomap DIO, this just requires two small changes:

- Let DIO proceed when supported, by checking fscrypt_dio_supported()
  instead of assuming that encrypted files never support DIO.

- In f2fs_iomap_begin(), use fscrypt_limit_io_blocks() to limit the
  length of the mapping in the rare case where a DUN discontiguity
  occurs in the middle of an extent.  The iomap DIO implementation
  requires this, since it assumes that it can submit a bio covering (up
  to) the whole mapping, without checking fscrypt constraints itself.

Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/f2fs/data.c | 7 +++++++
 fs/f2fs/f2fs.h | 6 +++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8c417864c66ae..020d47f97969c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -4044,6 +4044,13 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 
 	iomap->offset = blks_to_bytes(inode, map.m_lblk);
 
+	/*
+	 * When inline encryption is enabled, sometimes I/O to an encrypted file
+	 * has to be broken up to guarantee DUN contiguity.  Handle this by
+	 * limiting the length of the mapping returned.
+	 */
+	map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
+
 	if (map.m_flags & (F2FS_MAP_MAPPED | F2FS_MAP_UNWRITTEN)) {
 		iomap->length = blks_to_bytes(inode, map.m_len);
 		if (map.m_flags & F2FS_MAP_MAPPED) {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eb22fa91c2b26..db46f3cf0885d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4371,7 +4371,11 @@ static inline bool f2fs_force_buffered_io(struct inode *inode,
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	int rw = iov_iter_rw(iter);
 
-	if (f2fs_post_read_required(inode))
+	if (!fscrypt_dio_supported(iocb, iter))
+		return true;
+	if (fsverity_active(inode))
+		return true;
+	if (f2fs_compressed_file(inode))
 		return true;
 
 	/* disallow direct IO if any of devices has unaligned blksize */
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [f2fs-dev] [PATCH v11 4/5] f2fs: support direct I/O with fscrypt using blk-crypto
@ 2022-01-28 23:39   ` Eric Biggers
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: Satya Tangirala, Theodore Ts'o, Darrick J . Wong,
	Dave Chinner, linux-f2fs-devel, linux-xfs, linux-fsdevel,
	Jaegeuk Kim, linux-ext4, Christoph Hellwig

From: Eric Biggers <ebiggers@google.com>

Encrypted files traditionally haven't supported DIO, due to the need to
encrypt/decrypt the data.  However, when the encryption is implemented
using inline encryption (blk-crypto) instead of the traditional
filesystem-layer encryption, it is straightforward to support DIO.

Therefore, make f2fs support DIO on files that are using inline
encryption.  Since f2fs uses iomap for DIO, and fscrypt support was
already added to iomap DIO, this just requires two small changes:

- Let DIO proceed when supported, by checking fscrypt_dio_supported()
  instead of assuming that encrypted files never support DIO.

- In f2fs_iomap_begin(), use fscrypt_limit_io_blocks() to limit the
  length of the mapping in the rare case where a DUN discontiguity
  occurs in the middle of an extent.  The iomap DIO implementation
  requires this, since it assumes that it can submit a bio covering (up
  to) the whole mapping, without checking fscrypt constraints itself.

Co-developed-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Satya Tangirala <satyat@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/f2fs/data.c | 7 +++++++
 fs/f2fs/f2fs.h | 6 +++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 8c417864c66ae..020d47f97969c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -4044,6 +4044,13 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 
 	iomap->offset = blks_to_bytes(inode, map.m_lblk);
 
+	/*
+	 * When inline encryption is enabled, sometimes I/O to an encrypted file
+	 * has to be broken up to guarantee DUN contiguity.  Handle this by
+	 * limiting the length of the mapping returned.
+	 */
+	map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
+
 	if (map.m_flags & (F2FS_MAP_MAPPED | F2FS_MAP_UNWRITTEN)) {
 		iomap->length = blks_to_bytes(inode, map.m_len);
 		if (map.m_flags & F2FS_MAP_MAPPED) {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index eb22fa91c2b26..db46f3cf0885d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4371,7 +4371,11 @@ static inline bool f2fs_force_buffered_io(struct inode *inode,
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	int rw = iov_iter_rw(iter);
 
-	if (f2fs_post_read_required(inode))
+	if (!fscrypt_dio_supported(iocb, iter))
+		return true;
+	if (fsverity_active(inode))
+		return true;
+	if (f2fs_compressed_file(inode))
 		return true;
 
 	/* disallow direct IO if any of devices has unaligned blksize */
-- 
2.35.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v11 5/5] fscrypt: update documentation for direct I/O support
  2022-01-28 23:39 ` [f2fs-dev] " Eric Biggers
@ 2022-01-28 23:39   ` Eric Biggers
  -1 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-ext4, linux-f2fs-devel, linux-xfs,
	Theodore Ts'o, Jaegeuk Kim, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner

From: Eric Biggers <ebiggers@google.com>

Now that direct I/O is supported on encrypted files in some cases,
document what these cases are.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Documentation/filesystems/fscrypt.rst | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 4d5d50dca65c6..6ccd5efb25b77 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -1047,8 +1047,8 @@ astute users may notice some differences in behavior:
   may be used to overwrite the source files but isn't guaranteed to be
   effective on all filesystems and storage devices.
 
-- Direct I/O is not supported on encrypted files.  Attempts to use
-  direct I/O on such files will fall back to buffered I/O.
+- Direct I/O is supported on encrypted files only under some
+  circumstances.  For details, see `Direct I/O support`_.
 
 - The fallocate operations FALLOC_FL_COLLAPSE_RANGE and
   FALLOC_FL_INSERT_RANGE are not supported on encrypted files and will
@@ -1179,6 +1179,27 @@ Inline encryption doesn't affect the ciphertext or other aspects of
 the on-disk format, so users may freely switch back and forth between
 using "inlinecrypt" and not using "inlinecrypt".
 
+Direct I/O support
+==================
+
+For direct I/O on an encrypted file to work, the following conditions
+must be met (in addition to the conditions for direct I/O on an
+unencrypted file):
+
+* The file must be using inline encryption.  Usually this means that
+  the filesystem must be mounted with ``-o inlinecrypt`` and inline
+  encryption hardware must be present.  However, a software fallback
+  is also available.  For details, see `Inline encryption support`_.
+
+* The I/O request must be fully aligned to the filesystem block size.
+  This means that the file position the I/O is targeting, the lengths
+  of all I/O segments, and the memory addresses of all I/O buffers
+  must be multiples of this value.  Note that the filesystem block
+  size may be greater than the logical block size of the block device.
+
+If either of the above conditions is not met, then direct I/O on the
+encrypted file will fall back to buffered I/O.
+
 Implementation details
 ======================
 
-- 
2.35.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [f2fs-dev] [PATCH v11 5/5] fscrypt: update documentation for direct I/O support
@ 2022-01-28 23:39   ` Eric Biggers
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-01-28 23:39 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: Theodore Ts'o, Darrick J . Wong, Dave Chinner,
	linux-f2fs-devel, linux-xfs, linux-fsdevel, Jaegeuk Kim,
	linux-ext4, Christoph Hellwig

From: Eric Biggers <ebiggers@google.com>

Now that direct I/O is supported on encrypted files in some cases,
document what these cases are.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Documentation/filesystems/fscrypt.rst | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 4d5d50dca65c6..6ccd5efb25b77 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -1047,8 +1047,8 @@ astute users may notice some differences in behavior:
   may be used to overwrite the source files but isn't guaranteed to be
   effective on all filesystems and storage devices.
 
-- Direct I/O is not supported on encrypted files.  Attempts to use
-  direct I/O on such files will fall back to buffered I/O.
+- Direct I/O is supported on encrypted files only under some
+  circumstances.  For details, see `Direct I/O support`_.
 
 - The fallocate operations FALLOC_FL_COLLAPSE_RANGE and
   FALLOC_FL_INSERT_RANGE are not supported on encrypted files and will
@@ -1179,6 +1179,27 @@ Inline encryption doesn't affect the ciphertext or other aspects of
 the on-disk format, so users may freely switch back and forth between
 using "inlinecrypt" and not using "inlinecrypt".
 
+Direct I/O support
+==================
+
+For direct I/O on an encrypted file to work, the following conditions
+must be met (in addition to the conditions for direct I/O on an
+unencrypted file):
+
+* The file must be using inline encryption.  Usually this means that
+  the filesystem must be mounted with ``-o inlinecrypt`` and inline
+  encryption hardware must be present.  However, a software fallback
+  is also available.  For details, see `Inline encryption support`_.
+
+* The I/O request must be fully aligned to the filesystem block size.
+  This means that the file position the I/O is targeting, the lengths
+  of all I/O segments, and the memory addresses of all I/O buffers
+  must be multiples of this value.  Note that the filesystem block
+  size may be greater than the logical block size of the block device.
+
+If either of the above conditions is not met, then direct I/O on the
+encrypted file will fall back to buffered I/O.
+
 Implementation details
 ======================
 
-- 
2.35.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v11 4/5] f2fs: support direct I/O with fscrypt using blk-crypto
  2022-01-28 23:39   ` [f2fs-dev] " Eric Biggers
@ 2022-01-29  0:27     ` Jaegeuk Kim
  -1 siblings, 0 replies; 20+ messages in thread
From: Jaegeuk Kim @ 2022-01-29  0:27 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
	linux-xfs, Theodore Ts'o, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Satya Tangirala

On 01/28, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Encrypted files traditionally haven't supported DIO, due to the need to
> encrypt/decrypt the data.  However, when the encryption is implemented
> using inline encryption (blk-crypto) instead of the traditional
> filesystem-layer encryption, it is straightforward to support DIO.
> 
> Therefore, make f2fs support DIO on files that are using inline
> encryption.  Since f2fs uses iomap for DIO, and fscrypt support was
> already added to iomap DIO, this just requires two small changes:
> 
> - Let DIO proceed when supported, by checking fscrypt_dio_supported()
>   instead of assuming that encrypted files never support DIO.
> 
> - In f2fs_iomap_begin(), use fscrypt_limit_io_blocks() to limit the
>   length of the mapping in the rare case where a DUN discontiguity
>   occurs in the middle of an extent.  The iomap DIO implementation
>   requires this, since it assumes that it can submit a bio covering (up
>   to) the whole mapping, without checking fscrypt constraints itself.
> 
> Co-developed-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>

> ---
>  fs/f2fs/data.c | 7 +++++++
>  fs/f2fs/f2fs.h | 6 +++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 8c417864c66ae..020d47f97969c 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -4044,6 +4044,13 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  
>  	iomap->offset = blks_to_bytes(inode, map.m_lblk);
>  
> +	/*
> +	 * When inline encryption is enabled, sometimes I/O to an encrypted file
> +	 * has to be broken up to guarantee DUN contiguity.  Handle this by
> +	 * limiting the length of the mapping returned.
> +	 */
> +	map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
> +
>  	if (map.m_flags & (F2FS_MAP_MAPPED | F2FS_MAP_UNWRITTEN)) {
>  		iomap->length = blks_to_bytes(inode, map.m_len);
>  		if (map.m_flags & F2FS_MAP_MAPPED) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index eb22fa91c2b26..db46f3cf0885d 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4371,7 +4371,11 @@ static inline bool f2fs_force_buffered_io(struct inode *inode,
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  	int rw = iov_iter_rw(iter);
>  
> -	if (f2fs_post_read_required(inode))
> +	if (!fscrypt_dio_supported(iocb, iter))
> +		return true;
> +	if (fsverity_active(inode))
> +		return true;
> +	if (f2fs_compressed_file(inode))
>  		return true;
>  
>  	/* disallow direct IO if any of devices has unaligned blksize */
> -- 
> 2.35.0

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v11 4/5] f2fs: support direct I/O with fscrypt using blk-crypto
@ 2022-01-29  0:27     ` Jaegeuk Kim
  0 siblings, 0 replies; 20+ messages in thread
From: Jaegeuk Kim @ 2022-01-29  0:27 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Satya Tangirala, Theodore Ts'o, Darrick J . Wong,
	Dave Chinner, linux-f2fs-devel, linux-xfs, linux-fscrypt,
	linux-fsdevel, linux-ext4, Christoph Hellwig

On 01/28, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Encrypted files traditionally haven't supported DIO, due to the need to
> encrypt/decrypt the data.  However, when the encryption is implemented
> using inline encryption (blk-crypto) instead of the traditional
> filesystem-layer encryption, it is straightforward to support DIO.
> 
> Therefore, make f2fs support DIO on files that are using inline
> encryption.  Since f2fs uses iomap for DIO, and fscrypt support was
> already added to iomap DIO, this just requires two small changes:
> 
> - Let DIO proceed when supported, by checking fscrypt_dio_supported()
>   instead of assuming that encrypted files never support DIO.
> 
> - In f2fs_iomap_begin(), use fscrypt_limit_io_blocks() to limit the
>   length of the mapping in the rare case where a DUN discontiguity
>   occurs in the middle of an extent.  The iomap DIO implementation
>   requires this, since it assumes that it can submit a bio covering (up
>   to) the whole mapping, without checking fscrypt constraints itself.
> 
> Co-developed-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>

> ---
>  fs/f2fs/data.c | 7 +++++++
>  fs/f2fs/f2fs.h | 6 +++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 8c417864c66ae..020d47f97969c 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -4044,6 +4044,13 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>  
>  	iomap->offset = blks_to_bytes(inode, map.m_lblk);
>  
> +	/*
> +	 * When inline encryption is enabled, sometimes I/O to an encrypted file
> +	 * has to be broken up to guarantee DUN contiguity.  Handle this by
> +	 * limiting the length of the mapping returned.
> +	 */
> +	map.m_len = fscrypt_limit_io_blocks(inode, map.m_lblk, map.m_len);
> +
>  	if (map.m_flags & (F2FS_MAP_MAPPED | F2FS_MAP_UNWRITTEN)) {
>  		iomap->length = blks_to_bytes(inode, map.m_len);
>  		if (map.m_flags & F2FS_MAP_MAPPED) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index eb22fa91c2b26..db46f3cf0885d 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -4371,7 +4371,11 @@ static inline bool f2fs_force_buffered_io(struct inode *inode,
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  	int rw = iov_iter_rw(iter);
>  
> -	if (f2fs_post_read_required(inode))
> +	if (!fscrypt_dio_supported(iocb, iter))
> +		return true;
> +	if (fsverity_active(inode))
> +		return true;
> +	if (f2fs_compressed_file(inode))
>  		return true;
>  
>  	/* disallow direct IO if any of devices has unaligned blksize */
> -- 
> 2.35.0


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v11 1/5] fscrypt: add functions for direct I/O support
  2022-01-28 23:39   ` [f2fs-dev] " Eric Biggers
@ 2022-02-01  8:06     ` Christoph Hellwig
  -1 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-02-01  8:06 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
	linux-xfs, Theodore Ts'o, Jaegeuk Kim, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner, Satya Tangirala

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v11 1/5] fscrypt: add functions for direct I/O support
@ 2022-02-01  8:06     ` Christoph Hellwig
  0 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-02-01  8:06 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Satya Tangirala, Theodore Ts'o, Darrick J . Wong,
	Dave Chinner, linux-f2fs-devel, linux-xfs, linux-fscrypt,
	linux-fsdevel, Jaegeuk Kim, linux-ext4, Christoph Hellwig

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v11 5/5] fscrypt: update documentation for direct I/O support
  2022-01-28 23:39   ` [f2fs-dev] " Eric Biggers
@ 2022-02-01  8:07     ` Christoph Hellwig
  -1 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-02-01  8:07 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-fscrypt, linux-fsdevel, linux-ext4, linux-f2fs-devel,
	linux-xfs, Theodore Ts'o, Jaegeuk Kim, Christoph Hellwig,
	Darrick J . Wong, Dave Chinner

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v11 5/5] fscrypt: update documentation for direct I/O support
@ 2022-02-01  8:07     ` Christoph Hellwig
  0 siblings, 0 replies; 20+ messages in thread
From: Christoph Hellwig @ 2022-02-01  8:07 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Theodore Ts'o, Darrick J . Wong, Dave Chinner,
	linux-f2fs-devel, linux-xfs, linux-fscrypt, linux-fsdevel,
	Jaegeuk Kim, linux-ext4, Christoph Hellwig

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v11 0/5] add support for direct I/O with fscrypt using blk-crypto
  2022-01-28 23:39 ` [f2fs-dev] " Eric Biggers
@ 2022-02-11  6:17   ` Eric Biggers
  -1 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-02-11  6:17 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: Theodore Ts'o, Darrick J . Wong, Dave Chinner,
	linux-f2fs-devel, linux-xfs, linux-fsdevel, Jaegeuk Kim,
	linux-ext4, Christoph Hellwig

On Fri, Jan 28, 2022 at 03:39:35PM -0800, Eric Biggers wrote:
> [Note: I'm planning to send a patchset adding STATX_DIRECTIO as was
> discussed on v10, but that will be a separate patchset.]
> 
> Encrypted files traditionally haven't supported DIO, due to the need to
> encrypt/decrypt the data.  However, when the encryption is implemented
> using inline encryption (blk-crypto) instead of the traditional
> filesystem-layer encryption, it is straightforward to support DIO.
> 
> This series adds support for this.  There are multiple use cases for DIO
> on encrypted files, but avoiding double caching on loopback devices
> located in an encrypted directory is the main one currently.
> 
> v1 through v9 of this series were sent out by Satya Tangirala.  I've
> cleaned up a few things since Satya's last version
> (https://lore.kernel.org/all/20210604210908.2105870-1-satyat@google.com/T/#u).
> But more notably, I've made a couple simplifications.
> 
> First, since f2fs has now been converted to use iomap for DIO, I've
> dropped the patch which added fscrypt support to fs/direct-io.c.
> 
> Second, I've returned to the original design where DIO requests must be
> fully aligned to the FS block size in terms of file position, length,
> and memory buffers.  Satya previously was pursuing a slightly different
> design, where the memory buffers (but not the file position and length)
> were allowed to be aligned to just the block device logical block size.
> This was at the request of Dave Chinner on v4 and v6 of the patchset
> (https://lore.kernel.org/linux-fscrypt/20200720233739.824943-1-satyat@google.com/T/#u
> and
> https://lore.kernel.org/linux-fscrypt/20200724184501.1651378-1-satyat@google.com/T/#u).
> 
> I believe that approach is a dead end, for two reasons.  First, it
> necessarily causes it to be possible that crypto data units span bvecs.
> Splits cannot occur at such locations; however the block layer currently
> assumes that bios can be split at any bvec boundary.  Changing that is
> quite difficult, as Satya's v9 patchset demonstrated.  This is not an
> issue if we require FS block aligned buffers instead.  Second, it
> doesn't change the fact that FS block alignment is still required for
> the file position and I/O length; this is unavoidable due to the
> granularity of encryption being the FS block size.  So, it seems that
> relaxing the memory buffer alignment requirement wouldn't make things
> meaningfully easier for applications, which raises the question of why
> we would bother with it in the first place.
> 
> Christoph Hellwig also said that he much prefers that fscrypt DIO be
> supported without sector-only alignment to start:
> https://lore.kernel.org/r/YPu+88KReGlt94o3@infradead.org
> 
> Given the above, as far as I know the only remaining objection to this
> patchset would be that DIO constraints aren't sufficiently discoverable
> by userspace.  Now, to put this in context, this is a longstanding issue
> with all Linux filesystems, except XFS which has XFS_IOC_DIOINFO.  It's
> not specific to this feature, and it doesn't actually seem to be too
> important in practice; many other filesystem features place constraints
> on DIO, and f2fs even *only* allows fully FS block size aligned DIO.
> (And for better or worse, many systems using fscrypt already have
> out-of-tree patches that enable DIO support, and people don't seem to
> have trouble with the FS block size alignment requirement.)
> 
> To address the issue of DIO constraints being insufficiently
> discoverable, I plan to make statx() expose this information.
> 
> This series applies to v5.17-rc1.
> 
> Changed v10 => v11:
>   * Changed fscrypt_dio_unsupported() back to fscrypt_dio_supported().
>   * Removed a mention of f2fs from fscrypt_dio_supported().
>   * Added Reviewed-by and Acked-by tags, including a couple from earlier
>     I had dropped due to the renaming of fscrypt_dio_supported().
>   * In fscrypt_limit_io_blocks(), don't load i_crypt_info until it's
>     known to be valid, to avoid confusion as is done elsewhere.
> 
> Eric Biggers (5):
>   fscrypt: add functions for direct I/O support
>   iomap: support direct I/O with fscrypt using blk-crypto
>   ext4: support direct I/O with fscrypt using blk-crypto
>   f2fs: support direct I/O with fscrypt using blk-crypto
>   fscrypt: update documentation for direct I/O support
> 
>  Documentation/filesystems/fscrypt.rst | 25 ++++++-
>  fs/crypto/crypto.c                    |  8 +++
>  fs/crypto/inline_crypt.c              | 93 +++++++++++++++++++++++++++
>  fs/ext4/file.c                        | 10 +--
>  fs/ext4/inode.c                       |  7 ++
>  fs/f2fs/data.c                        |  7 ++
>  fs/f2fs/f2fs.h                        |  6 +-
>  fs/iomap/direct-io.c                  |  6 ++
>  include/linux/fscrypt.h               | 18 ++++++
>  9 files changed, 173 insertions(+), 7 deletions(-)

I've applied this patchset to fscrypt.git#master for 5.18
(https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git/log/).

- Eric

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [f2fs-dev] [PATCH v11 0/5] add support for direct I/O with fscrypt using blk-crypto
@ 2022-02-11  6:17   ` Eric Biggers
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Biggers @ 2022-02-11  6:17 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: Theodore Ts'o, Darrick J . Wong, Dave Chinner,
	linux-f2fs-devel, linux-xfs, linux-fsdevel, Jaegeuk Kim,
	linux-ext4, Christoph Hellwig

On Fri, Jan 28, 2022 at 03:39:35PM -0800, Eric Biggers wrote:
> [Note: I'm planning to send a patchset adding STATX_DIRECTIO as was
> discussed on v10, but that will be a separate patchset.]
> 
> Encrypted files traditionally haven't supported DIO, due to the need to
> encrypt/decrypt the data.  However, when the encryption is implemented
> using inline encryption (blk-crypto) instead of the traditional
> filesystem-layer encryption, it is straightforward to support DIO.
> 
> This series adds support for this.  There are multiple use cases for DIO
> on encrypted files, but avoiding double caching on loopback devices
> located in an encrypted directory is the main one currently.
> 
> v1 through v9 of this series were sent out by Satya Tangirala.  I've
> cleaned up a few things since Satya's last version
> (https://lore.kernel.org/all/20210604210908.2105870-1-satyat@google.com/T/#u).
> But more notably, I've made a couple simplifications.
> 
> First, since f2fs has now been converted to use iomap for DIO, I've
> dropped the patch which added fscrypt support to fs/direct-io.c.
> 
> Second, I've returned to the original design where DIO requests must be
> fully aligned to the FS block size in terms of file position, length,
> and memory buffers.  Satya previously was pursuing a slightly different
> design, where the memory buffers (but not the file position and length)
> were allowed to be aligned to just the block device logical block size.
> This was at the request of Dave Chinner on v4 and v6 of the patchset
> (https://lore.kernel.org/linux-fscrypt/20200720233739.824943-1-satyat@google.com/T/#u
> and
> https://lore.kernel.org/linux-fscrypt/20200724184501.1651378-1-satyat@google.com/T/#u).
> 
> I believe that approach is a dead end, for two reasons.  First, it
> necessarily causes it to be possible that crypto data units span bvecs.
> Splits cannot occur at such locations; however the block layer currently
> assumes that bios can be split at any bvec boundary.  Changing that is
> quite difficult, as Satya's v9 patchset demonstrated.  This is not an
> issue if we require FS block aligned buffers instead.  Second, it
> doesn't change the fact that FS block alignment is still required for
> the file position and I/O length; this is unavoidable due to the
> granularity of encryption being the FS block size.  So, it seems that
> relaxing the memory buffer alignment requirement wouldn't make things
> meaningfully easier for applications, which raises the question of why
> we would bother with it in the first place.
> 
> Christoph Hellwig also said that he much prefers that fscrypt DIO be
> supported without sector-only alignment to start:
> https://lore.kernel.org/r/YPu+88KReGlt94o3@infradead.org
> 
> Given the above, as far as I know the only remaining objection to this
> patchset would be that DIO constraints aren't sufficiently discoverable
> by userspace.  Now, to put this in context, this is a longstanding issue
> with all Linux filesystems, except XFS which has XFS_IOC_DIOINFO.  It's
> not specific to this feature, and it doesn't actually seem to be too
> important in practice; many other filesystem features place constraints
> on DIO, and f2fs even *only* allows fully FS block size aligned DIO.
> (And for better or worse, many systems using fscrypt already have
> out-of-tree patches that enable DIO support, and people don't seem to
> have trouble with the FS block size alignment requirement.)
> 
> To address the issue of DIO constraints being insufficiently
> discoverable, I plan to make statx() expose this information.
> 
> This series applies to v5.17-rc1.
> 
> Changed v10 => v11:
>   * Changed fscrypt_dio_unsupported() back to fscrypt_dio_supported().
>   * Removed a mention of f2fs from fscrypt_dio_supported().
>   * Added Reviewed-by and Acked-by tags, including a couple from earlier
>     I had dropped due to the renaming of fscrypt_dio_supported().
>   * In fscrypt_limit_io_blocks(), don't load i_crypt_info until it's
>     known to be valid, to avoid confusion as is done elsewhere.
> 
> Eric Biggers (5):
>   fscrypt: add functions for direct I/O support
>   iomap: support direct I/O with fscrypt using blk-crypto
>   ext4: support direct I/O with fscrypt using blk-crypto
>   f2fs: support direct I/O with fscrypt using blk-crypto
>   fscrypt: update documentation for direct I/O support
> 
>  Documentation/filesystems/fscrypt.rst | 25 ++++++-
>  fs/crypto/crypto.c                    |  8 +++
>  fs/crypto/inline_crypt.c              | 93 +++++++++++++++++++++++++++
>  fs/ext4/file.c                        | 10 +--
>  fs/ext4/inode.c                       |  7 ++
>  fs/f2fs/data.c                        |  7 ++
>  fs/f2fs/f2fs.h                        |  6 +-
>  fs/iomap/direct-io.c                  |  6 ++
>  include/linux/fscrypt.h               | 18 ++++++
>  9 files changed, 173 insertions(+), 7 deletions(-)

I've applied this patchset to fscrypt.git#master for 5.18
(https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git/log/).

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-02-11  6:17 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 23:39 [PATCH v11 0/5] add support for direct I/O with fscrypt using blk-crypto Eric Biggers
2022-01-28 23:39 ` [f2fs-dev] " Eric Biggers
2022-01-28 23:39 ` [PATCH v11 1/5] fscrypt: add functions for direct I/O support Eric Biggers
2022-01-28 23:39   ` [f2fs-dev] " Eric Biggers
2022-02-01  8:06   ` Christoph Hellwig
2022-02-01  8:06     ` [f2fs-dev] " Christoph Hellwig
2022-01-28 23:39 ` [PATCH v11 2/5] iomap: support direct I/O with fscrypt using blk-crypto Eric Biggers
2022-01-28 23:39   ` [f2fs-dev] " Eric Biggers
2022-01-28 23:39 ` [PATCH v11 3/5] ext4: " Eric Biggers
2022-01-28 23:39   ` [f2fs-dev] " Eric Biggers
2022-01-28 23:39 ` [PATCH v11 4/5] f2fs: " Eric Biggers
2022-01-28 23:39   ` [f2fs-dev] " Eric Biggers
2022-01-29  0:27   ` Jaegeuk Kim
2022-01-29  0:27     ` [f2fs-dev] " Jaegeuk Kim
2022-01-28 23:39 ` [PATCH v11 5/5] fscrypt: update documentation for direct I/O support Eric Biggers
2022-01-28 23:39   ` [f2fs-dev] " Eric Biggers
2022-02-01  8:07   ` Christoph Hellwig
2022-02-01  8:07     ` [f2fs-dev] " Christoph Hellwig
2022-02-11  6:17 ` [PATCH v11 0/5] add support for direct I/O with fscrypt using blk-crypto Eric Biggers
2022-02-11  6:17   ` [f2fs-dev] " Eric Biggers

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.