linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Satya Tangirala <satyat@google.com>
Cc: linux-fsdevel@vger.kernel.org, linux-fscrypt@vger.kernel.org,
	linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 1/5] fscrypt: Add functions for direct I/O support
Date: Thu, 9 Jul 2020 14:54:58 -0700	[thread overview]
Message-ID: <20200709215458.GC3855682@gmail.com> (raw)
In-Reply-To: <20200709194751.2579207-2-satyat@google.com>

On Thu, Jul 09, 2020 at 07:47:47PM +0000, Satya Tangirala via Linux-f2fs-devel wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Introduce fscrypt_dio_supported() to check whether a direct I/O request
> is unsupported due to encryption constraints, and
> fscrypt_limit_dio_pages() to check how many pages may be added to a bio
> being prepared for direct I/O.
> 
> The IV_INO_LBLK_32 fscrypt policy introduces the possibility that DUNs
> in logically continuous file blocks might wrap from 0xffffffff to 0.
> Bios in which the DUN wraps around like this cannot be submitted. This
> is especially difficult to handle when block_size != PAGE_SIZE, since in
> that case the DUN can wrap in the middle of a page.
> 
> For now, we add direct I/O support while using IV_INO_LBLK_32 policies
> only for the case when block_size == PAGE_SIZE. When IV_INO_LBLK_32
> policy is used, fscrypt_dio_supported() rejects the bio when
> block_size != PAGE_SIZE. fscrypt_limit_dio_pages() returns the number of
> pages that may be added to the bio without causing the DUN to wrap
> around within the bio.

This commit message is a bit outdated, since the latest version of
"fscrypt: add inline encryption support" already makes IV_INO_LBLK_32
with block_size != PAGE_SIZE fall back to filesystem-layer encryption,
and hence it won't allow direct I/O.

> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Satya Tangirala <satyat@google.com>

Can you mention any changes you made, e.g.:

Signed-off-by: Eric Biggers <ebiggers@google.com>
[ST: split original change into separate patches, and updated to account
 for inline encryption no longer being allowed with IV_INO_LBLK_32 and
 blocksize != PAGE_SIZE]
Signed-off-by: Satya Tangirala <satyat@google.com>

> +/**
> + * fscrypt_limit_dio_pages() - limit I/O pages to avoid discontiguous DUNs
> + * @inode: the file on which I/O is being done
> + * @pos: the file position (in bytes) at which the I/O is being done
> + * @nr_pages: the number of pages we want to submit starting at @pos
> + *
> + * For direct I/O: limit the number of pages that will be submitted in the bio
> + * targeting @pos, in order to avoid crossing a data unit number (DUN)
> + * discontinuity.  This is only needed for certain IV generation methods.
> + *
> + * This assumes block_size == PAGE_SIZE; see fscrypt_dio_supported().

The note about block_size == PAGE_SIZE here is outdated.

I was also struggling a bit to decide what to name this function.  Note
that it's not really direct I/O specific.  Also, fs/iomap/direct-io.c
needs it but fs/direct-io.c does not.

What this function really does is batch together the mergeability checks
for a logical range.

Maybe the comment could explain this better, and maybe the function
should be called "fscrypt_limit_io_pages()" instead.

> + * Return: the actual number of pages that can be submitted
> + */
> +int fscrypt_limit_dio_pages(const struct inode *inode, loff_t pos, int nr_pages)
> +{
> +	const struct fscrypt_info *ci = inode->i_crypt_info;
> +	u32 dun;
> +
> +	if (!fscrypt_inode_uses_inline_crypto(inode))
> +		return nr_pages;
> +
> +	if (nr_pages <= 1)
> +		return nr_pages;
> +
> +	if (!(fscrypt_policy_flags(&ci->ci_policy) &
> +	      FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
> +		return nr_pages;
> +
> +	if (WARN_ON_ONCE(i_blocksize(inode) != PAGE_SIZE))
> +		return 1;
> +
> +	/* With IV_INO_LBLK_32, the DUN can wrap around from U32_MAX to 0. */
> +
> +	dun = ci->ci_hashed_ino + (pos >> inode->i_blkbits);
> +
> +	return min_t(u64, nr_pages, (u64)U32_MAX + 1 - dun);
> +}

- Eric


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

  reply	other threads:[~2020-07-09 21:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-09 19:47 [f2fs-dev] [PATCH 0/5] add support for direct I/O with fscrypt using blk-crypto Satya Tangirala via Linux-f2fs-devel
2020-07-09 19:47 ` [f2fs-dev] [PATCH 1/5] fscrypt: Add functions for direct I/O support Satya Tangirala via Linux-f2fs-devel
2020-07-09 21:54   ` Eric Biggers [this message]
2020-07-09 19:47 ` [f2fs-dev] [PATCH 2/5] direct-io: add support for fscrypt using blk-crypto Satya Tangirala via Linux-f2fs-devel
2020-07-10  5:34   ` Christoph Hellwig
2020-07-13 18:36     ` Eric Biggers
2020-07-17  1:56       ` Satya Tangirala via Linux-f2fs-devel
2020-07-09 19:47 ` [f2fs-dev] [PATCH 3/5] iomap: support direct I/O with " Satya Tangirala via Linux-f2fs-devel
2020-07-09 21:59   ` Eric Biggers
2020-07-09 19:47 ` [f2fs-dev] [PATCH 4/5] ext4: " Satya Tangirala via Linux-f2fs-devel
2020-07-09 22:30   ` Eric Biggers
2020-07-09 19:47 ` [f2fs-dev] [PATCH 5/5] f2fs: " Satya Tangirala via Linux-f2fs-devel
2020-07-10  1:05   ` Chao Yu
2020-07-10  1:15     ` Eric Biggers
2020-07-09 22:46 ` [f2fs-dev] [PATCH 0/5] add support for " Eric Biggers

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=20200709215458.GC3855682@gmail.com \
    --to=ebiggers@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=satyat@google.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).