Received-SPF: pass (google.com: domain of linux-kernel+bounces-153147-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Yang Yang <yang.yang@vivo.com>
To: Alasdair Kergon <agk@redhat.com>,
	Mike Snitzer <snitzer@kernel.org>,
	Mikulas Patocka <mpatocka@redhat.com>,
	dm-devel@lists.linux.dev,
	linux-kernel@vger.kernel.org
Cc: Yang Yang <yang.yang@vivo.com>
Subject: [RFC PATCH] dm: Avoid sending redundant empty flush bios to the same block device
Date: Mon, 22 Apr 2024 18:05:40 +0800
Message-Id: <20240422100540.2213-1-yang.yang@vivo.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?jtCXRhi1svph1waMulrcjRAdVoqDiyFA27Gqmgi4DStVDRt9Olw3VPqrtDnl?=
 =?us-ascii?Q?fAfqsohPEDENlFF2WZj201lkK0vhlbGiWtUi/2feHYabDa700L2FfeuZLR7W?=
 =?us-ascii?Q?+Fl/i51MNss2tEQhRYZYtFgfa5hJeDvUIPt85UQ8zHSUQpf6vz3Xr9K9AfzF?=
 =?us-ascii?Q?dOk3lsXptlrGZ/5a6DFnpxxQSUjJDgYAzgfQW7BN7qwxUTyNm7tsprSQ5Zng?=
 =?us-ascii?Q?gkC682PzTKgb3BhwbReEvylBoeu0qCcSjgmJ1G8OABI6XJ+JfymHOqF0SJlb?=
 =?us-ascii?Q?1FlfnhsgT28OOxzpS2ibjUOUcsDXZY7orXeckyHuBNZ2OTlfBY/1RIae4yYt?=
 =?us-ascii?Q?bpFx6L8ghLo4EK4mFFJN5IDSlMJx0kpS/etgvoLRr8SPUgTvVg96rM28L9eY?=
 =?us-ascii?Q?j8mAgzPJhnD2A9kw9PTet8ds3tqurcFZ6l2WjeJEu+x9wxMfI60uCfW46g5D?=
 =?us-ascii?Q?IA3HEl7t3CXuHiSWxZM7tOkRkvgYc6auaqXyuWaPycYtQg4i2MCpVVriTQfa?=
 =?us-ascii?Q?xeMf33TbbmJKgGvJfodD+Ju0T9xvz9KbLUR+URuXKORVpz8jGthZtjW4mvLj?=
 =?us-ascii?Q?zn5pUN9D1FbzoonuNYoIDNs8D27fUkos6M905xlmgU92ptkDccECLszP8w8P?=
 =?us-ascii?Q?py3QDk37XAWRsM/crdiiwjusGhMUuu+tGb9RE1B8LhRkvzRwzCWwHAzalsl8?=
 =?us-ascii?Q?TgB3j7HREZwRCTaa4u4qCMGGM2y4jMq70e5Bmu+MjudSltp6hRUsXSweSMsT?=
 =?us-ascii?Q?DNuJRjLL7KqCAypiM41sYVw4NKJvRGUYWvvPVZHX1mKbKtYCniYlu3TaYDko?=
 =?us-ascii?Q?jO6XMyMEv46Ex6ZmM2xL0pnsKdhQeL9lX16REUY6osXfd7GeAlxliZuq29Gy?=
 =?us-ascii?Q?QTPnOVUxz2aqdQgyRGKACYQS4DF0Rzdt6HwBb+SS7/aSj5M6tr/HukXHF1e3?=
 =?us-ascii?Q?C38YcnsvcW0Xe7BKOHdsPB1XR3PV6NSAoqAyzvS7gxGrpO4kMStBKdDsW1Cs?=
 =?us-ascii?Q?7Hy9M24U63hNCRUFcEgfazUXdeCdy9HiaTlsi3I3J8wcGMwNF15Eu9bFURKy?=
 =?us-ascii?Q?k7djhUDWbMTW4e/r20EunILrYrV4Z4M9uJp1vOdK2QsCOqZdSbsD7rPJQTt5?=
 =?us-ascii?Q?Z7h4D4x7njDrEDBIgV3U567OeK0GL0JAX/1nM4unWBzhu5V4ocLNjr06GYgJ?=
 =?us-ascii?Q?Y7e3b2pG05xPyntClgGcThCGHy/cVr/+zKhRjtwk2dqN+fdWye8kXEt8PqvS?=
 =?us-ascii?Q?RMvtWNjP3dA6hOUk+G/aCxGQwlz7/7i2L1Q8ljQXaU4+rfSAQU/zd2VJXHDe?=
 =?us-ascii?Q?bvg9mqsDp5eBgsvlle1XRL/eR5t9PZLbWdtYBGmQe6NN6NgGvGDmrBhJ6Nk3?=
 =?us-ascii?Q?vJB9+XWwOiosGFiOxShQy/2kwNZhqdzB4fgftdetI2WNyadBZJoeggosXKiY?=
 =?us-ascii?Q?qV5AT+Mj4QonQDoyRZiliUuzW7kdQ0w1fPjURbm22qY6Kbb9BzQo7RA0e0UY?=
 =?us-ascii?Q?ePdzq2LAFr9UJS7LY7tfFSKxKvCmPFJUOV1ajl+xNpEOW8LS1u+XGLZNZ851?=
 =?us-ascii?Q?kxTXIw1Tn8WyHYfTXwItt8OvOCB4I98LgfWy3oxJ?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ad5b6b13-9915-4321-e6aa-08dc62b3daf1
X-MS-Exchange-CrossTenant-AuthSource: TYSPR06MB6411.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2024 10:06:19.3425
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: J4rCr9U+c9Y65GdfysmbIH5kEyp39oHTW+s1GG8ocWxAvLUTtf8FfrpUsJVoLUL7yp/2883GHZ6K1vIe40bmBg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SI2PR06MB5116

__send_empty_flush() sends empty flush bios to every target in the
dm_table. However, if the num_targets exceeds the number of block
devices in the dm_table's device list, it could lead to multiple
invocations of __send_duplicate_bios() for the same block device.
Typically, a single thread sending numerous empty flush bios to one
block device is redundant, as these bios are likely to be merged by the
flush state machine. In scenarios where num_targets significantly
outweighs the number of block devices, such behavior may result in a
noteworthy decrease in performance.

This issue can be reproduced using this command line:
  for i in {0..1023}; do
    echo $((8000*$i)) 8000 linear /dev/sda2 $((16384*$i))
  done | dmsetup create example

With this fix, a random write with fsync workload executed with the
following fio command:

  fio --group_reporting --name=benchmark --filename=/dev/mapper/example \
      --ioengine=sync --invalidate=1 --numjobs=16 --rw=randwrite \
      --blocksize=4k --size=2G --time_based --runtime=30 --fdatasync=1

results in an increase from 857 KB/s to 30.8 MB/s of the write
throughput (3580% increase).

Signed-off-by: Yang Yang <yang.yang@vivo.com>
---
 drivers/md/dm-core.h          |  1 +
 drivers/md/dm-table.c         |  7 +++++
 drivers/md/dm.c               | 59 +++++++++++++++++++++++++++++++++++
 include/linux/device-mapper.h |  6 ++++
 4 files changed, 73 insertions(+)

diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
index e6757a30dcca..7e3f2168289f 100644
--- a/drivers/md/dm-core.h
+++ b/drivers/md/dm-core.h
@@ -217,6 +217,7 @@ struct dm_table {
 	/* a list of devices used by this table */
 	struct list_head devices;
 	struct rw_semaphore devices_lock;
+	unsigned short num_devices;
 
 	/* events get handed up using this callback */
 	void (*event_fn)(void *data);
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 41f1d731ae5a..ddc60e498afb 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -2133,6 +2133,8 @@ void dm_table_postsuspend_targets(struct dm_table *t)
 
 int dm_table_resume_targets(struct dm_table *t)
 {
+	struct list_head *devices = dm_table_get_devices(t);
+	struct dm_dev_internal *dd;
 	unsigned int i;
 	int r = 0;
 
@@ -2159,6 +2161,11 @@ int dm_table_resume_targets(struct dm_table *t)
 			ti->type->resume(ti);
 	}
 
+	t->num_devices = 0;
+
+	list_for_each_entry(dd, devices, list)
+		dd->dm_dev->index = ++(t->num_devices);
+
 	return 0;
 }
 
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 56aa2a8b9d71..7297235291f6 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -48,6 +48,8 @@
  */
 #define REQ_DM_POLL_LIST	REQ_DRV
 
+#define DM_MAX_TABLE_DEVICES	1024
+
 static const char *_name = DM_NAME;
 
 static unsigned int major;
@@ -1543,10 +1545,38 @@ static unsigned int __send_duplicate_bios(struct clone_info *ci, struct dm_targe
 	return ret;
 }
 
+static inline bool has_redundant_flush(struct dm_table *t,
+		unsigned long **bitmap)
+{
+	if (t->num_devices < t->num_targets) {
+		/* Add a limit here to prevent excessive memory usage for bitmaps */
+		if (t->num_devices >= DM_MAX_TABLE_DEVICES)
+			return false;
+
+		/* dm_dev's index starts from 1, so need plus 1 here */
+		*bitmap = bitmap_zalloc(t->num_devices + 1, GFP_KERNEL);
+		if (*bitmap)
+			return true;
+	}
+
+	return false;
+}
+
+static int dm_get_dev_index(struct dm_target *ti, struct dm_dev *dev,
+				     sector_t start, sector_t len, void *data)
+{
+	unsigned short *index = data;
+	*index = dev->index;
+	return 0;
+}
+
 static void __send_empty_flush(struct clone_info *ci)
 {
 	struct dm_table *t = ci->map;
 	struct bio flush_bio;
+	unsigned long *handled_map;
+	unsigned int nr_handled = 0;
+	bool check = has_redundant_flush(t, &handled_map);
 
 	/*
 	 * Use an on-stack bio for this, it's safe since we don't
@@ -1562,17 +1592,46 @@ static void __send_empty_flush(struct clone_info *ci)
 
 	for (unsigned int i = 0; i < t->num_targets; i++) {
 		unsigned int bios;
+		unsigned short index = 0;
 		struct dm_target *ti = dm_table_get_target(t, i);
 
 		if (unlikely(ti->num_flush_bios == 0))
 			continue;
 
+		/*
+		 * If the num_targets is greater than the number of block devices
+		 * in the dm_table's devices list, __send_empty_flush() might
+		 * invoke __send_duplicate_bios() multiple times for the same
+		 * block device. This could lead to a substantial decrease in
+		 * performance when num_targets significantly exceeds the number
+		 * of block devices.
+		 * Ensure that __send_duplicate_bios() is only called once for
+		 * each block device.
+		 */
+		if (check) {
+			if (nr_handled == t->num_devices)
+				break;
+
+			if (ti->type->iterate_devices)
+				ti->type->iterate_devices(ti, dm_get_dev_index, &index);
+
+			if (index > 0) {
+				if (__test_and_set_bit(index, handled_map))
+					continue;
+				else
+					nr_handled++;
+			}
+		}
+
 		atomic_add(ti->num_flush_bios, &ci->io->io_count);
 		bios = __send_duplicate_bios(ci, ti, ti->num_flush_bios,
 					     NULL, GFP_NOWAIT);
 		atomic_sub(ti->num_flush_bios - bios, &ci->io->io_count);
 	}
 
+	if (check)
+		bitmap_free(handled_map);
+
 	/*
 	 * alloc_io() takes one extra reference for submission, so the
 	 * reference won't reach 0 without the following subtraction
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 82b2195efaca..4a54b4f0a609 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -169,6 +169,12 @@ struct dm_dev {
 	struct dax_device *dax_dev;
 	blk_mode_t mode;
 	char name[16];
+
+	/*
+	 * sequential number for each dm_dev in dm_table's devices list,
+	 * start from 1
+	 */
+	unsigned short index;
 };
 
 /*
-- 
2.34.1