Re: [RFC 0/5] Ext4: Add support for blocksize < pagesize for dioread_nolock

From: Matthew Bobrowski <mbobrowski@mbobrowski.org>
To: Ritesh Harjani <riteshh@linux.ibm.com>
Cc: "Theodore Y. Ts'o" <tytso@mit.edu>,
	jack@suse.cz, linux-ext4@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [RFC 0/5] Ext4: Add support for blocksize < pagesize for dioread_nolock
Date: Mon, 4 Nov 2019 21:49:14 +1100	[thread overview]
Message-ID: <20191104104913.GC27115@bobrowski> (raw)
In-Reply-To: <20191104103759.4085C4C046@d06av22.portsmouth.uk.ibm.com>

On Mon, Nov 04, 2019 at 04:07:56PM +0530, Ritesh Harjani wrote:
> On 11/4/19 3:46 PM, Matthew Bobrowski wrote:
> > On Sun, Nov 03, 2019 at 02:16:06PM -0500, Theodore Y. Ts'o wrote:
> > > On Tue, Oct 29, 2019 at 12:49:24PM +0530, Ritesh Harjani wrote:
> > > > 
> > > > So it looks like these failed tests does not seem to be because of this
> > > > patch series. But these are broken in general for at least 1K blocksize.
> > > 
> > > Agreed, I failed to add them to the exclude list for diread_nolock_1k.
> > > Thanks for pointing that out!
> > > 
> > > After looking through these patches, it looks good.  So, I've landed
> > > this series on the ext4 git tree.
> > > 
> > > There are some potential conflicts with Matthew's DIO using imap patch
> > > set.  I tried resolving them in the obvious way (see the tt/mb-dio
> > > branch[1] on ext4.git), and unfortunately, there is a flaky test
> > > failure with generic/270 --- 2 times out 30 runs of generic/270, the
> > > file system is left inconsistent, with problems found in the block
> > > allocation bitmap.
> > > 
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git/log/?h=tt/mb-dio
> > > 
> > > I've verified that generic/270 isn't a problem on -rc3, and it's not a
> > > problem with just your patch series.  So, it's almost certain it's
> > > because I screwed up the merge.  I applied each of Matthew's patch one
> > > at a time, and conflict was in changes in ext4_end_io_dio, which is
> > > dropped in Matthew's patch.  It wasn't obvious though where the
> > > dioread-nolock-1k change should be applied in Matthew's patch series.
> > > Could you take a look?  Thanks!!
> > 
> > Hang on a second.
> > 
> > Are we not prematurely merging this series in with master? I thought
> > that this is something that should've come after the iomap direct I/O
> > port, no? The use of io_end's within the new direct I/O implementation
> > are effectively redundant...
> 
> It sure may be giving a merge conflict (due to io_end structure).
> But this dioread_nolock series was not dependent over iomap series.

Uh ha. Well, there's been a chunk of code injected into
ext4_end_io_dio() here and by me removing it, I'm not entirely sure
what the downstream effects will be for this specific change...

/M