On Sat, Jul 25, 2020 at 10:14:41AM +1000, Dave Chinner wrote:
> > +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 direct I/O 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 I/O
> > + * must be block aligned -- not just disk sector aligned.
> > + */
> > + if (!IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(iter), blocksize))
> > + return false;
>
> Doesn't this force user buffers to be filesystem block size aligned,
> instead of 512 byte aligned as is typical for direct IO?
>
> That's going to cause applications that work fine on normal
> filesystems becaues the memalign() buffers to 512 bytes or logical
> block device sector sizes (as per the open(2) man page) to fail on
> encrypted volumes, and it's not going to be obvious to users as to
> why this happens.
The status quo is that direct I/O on encrypted files falls back to buffered I/O.
So this patch is strictly an improvement; it's making direct I/O work in a case
where previously it didn't work.
>
> XFS has XFS_IOC_DIOINFO to expose exactly this information to
> userspace on a per-file basis. Other filesystem and VFS developers
> have said for the past 15 years "we don't need no stinking DIOINFO".
> The same people shot down adding optional IO alignment
> constraint fields to statx() a few years ago, too.
>
> Yet here were are again, with alignment of DIO buffers being an
> issue that userspace needs to know about....
>
A DIOINFO ioctl sounds like a good idea to me, although I'm not familiar with
previous discussions about it.
Note that there are lots of other cases where ext4 and f2fs fall back to
buffered I/O; see ext4_dio_supported() and f2fs_force_buffered_io(). So this
isn't a new problem.
- Eric
