This is a note to let you know that I've just added the patch titled sbitmap: Try each queue to wake up at least one waiter to the 6.1-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: sbitmap-try-each-queue-to-wake-up-at-least-one-waiter.patch and it can be found in the queue-6.1 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. >From 26edb30dd1c0c9be11fa676b4f330ada7b794ba6 Mon Sep 17 00:00:00 2001 From: Gabriel Krisman Bertazi <krisman@xxxxxxx> Date: Tue, 15 Nov 2022 17:45:53 -0500 Subject: sbitmap: Try each queue to wake up at least one waiter From: Gabriel Krisman Bertazi <krisman@xxxxxxx> commit 26edb30dd1c0c9be11fa676b4f330ada7b794ba6 upstream. Jan reported the new algorithm as merged might be problematic if the queue being awaken becomes empty between the waitqueue_active inside sbq_wake_ptr check and the wake up. If that happens, wake_up_nr will not wake up any waiter and we loose too many wake ups. In order to guarantee progress, we need to wake up at least one waiter here, if there are any. This now requires trying to wake up from every queue. Instead of walking through all the queues with sbq_wake_ptr, this call moves the wake up inside that function. In a previous version of the patch, I found that updating wake_index several times when walking through queues had a measurable overhead. This ensures we only update it once, at the end. Fixes: 4f8126bb2308 ("sbitmap: Use single per-bitmap counting to wake up queued tags") Reported-by: Jan Kara <jack@xxxxxxx> Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxx> Reviewed-by: Jan Kara <jack@xxxxxxx> Link: https://lore.kernel.org/r/20221115224553.23594-4-krisman@xxxxxxx Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- lib/sbitmap.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) --- a/lib/sbitmap.c +++ b/lib/sbitmap.c @@ -555,12 +555,12 @@ void sbitmap_queue_min_shallow_depth(str } EXPORT_SYMBOL_GPL(sbitmap_queue_min_shallow_depth); -static struct sbq_wait_state *sbq_wake_ptr(struct sbitmap_queue *sbq) +static void __sbitmap_queue_wake_up(struct sbitmap_queue *sbq, int nr) { int i, wake_index; if (!atomic_read(&sbq->ws_active)) - return NULL; + return; wake_index = atomic_read(&sbq->wake_index); for (i = 0; i < SBQ_WAIT_QUEUES; i++) { @@ -574,20 +574,22 @@ static struct sbq_wait_state *sbq_wake_p */ wake_index = sbq_index_inc(wake_index); - if (waitqueue_active(&ws->wait)) { - if (wake_index != atomic_read(&sbq->wake_index)) - atomic_set(&sbq->wake_index, wake_index); - return ws; - } + /* + * It is sufficient to wake up at least one waiter to + * guarantee forward progress. + */ + if (waitqueue_active(&ws->wait) && + wake_up_nr(&ws->wait, nr)) + break; } - return NULL; + if (wake_index != atomic_read(&sbq->wake_index)) + atomic_set(&sbq->wake_index, wake_index); } void sbitmap_queue_wake_up(struct sbitmap_queue *sbq, int nr) { unsigned int wake_batch = READ_ONCE(sbq->wake_batch); - struct sbq_wait_state *ws = NULL; unsigned int wakeups; if (!atomic_read(&sbq->ws_active)) @@ -599,16 +601,10 @@ void sbitmap_queue_wake_up(struct sbitma do { if (atomic_read(&sbq->completion_cnt) - wakeups < wake_batch) return; - - if (!ws) { - ws = sbq_wake_ptr(sbq); - if (!ws) - return; - } } while (!atomic_try_cmpxchg(&sbq->wakeup_cnt, &wakeups, wakeups + wake_batch)); - wake_up_nr(&ws->wait, wake_batch); + __sbitmap_queue_wake_up(sbq, wake_batch); } EXPORT_SYMBOL_GPL(sbitmap_queue_wake_up); Patches currently in stable-queue which might be from krisman@xxxxxxx are queue-6.1/sbitmap-use-single-per-bitmap-counting-to-wake-up-qu.patch queue-6.1/sbitmap-advance-the-queue-index-before-waking-up-a-queue.patch queue-6.1/sbitmap-try-each-queue-to-wake-up-at-least-one-waiter.patch queue-6.1/wait-return-number-of-exclusive-waiters-awaken.patch