Re: [patch] O_DIRECT: fix the splitting up of contiguous I/O

[Christoph Hellwig]

Andrew, can you please send this on to Linus and -stable ASAP?  It's
causing massive problems for our users.

On Thu, Aug 12, 2010 at 04:50:59PM -0400, Jeff Moyer wrote:
> Hi,
> 
> commit c2c6ca4 (direct-io: do not merge logically non-contiguous
> requests) introduced a bug whereby all O_DIRECT I/Os were submitted a
> page at a time to the block layer.  The problem is that the code
> expected dio->block_in_file to correspond to the current page in the
> dio.  In fact, it corresponds to the previous page submitted via
> submit_page_section.  This was purely an oversight, as the
> dio->cur_page_fs_offset field was introduced for just this purpose.
> This patch simply uses the correct variable when calculating whether
> there is a mismatch between contiguous logical blocks and contiguous
> physical blocks (as described in the comments).
> 
> I also switched the if conditional following this check to an else if,
> to ensure that we never call dio_bio_submit twice for the same dio (in
> theory, this should not happen, anyway).
> 
> I've tested this by running blktrace and verifying that a 64KB I/O was
> submitted as a single I/O.  I also ran the patched kernel through
> xfstests' aio tests using xfs, ext4 (with 1k and 4k block sizes) and
> btrfs and verified that there were no regressions as compared to an
> unpatched kernel.
> 
> Comments, as always, are welcome.
> 
> Cheers,
> Jeff
> 
> Signed-off-by: Jeff Moyer <jmoyer@xxxxxxxxxx>
> Acked-by: Josef Bacik <jbacik@xxxxxxxxxx>
> 
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index 7600aac..445901c 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -632,7 +632,7 @@ static int dio_send_cur_page(struct dio *dio)
>  	int ret = 0;
>  
>  	if (dio->bio) {
> -		loff_t cur_offset = dio->block_in_file << dio->blkbits;
> +		loff_t cur_offset = dio->cur_page_fs_offset;
>  		loff_t bio_next_offset = dio->logical_offset_in_bio +
>  			dio->bio->bi_size;
>  
> @@ -657,7 +657,7 @@ static int dio_send_cur_page(struct dio *dio)
>  		 * Submit now if the underlying fs is about to perform a
>  		 * metadata read
>  		 */
> -		if (dio->boundary)
> +		else if (dio->boundary)
>  			dio_bio_submit(dio);
>  	}
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/