Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
From: Matthew Bobrowski <mbobrowski@mbobrowski.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>,
	Ritesh Harjani <riteshh@linux.ibm.com>,
	tytso@mit.edu, jack@suse.cz, adilger.kernel@dilger.ca,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	aneesh.kumar@linux.ibm.com
Subject: Re: [PATCH 0/5] ext4: direct IO via iomap infrastructure
Date: Tue, 27 Aug 2019 19:52:23 +1000
Message-ID: <20190827095221.GA1568@poseidon.bobrowski.net> (raw)
In-Reply-To: <20190824230427.GA32012@infradead.org>

On Sat, Aug 24, 2019 at 04:04:27PM -0700, Christoph Hellwig wrote:
> On Fri, Aug 23, 2019 at 08:55:54PM -0700, Darrick J. Wong wrote:
> > I'm probably misunderstanding the ext4 extent cache horribly, but I keep
> > wondering why any of this is necessary -- why can't ext4 track the
> > unwritten status in the extent records directly?  And why is there all
> > this strange "can merge" logic?  If you need to convert blocks X to Y
> > to written state because a write to those blocks completed, isn't that
> > just manipulation of a bunch of incore records?  And can't you just seek
> > back and forth in the extent cache to look for adjacent records to merge
> > with? <confuseD>
> 
> Same here.  I'm not an ext4 expert, but here is what we do in XFS, which
> hopefully works in some form for ext4 a well:
> 
>  - when starting a direct I/O we allocate any needed blocks and do so
>    as unwritten extent.  The extent tree code will merge them in
>    whatever way that seems suitable
>  - if the IOMAP_DIO_UNWRITTEN is set on the iomap at ->end_io time we
>    call a function that walks the whole range covered by the ioend,
>    and convert any unwritten extent to a normal written extent.  Any
>    splitting and merging will be done as needed by the low-level
>    extent tree code
>  - this also means we don't need the xfs_ioen structure (which ext4)
>    copied from for direct I/O at all (we used to have it initially,
>    though including the time when ext4 copied this code).
>  - we don't need the equivalent to the ext4_unwritten_wait call in
>    ext4_file_write_iter because we serialize any non-aligned I/O
>    instead of trying to optimize for weird corner cases

Yeah, so what you've detailed above is essentially the approach I've
taken in my patch series...

What is not clear to me at this point though is whether it is still
necessary to explicitly track unwritten extents via in-core inode
attributes i.e. ->i_unwritten and ->i_state_flags under the new direct
IO code path implementation, which makes use of the iomap
infrastructure. Or, whether we can get away with simply not using
these in-core inode attributes and rely just on checks against the
extent record directly, as breifly mentioned by Darrick. I would think
that this type of check would be enough, however the checks around
whether the inode is currently undergoing direct IO were implemented
at some point, so there must be a reason for having them
(a9b8241594add)?

--M

  reply index

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-12 12:52 Matthew Bobrowski
2019-08-12 12:52 ` [PATCH 1/5] ext4: introduce direct IO read code path using " Matthew Bobrowski
2019-08-12 17:18   ` Christoph Hellwig
2019-08-12 20:17     ` Matthew Wilcox
2019-08-13 10:45       ` Matthew Bobrowski
2019-08-12 12:52 ` [PATCH 2/5] ext4: move inode extension/truncate code out from ext4_iomap_end() Matthew Bobrowski
2019-08-12 17:18   ` Christoph Hellwig
2019-08-13 10:46     ` Matthew Bobrowski
2019-08-28 19:59   ` Jan Kara
2019-08-28 21:54     ` Matthew Bobrowski
2019-08-29  8:18       ` Jan Kara
2019-08-12 12:53 ` [PATCH 3/5] iomap: modify ->end_io() calling convention Matthew Bobrowski
2019-08-12 17:18   ` Christoph Hellwig
2019-08-13 10:43     ` Matthew Bobrowski
2019-08-12 12:53 ` [PATCH 4/5] ext4: introduce direct IO write code path using iomap infrastructure Matthew Bobrowski
2019-08-12 17:04   ` RITESH HARJANI
2019-08-13 12:58     ` Matthew Bobrowski
2019-08-13 14:35       ` Darrick J. Wong
2019-08-14  9:51         ` Matthew Bobrowski
2019-08-12 17:34   ` Christoph Hellwig
2019-08-13 10:45     ` Matthew Bobrowski
2019-08-28 20:26   ` Jan Kara
2019-08-28 22:32     ` Dave Chinner
2019-08-29  8:03       ` Jan Kara
2019-08-29 11:47       ` Matthew Bobrowski
2019-08-29 11:45     ` Matthew Bobrowski
2019-08-29 12:38       ` Jan Kara
2019-08-12 12:53 ` [PATCH 5/5] ext4: clean up redundant buffer_head direct IO code Matthew Bobrowski
2019-08-12 17:31 ` [PATCH 0/5] ext4: direct IO via iomap infrastructure RITESH HARJANI
2019-08-13 11:10   ` Matthew Bobrowski
2019-08-13 12:27     ` RITESH HARJANI
2019-08-14  9:48       ` Matthew Bobrowski
2019-08-14 11:58         ` RITESH HARJANI
2019-08-21 13:14       ` Matthew Bobrowski
2019-08-22 12:00         ` Matthew Bobrowski
2019-08-22 14:11           ` Ritesh Harjani
2019-08-24  3:18             ` Matthew Bobrowski
2019-08-24  3:55               ` Darrick J. Wong
2019-08-24 23:04                 ` Christoph Hellwig
2019-08-27  9:52                   ` Matthew Bobrowski [this message]
2019-08-28 12:05                     ` Matthew Bobrowski
2019-08-28 14:27                       ` Theodore Y. Ts'o
2019-08-28 18:02                         ` Jan Kara
2019-08-29  6:36                           ` Christoph Hellwig
2019-08-29 11:20                             ` Matthew Bobrowski
2019-08-29 14:41                               ` Christoph Hellwig
2019-08-23 13:43           ` [RFC 1/1] ext4: PoC implementation of option-1 Ritesh Harjani
2019-08-23 13:49             ` Ritesh Harjani

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=20190827095221.GA1568@poseidon.bobrowski.net \
    --to=mbobrowski@mbobrowski.org \
    --cc=adilger.kernel@dilger.ca \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=darrick.wong@oracle.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=riteshh@linux.ibm.com \
    --cc=tytso@mit.edu \
    /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-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/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-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

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


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