Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-block@vger.kernel.org, linux-fscrypt@vger.kernel.org
Subject: Re: [PATCH 3/6] fs: Convert block_read_full_page to be synchronous
Date: Fri, 23 Oct 2020 21:42:10 +0100
Message-ID: <20201023204210.GF20115@casper.infradead.org> (raw)
In-Reply-To: <20201023161335.GB3908702@gmail.com>

On Fri, Oct 23, 2020 at 09:13:35AM -0700, Eric Biggers wrote:
> On Fri, Oct 23, 2020 at 02:21:38PM +0100, Matthew Wilcox wrote:
> > I wonder about allocating bios that can accommodate more bvecs.  Not sure
> > how often filesystems have adjacent blocks which go into non-adjacent
> > sub-page blocks.  It's certainly possible that a filesystem might have
> > a page consisting of DDhhDDDD ('D' for Data, 'h' for hole), but how
> > likely is it to have written the two data chunks next to each other?
> > Maybe with O_SYNC?
> 
> I think that's a rare case that's not very important to optimize.  And there's
> already a lot of code where filesystems *could* submit a single bio in that case
> but don't.  For example, both fs/direct-io.c and fs/iomap/direct-io.c only
> submit bios that contain logically contiguous data.

True.  iomap/buffered-io.c will do it though.

> If you do implement this optimization, note that it wouldn't work when a
> bio_crypt_ctx is set, since the data must be logically contiguous in that case.
> To handle that you'd need to call fscrypt_mergeable_bio_bh() when adding each
> block, and submit the bio if it returns false.  (In contrast, with your current
> proposal, calling fscrypt_mergeable_bio_bh() isn't necessary because each bio
> only contains logically contiguous data within one page.)

Oh, that's disappointing.  I had assumed that you'd set up the dun for
the logical block corresponding to the start of the page and then you'd
be able to decrypt any range in the page.

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-22 21:22 [PATCH 0/6] Make block_read_full_page synchronous Matthew Wilcox (Oracle)
2020-10-22 21:22 ` [PATCH 1/6] block: Add blk_completion Matthew Wilcox (Oracle)
2020-10-27 18:30   ` Christoph Hellwig
2020-10-22 21:22 ` [PATCH 2/6] fs: Return error from block_read_full_page Matthew Wilcox (Oracle)
2020-10-22 21:22 ` [PATCH 3/6] fs: Convert block_read_full_page to be synchronous Matthew Wilcox (Oracle)
2020-10-22 23:35   ` Eric Biggers
2020-10-22 23:40   ` Eric Biggers
2020-10-23 13:21     ` Matthew Wilcox
2020-10-23 16:13       ` Eric Biggers
2020-10-23 20:42         ` Matthew Wilcox [this message]
2020-10-22 21:22 ` [PATCH 4/6] fs: Hoist fscrypt decryption to bio completion handler Matthew Wilcox (Oracle)
2020-10-22 21:22 ` [PATCH 5/6] fs: Turn decrypt_bh into decrypt_bio Matthew Wilcox (Oracle)
2020-10-22 21:22 ` [PATCH 6/6] fs: Convert block_read_full_page to be synchronous with fscrypt enabled Matthew Wilcox (Oracle)

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=20201023204210.GF20115@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=ebiggers@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git