Re: [PATCH v7 11/11] ext4: introduce direct I/O write using iomap infrastructure

From: Matthew Bobrowski <mbobrowski@mbobrowski.org>
To: "Theodore Y. Ts'o" <tytso@mit.edu>
Cc: jack@suse.cz, adilger.kernel@dilger.ca,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	riteshh@linux.ibm.com
Subject: Re: [PATCH v7 11/11] ext4: introduce direct I/O write using iomap infrastructure
Date: Wed, 6 Nov 2019 07:57:13 +1100	[thread overview]
Message-ID: <20191105205712.GB1739@bobrowski> (raw)
In-Reply-To: <20191105162855.GK28764@mit.edu>

On Tue, Nov 05, 2019 at 11:28:55AM -0500, Theodore Y. Ts'o wrote:
> On Tue, Nov 05, 2019 at 11:02:39PM +1100, Matthew Bobrowski wrote:
> > +	ret = iomap_dio_rw(iocb, from, &ext4_iomap_ops, &ext4_dio_write_ops,
> > +			   is_sync_kiocb(iocb) || unaligned_aio || extend);
> > +
> > +	if (extend)
> > +		ret = ext4_handle_inode_extension(inode, offset, ret, count);
> > +
> 
> Can we do a slight optimization here like this?
> 
> 	ret = iomap_dio_rw(iocb, from, &ext4_iomap_ops, &ext4_dio_write_ops,
> 			   is_sync_kiocb(iocb) || unaligned_aio || extend);
> 
> 	if (extend && ret != -EBIOCQUEUED)
> 		ret = ext4_handle_inode_extension(inode, offset, ret, count);
> 
> 
> If iomap_dio_rw() returns -EBIOCQUEUED, there's no need to do any of
> the ext4_handle_inode_extension --- in particular, there's no need to
> call ext4_truncate_failed_write(), which has a bunch of extra
> overhead, including taking and releasing i_data_sem.

Hm, but for extension, unaligned asynchronous IO, or synchronous IO
cases, 'wait_for_completion' within iomap_dio_rw() is set to true and
as a result we'd never expect to receive -EIOCBQUEUED from
iomap_dio_rw()?

So, with that said, would the above change be necessary seeing as
though we'd never expect ret == -EIOCBQUEUED when extend == true?

Maybe I'm missing something?

/M