Received-SPF: pass (google.com: domain of linux-kernel+bounces-199977-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: Jens Axboe <axboe@kernel.dk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Yang Yang <yang.yang@vivo.com>,
	Pavel Begunkov <asml.silence@gmail.com>,
	linux-kernel@vger.kernel.org,
	linux-block@vger.kernel.org
Subject: [PATCH v2] sbitmap: fix io hung due to race on sbitmap_word::cleared
Date: Tue,  4 Jun 2024 11:11:20 +0800
Message-Id: <20240604031124.2261-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?gFdX/zT/gZ1MRDma/YVdgRa+Mu2btuPhkksdzW1f89dOM/WIqq2duEcL9IzN?=
 =?us-ascii?Q?COljqZPGergdhHsA8vfpp2sywQHHUU2uHM0Vgklm2w0NzLMlPk9HmTJLMOJh?=
 =?us-ascii?Q?XDww4oqJ4RYM6ESVxhTNzRngYhbwgp8jNHxsdj5EYbVomUgMXrDEFjy7/Rri?=
 =?us-ascii?Q?I6kzx+En5lHDYRQMHskMsY7scJ6loH99xvfn2O0HkuBbMUI4lTA3AgqSwU9q?=
 =?us-ascii?Q?CUdKA0pAZQEmY8c9FHyf+h9fXxXfJkMRl9kaELdHRVB0V/4PJEjxvsSuzRyb?=
 =?us-ascii?Q?p8gf5RysdX6nORX6MNtF6LYTyUm7WH1f21KOo87sQGHDB8A6n1zwEAqixs+M?=
 =?us-ascii?Q?t345CCqHnROzjZPP9HUj2m4e7w+t4cbBaWhsaBIqegW/Z2JfUKSjBxBTNyL7?=
 =?us-ascii?Q?4Ai7NtO86n79pf3DQ142ANIfklpZ/vHwitEY3ruQkCpNR3ybI4fcZzqLa4qa?=
 =?us-ascii?Q?Lw2ykP/HKDoRgMUWmfw2rwV3UbgQR0lLoPBS0hQIrsgSo8oQZNXJ/vddUuWu?=
 =?us-ascii?Q?dm85KcAmAJiEVaC9Mcy4CGb4ibLgCZ8QnTMLLLXE1av16PPs3m57++8OYeL2?=
 =?us-ascii?Q?j4jXQRz1RQI2fu0o8ZhzIfuJA3568XTxoqM3PG/cSQdxo6e6LVHsCK1wKVtL?=
 =?us-ascii?Q?U/e7889H9WX4iCvMMps4BvoRS8UemPlM8cmxwAygD6WIUyZ9yo2uR9lG4wIN?=
 =?us-ascii?Q?mlg5lKPMdrpaZyHiojta7PQdFfwQZb+mq7u2NJjWIv2H9dvExehlLB4wKF9J?=
 =?us-ascii?Q?xG556NXaa/CP6hUupsgPlpqG1mLbIMdRza9UMWVVptnaw2UNX4T33vLxfK2O?=
 =?us-ascii?Q?dA2TkPgSD2VhS4xJb1lIpUVBwLH5I/NZQ+i5r3NobbOLp2eVTt9kpHf502hY?=
 =?us-ascii?Q?xoNroAAY50cZ7tdFEeJMfV+sbvfWj9uXhkFZgfThsXeiuwYg8vsDEhW1Mihe?=
 =?us-ascii?Q?sGHoqalb2s8r3mEY9H7FFoaIEFhgYPYoR0rsAz2gSFkm85BpUeScHd6RDG+J?=
 =?us-ascii?Q?abRm9qyI0R+91SAKhCZmS0vJUCFJsJgijq6SOkX17GrBYe9cS7FPbyzpp7yo?=
 =?us-ascii?Q?AjiqPz4NC1y1uRxm/KZnVKD8qvvCiVmzh2E8OuihbPfHqfAL3xRDyWLpZ12V?=
 =?us-ascii?Q?N+B/1oGmtl11ct4VYzy4ttGnuq/v9n4/sU5pY8A+1nYOdsX9pq2bM71d71hK?=
 =?us-ascii?Q?VqPI/AMW0beYBrMO1qzBBt+yDIlMqPQfHP8X9iafEANjkYo7wofmp3ORqJln?=
 =?us-ascii?Q?yceV3fOwUtECCiQBYlcAmSUnak2tm/EwoYZJdLtDbHSU5j15bH7Wff7E9QX1?=
 =?us-ascii?Q?uXDb3gqCI6qYx72HvzlLPnuzNul4rc7kJ7rn5dm3F0qXp78EVu4oqZD7x3+T?=
 =?us-ascii?Q?eAMIBpGqShqpLUuC8fav8RYev5xM9gPCwAwIIyiDmXDsJn34ELGlgiYbQYMK?=
 =?us-ascii?Q?lyLVH14qKR4UodOn90DfU09VBXa8UAw8oOF2wg7usmMh7qZs/TXYrL+mHmxA?=
 =?us-ascii?Q?1b90B7ClB4L+Gpicopy/ahzFzb4aEqvgolqBh6fZrmyAtSnEKOCN1WOr4VP2?=
 =?us-ascii?Q?3VkLUylU6fD0C+w2NQRZVQGsititSuW06gdSCbhR?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4eda0607-eb40-4baf-ad2d-08dc84442246
X-MS-Exchange-CrossTenant-AuthSource: KL1PR06MB7401.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2024 03:12:14.9181
 (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: D80DufWL9w8xCinpKT2zbHoJoFnLIA5yWzbqHNKxxdQREiFH6Z12WrMwxngW+E26gkLD293RO2ey71Wz4Ada9Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: OSQPR06MB7232

Configuration for sbq:
  depth=64, wake_batch=6, shift=6, map_nr=1

1. There are 64 requests in progress:
  map->word = 0xFFFFFFFFFFFFFFFF
2. After all the 64 requests complete, and no more requests come:
  map->word = 0xFFFFFFFFFFFFFFFF, map->cleared = 0xFFFFFFFFFFFFFFFF
3. Now two tasks try to allocate requests:
  T1:                                       T2:
  __blk_mq_get_tag                          .
  __sbitmap_queue_get                       .
  sbitmap_get                               .
  sbitmap_find_bit                          .
  sbitmap_find_bit_in_word                  .
  __sbitmap_get_word  -> nr=-1              __blk_mq_get_tag
  sbitmap_deferred_clear                    __sbitmap_queue_get
  /* map->cleared=0xFFFFFFFFFFFFFFFF */     sbitmap_find_bit
    if (!READ_ONCE(map->cleared))           sbitmap_find_bit_in_word
      return false;                         __sbitmap_get_word -> nr=-1
    mask = xchg(&map->cleared, 0)           sbitmap_deferred_clear
    atomic_long_andnot()                    /* map->cleared=0 */
                                              if (!(map->cleared))
                                                return false;
                                     /*
                                      * map->cleared is cleared by T1
                                      * T2 fail to acquire the tag
                                      */

4. T2 is the sole tag waiter. When T1 puts the tag, T2 cannot be woken
up due to the wake_batch being set at 6. If no more requests come, T1
will wait here indefinitely.

To fix this issue, simply revert commit 661d4f55a794 ("sbitmap:
remove swap_lock"), which causes this issue.

Fixes: 661d4f55a794 ("sbitmap: remove swap_lock")
Signed-off-by: Yang Yang <yang.yang@vivo.com>

---
Changes from v1:
  - simply revert commit 661d4f55a794 ("sbitmap: remove swap_lock")
---
 include/linux/sbitmap.h |  5 +++++
 lib/sbitmap.c           | 17 ++++++++++++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/include/linux/sbitmap.h b/include/linux/sbitmap.h
index d662cf136021..ec0b0e73c906 100644
--- a/include/linux/sbitmap.h
+++ b/include/linux/sbitmap.h
@@ -36,6 +36,11 @@ struct sbitmap_word {
 	 * @cleared: word holding cleared bits
 	 */
 	unsigned long cleared ____cacheline_aligned_in_smp;
+
+	/**
+	 * @swap_lock: Held while swapping word <-> cleared
+	 */
+	spinlock_t swap_lock;
 } ____cacheline_aligned_in_smp;
 
 /**
diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index 1e453f825c05..dee02a0266a6 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -63,9 +63,13 @@ static inline void update_alloc_hint_after_get(struct sbitmap *sb,
 static inline bool sbitmap_deferred_clear(struct sbitmap_word *map)
 {
 	unsigned long mask;
+	bool ret = false;
+	unsigned long flags;
 
-	if (!READ_ONCE(map->cleared))
-		return false;
+	spin_lock_irqsave(&map->swap_lock, flags);
+
+	if (!map->cleared)
+		goto out_unlock;
 
 	/*
 	 * First get a stable cleared mask, setting the old mask to 0.
@@ -77,7 +81,10 @@ static inline bool sbitmap_deferred_clear(struct sbitmap_word *map)
 	 */
 	atomic_long_andnot(mask, (atomic_long_t *)&map->word);
 	BUILD_BUG_ON(sizeof(atomic_long_t) != sizeof(map->word));
-	return true;
+	ret = true;
+out_unlock:
+	spin_unlock_irqrestore(&map->swap_lock, flags);
+	return ret;
 }
 
 int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
@@ -85,6 +92,7 @@ int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
 		      bool alloc_hint)
 {
 	unsigned int bits_per_word;
+	int i;
 
 	if (shift < 0)
 		shift = sbitmap_calculate_shift(depth);
@@ -116,6 +124,9 @@ int sbitmap_init_node(struct sbitmap *sb, unsigned int depth, int shift,
 		return -ENOMEM;
 	}
 
+	for (i = 0; i < sb->map_nr; i++)
+		spin_lock_init(&sb->map[i].swap_lock);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(sbitmap_init_node);
-- 
2.34.1