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@xxxxxxxxxx/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@xxxxxxxxxx/T/#u > and > https://lore.kernel.org/linux-fscrypt/20200724184501.1651378-1-satyat@xxxxxxxxxx/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@xxxxxxxxxxxxx > > 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
