Received-SPF: pass (google.com: domain of linux-kernel+bounces-159879-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Fri, 26 Apr 2024 18:37:27 +0800
From: Xu Yang <xu.yang_2@nxp.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>, "Darrick J. Wong" <djwong@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Matthew Wilcox <willy@infradead.org>,
	Christian Brauner <christian@brauner.io>,
	linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-xfs@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, Luis Chamberlain <mcgrof@kernel.org>,
	Pankaj Raghav <p.raghav@samsung.com>,
	Hannes Reinecke <hare@suse.de>, jun.li@nxp.com, haibo.chen@nxp.com,
	xu.yang_2@nxp.com
Subject: Re: [PATCH 5/6] block: use iomap for writes to block devices
Message-ID: <20240426103727.hzzv4hv54an5jzab@hippo>
References: <20230801172201.1923299-1-hch@lst.de>
 <20230801172201.1923299-6-hch@lst.de>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230801172201.1923299-6-hch@lst.de>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?+s5R28qC/b1d/A/K+m8jq+1usTl0XXrdrkP+soBw1aOSaKSj71yt+lX76Ix2?=
 =?us-ascii?Q?9JgVD826bJx9fnWAo3Nt1DS1iDH8GpCpHVYGS3GLiL+nI1NJzTp3o6NkYCjX?=
 =?us-ascii?Q?AebsRtj3dIReUJXT/ZsGZOST/WbVIJgH7mMEENkdknQ0Q8vb8Go7ahwiAlmj?=
 =?us-ascii?Q?lrs6Z8LVXpW5d9zXstRu7+wjIc9qGoj53wRJprJkq43A+65iWJ4kWJFu4Jy+?=
 =?us-ascii?Q?JhXjgKhSJ4Y236UZXTXRSgEdTEV3ml44RgiJQUABfRqeqn+jk2vV3i4gVf39?=
 =?us-ascii?Q?n0rDSaAe+hW7ewUFOlpJT8nP6cqx50FlUPLnd1POFs6VMx/5s9u8tG13utmc?=
 =?us-ascii?Q?zNhwTuepCKJRm4FO1ansfiVgfCfKV/FIp/+DCKteOTMASZBQPqPlbXJLE0YL?=
 =?us-ascii?Q?87Y0sNAuXj9LGtcbykMlyG52+OOCK4tzS5W3mwSLcXvnupDtDVgFJ2OlvMKO?=
 =?us-ascii?Q?wB8/ZqojgofvJlOPvjV37x5+GtR3fWNy5AAOw2qfgU15uzcVACShGcpPhLLc?=
 =?us-ascii?Q?cotTLaZV8yBwdJGHj10FrEX8cst18JESMjAvYzMUGP/yD8JhOuDCRF4mJNtf?=
 =?us-ascii?Q?SAJAT4h94MfddBmd8kKzz1WPCzdqYjL5ESNu2siw417jiApqeIHzgsj02OdA?=
 =?us-ascii?Q?iiquNbPLm0fM4Kg7SZ8cBr5y7c9MUo5BtKtKou4ibvkPrmW4LXy+aqkd38Po?=
 =?us-ascii?Q?TKZ1jotkaqZEK3FEatq4e14oGERk+uzvYTInHXYfNZvsaLvz2QlBzjipLr7P?=
 =?us-ascii?Q?FMrTh1hlSZvDyEHJQPozntRRFo3asPbkh1VFlXSmwnbSyvuQJaxL8sIUp7gn?=
 =?us-ascii?Q?NsIOExdtL//HTEv+v2XAy04Jp7rg5q4iz/fuDYvA3nnGanHaYJJPFheAyrt3?=
 =?us-ascii?Q?XiMh2oBS0qKbXkAv3Xcyb6L+UAyUZk4XTwCfsGe7sAKEJfH+qobwLf7Kggr8?=
 =?us-ascii?Q?UO4zYKAc0XSa58vBHnq7GDngTAbPebxsYK7T8CCRztZok4W5j6c/89jvd7V1?=
 =?us-ascii?Q?Tko1R1fPQyrSBgi41XkoFjpCxYI91+PZRno8E5H5kWqvSKfe/5ZTFqEtAQKU?=
 =?us-ascii?Q?1OrjbufanoU5L7U1i9hxhNxyK/HVINxnzNelQ16u6r5dQoS6RS96c+lgBgQH?=
 =?us-ascii?Q?ai83fdHo2tl3wLnztVT9GqWrFgexMEXvu/Z779mzM5HzDr7s1Wfg3KeOQUlx?=
 =?us-ascii?Q?KmrAkVu45bKzj2tPxoToYsCbkHLVgagXaJvKLg6XgZ/C+763VwqZ+P7aC1gP?=
 =?us-ascii?Q?3099oxsa4okq23LRqKuAoZMTu/QtNxLcUUoLL7kbL0lYW13o7P2bL06gSJ8L?=
 =?us-ascii?Q?UC52N5H02uPlyeBIimRpXiM6BhfxaZc1MuDY3FOdBdXzdxFTMcenHF4r9xEg?=
 =?us-ascii?Q?/weV7Cgnx4iwenjZQ6sZo2jCWvHY9gaNT+LHNYt1vttN9h0U1neXbbl59NWI?=
 =?us-ascii?Q?03RehLdCwu7msiKFGYLJA81jsQ9r7+EISBPJ/S8Bl8ru+W6UigE/2QTNhmza?=
 =?us-ascii?Q?WmXi78/8CgJrKbKKfE8amTKGdFXOGi6EVdt1btcPEAmr4mIDICKZ04Up2Od9?=
 =?us-ascii?Q?UsnujzoRpf03b1TQ5AyRJZSQ/s2RuWUC6RcQw3wp?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a93843b7-c9dd-4eec-8575-08dc65dd0815
X-MS-Exchange-CrossTenant-AuthSource: DU2PR04MB8822.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2024 10:38:38.2145
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Z97VJEY756hBMEmO7EwYtHN5XZnVXSsfhhnrxQjrNVnXI1RXULwNuhAgW4UsT8i3Zfyyk4FKOxGDrlwePPSmnA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA1PR04MB10225

Hi Christoph,

On Tue, Aug 01, 2023 at 07:22:00PM +0200, Christoph Hellwig wrote:
> Use iomap in buffer_head compat mode to write to block devices.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
> Reviewed-by: Pankaj Raghav <p.raghav@samsung.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> ---
>  block/Kconfig |  1 +
>  block/fops.c  | 31 +++++++++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/block/Kconfig b/block/Kconfig
> index 86122e459fe046..1a13ef0b1ca10c 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -5,6 +5,7 @@
>  menuconfig BLOCK
>         bool "Enable the block layer" if EXPERT
>         default y
> +       select FS_IOMAP
>         select SBITMAP
>         help
>  	 Provide block layer support for the kernel.
> diff --git a/block/fops.c b/block/fops.c
> index f0b822c28ddfe2..063ece37d44e44 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -15,6 +15,7 @@
>  #include <linux/falloc.h>
>  #include <linux/suspend.h>
>  #include <linux/fs.h>
> +#include <linux/iomap.h>
>  #include <linux/module.h>
>  #include "blk.h"
>  
> @@ -386,6 +387,27 @@ static ssize_t blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
>  	return __blkdev_direct_IO(iocb, iter, bio_max_segs(nr_pages));
>  }
>  
> +static int blkdev_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> +		unsigned int flags, struct iomap *iomap, struct iomap *srcmap)
> +{
> +	struct block_device *bdev = I_BDEV(inode);
> +	loff_t isize = i_size_read(inode);
> +
> +	iomap->bdev = bdev;
> +	iomap->offset = ALIGN_DOWN(offset, bdev_logical_block_size(bdev));
> +	if (iomap->offset >= isize)
> +		return -EIO;
> +	iomap->type = IOMAP_MAPPED;
> +	iomap->addr = iomap->offset;
> +	iomap->length = isize - iomap->offset;
> +	iomap->flags |= IOMAP_F_BUFFER_HEAD;
> +	return 0;
> +}
> +
> +static const struct iomap_ops blkdev_iomap_ops = {
> +	.iomap_begin		= blkdev_iomap_begin,
> +};
> +
>  static int blkdev_writepage(struct page *page, struct writeback_control *wbc)
>  {
>  	return block_write_full_page(page, blkdev_get_block, wbc);
> @@ -556,6 +578,11 @@ blkdev_direct_write(struct kiocb *iocb, struct iov_iter *from)
>  	return written;
>  }
>  
> +static ssize_t blkdev_buffered_write(struct kiocb *iocb, struct iov_iter *from)
> +{
> +	return iomap_file_buffered_write(iocb, from, &blkdev_iomap_ops);
> +}
> +
>  /*
>   * Write data to the block device.  Only intended for the block device itself
>   * and the raw driver which basically is a fake block device.
> @@ -605,9 +632,9 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  		ret = blkdev_direct_write(iocb, from);
>  		if (ret >= 0 && iov_iter_count(from))
>  			ret = direct_write_fallback(iocb, from, ret,
> -					generic_perform_write(iocb, from));
> +					blkdev_buffered_write(iocb, from));
>  	} else {
> -		ret = generic_perform_write(iocb, from);
> +		ret = blkdev_buffered_write(iocb, from);
>  	}
>  
>  	if (ret > 0)

I'm testing SSD block device write performance recently. I found the write
speed descrased greatly on my board (330MB/s -> 130MB/s). Then I spent some
time to find cause, finally find that it's caused by this patch and if I
revert this patch, write speed can recover to 330MB/s.

I'm using below command to test write performance:
dd if=/dev/zero of=/dev/sda bs=4M count=1024

And I also do more tests to get more findings. In short, I found write
speed changes with the "bs=" parameter.

I totally write 4GB data to sda for each test, the results as below:

 - dd if=/dev/zero of=/dev/sda bs=400K  count=10485  (334 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=800K  count=5242   (278 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=1600K count=2621   (204 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=2200K count=1906   (170 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=3000K count=1398   (150 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=4500K count=932    (139 MB/s)

When this patch reverted, I got below results:

 - dd if=/dev/zero of=/dev/sda bs=400K  count=10485  (339 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=800K  count=5242   (330 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=1600K count=2621   (332 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=2200K count=1906   (333 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=3000K count=1398   (333 MB/s)
 - dd if=/dev/zero of=/dev/sda bs=4500K count=932    (333 MB/s)

I just want to know if this results is expected when uses iomap, or it's
a real issue?

Many thanks in advance!

Best Regards,
Xu Yang

> -- 
> 2.39.2
>