* [PATCH v5 03/21] Add a function to kmap one page of a multipage bio_vec
From: David Howells @ 2026-07-06 15:33 UTC (permalink / raw)
To: Christian Brauner, Matthew Wilcox, Christoph Hellwig
Cc: David Howells, Paulo Alcantara, Jens Axboe, Leon Romanovsky,
Steve French, ChenXiaoSong, Marc Dionne, Stefan Metzmacher,
Eric Van Hensbergen, Dominique Martinet, Ilya Dryomov, netfs,
linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs, linux-erofs,
linux-fsdevel, linux-kernel, linux-block
In-Reply-To: <20260706153408.1231650-1-dhowells@redhat.com>
Add a function to kmap one page of a multipage bio_vec by offset (which is
added to the offset in the bio_vec internally). The caller is responsible
for calculating how much of the page is then available.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
cc: Matthew Wilcox <willy@infradead.org>
cc: Christoph Hellwig <hch@infradead.org>
cc: Jens Axboe <axboe@kernel.dk>
cc: linux-block@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
include/linux/bvec.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/include/linux/bvec.h b/include/linux/bvec.h
index 92837e2743f1..53cf36e73967 100644
--- a/include/linux/bvec.h
+++ b/include/linux/bvec.h
@@ -343,4 +343,22 @@ static inline phys_addr_t bvec_phys(const struct bio_vec *bvec)
return page_to_phys(bvec->bv_page) + bvec->bv_offset;
}
+/**
+ * bvec_kmap_partial - Map part of a bvec into the kernel virtual address space
+ * @bvec: bvec to map
+ * @offset: Offset into bvec
+ *
+ * Map the page containing the byte at @offset into the kernel virtual address
+ * space. The caller is responsible for making sure this doesn't overrun.
+ *
+ * Call kunmap_local on the returned address to unmap.
+ */
+static inline void *bvec_kmap_partial(struct bio_vec *bvec, size_t offset)
+{
+ offset += bvec->bv_offset;
+
+ return kmap_local_page(bvec->bv_page + (offset >> PAGE_SHIFT)) +
+ (offset & ~PAGE_MASK);
+}
+
#endif /* __LINUX_BVEC_H */
^ permalink raw reply related
* [PATCH v5 04/21] iov_iter: Make iov_iter_get_pages*() wrap iov_iter_extract_pages()
From: David Howells @ 2026-07-06 15:33 UTC (permalink / raw)
To: Christian Brauner, Matthew Wilcox, Christoph Hellwig
Cc: David Howells, Paulo Alcantara, Jens Axboe, Leon Romanovsky,
Steve French, ChenXiaoSong, Marc Dionne, Stefan Metzmacher,
Eric Van Hensbergen, Dominique Martinet, Ilya Dryomov, netfs,
linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs, linux-erofs,
linux-fsdevel, linux-kernel, linux-block
In-Reply-To: <20260706153408.1231650-1-dhowells@redhat.com>
Make iov_iter_get_pages*() wrap iov_iter_extract_pages() for kernel
iterator types (e.g. ITER_BVEC, ITER_FOLIOQ, ITER_XARRAY). The pages
obtained have their refcounts incremented afterwards if they're not slab
pages. ITER_KVEC is left returning -EFAULT.
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
cc: Matthew Wilcox <willy@infradead.org>
cc: Christoph Hellwig <hch@infradead.org>
cc: Jens Axboe <axboe@kernel.dk>
cc: linux-block@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
lib/iov_iter.c | 164 ++++++-------------------------------------------
1 file changed, 19 insertions(+), 145 deletions(-)
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index c2484551a4e8..31afc9687eb6 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -910,118 +910,34 @@ static int want_pages_array(struct page ***res, size_t size,
return count;
}
-static ssize_t iter_folioq_get_pages(struct iov_iter *iter,
+/*
+ * Wrap iov_iter_extract_pages() and then pin the non-slab pages we got back.
+ * This only works for non-user iterator types as get_pages uses get_user_pages
+ * not pin_user_pages.
+ */
+static ssize_t iter_get_kernel_pages(struct iov_iter *iter,
struct page ***ppages, size_t maxsize,
unsigned maxpages, size_t *_start_offset)
{
- const struct folio_queue *folioq = iter->folioq;
struct page **pages;
- unsigned int slot = iter->folioq_slot;
- size_t extracted = 0, count = iter->count, iov_offset = iter->iov_offset;
+ ssize_t ret, done;
- if (slot >= folioq_nr_slots(folioq)) {
- folioq = folioq->next;
- slot = 0;
- if (WARN_ON(iov_offset != 0))
- return -EIO;
- }
+ ret = iov_iter_extract_pages(iter, ppages, maxsize, maxpages,
+ 0, _start_offset);
+ if (ret <= 0)
+ return ret;
- maxpages = want_pages_array(ppages, maxsize, iov_offset & ~PAGE_MASK, maxpages);
- if (!maxpages)
- return -ENOMEM;
- *_start_offset = iov_offset & ~PAGE_MASK;
pages = *ppages;
+ for (done = ret + *_start_offset; done > 0; done -= PAGE_SIZE) {
+ struct folio *folio = page_folio(*pages);
- for (;;) {
- struct folio *folio = folioq_folio(folioq, slot);
- size_t offset = iov_offset, fsize = folioq_folio_size(folioq, slot);
- size_t part = PAGE_SIZE - offset % PAGE_SIZE;
-
- if (offset < fsize) {
- part = umin(part, umin(maxsize - extracted, fsize - offset));
- count -= part;
- iov_offset += part;
- extracted += part;
-
- *pages = folio_page(folio, offset / PAGE_SIZE);
- get_page(*pages);
- pages++;
- maxpages--;
- }
-
- if (maxpages == 0 || extracted >= maxsize)
- break;
-
- if (iov_offset >= fsize) {
- iov_offset = 0;
- slot++;
- if (slot == folioq_nr_slots(folioq) && folioq->next) {
- folioq = folioq->next;
- slot = 0;
- }
- }
- }
-
- iter->count = count;
- iter->iov_offset = iov_offset;
- iter->folioq = folioq;
- iter->folioq_slot = slot;
- return extracted;
-}
-
-static ssize_t iter_xarray_populate_pages(struct page **pages, struct xarray *xa,
- pgoff_t index, unsigned int nr_pages)
-{
- XA_STATE(xas, xa, index);
- struct folio *folio;
- unsigned int ret = 0;
-
- rcu_read_lock();
- for (folio = xas_load(&xas); folio; folio = xas_next(&xas)) {
- if (xas_retry(&xas, folio))
- continue;
-
- /* Has the folio moved or been split? */
- if (unlikely(folio != xas_reload(&xas))) {
- xas_reset(&xas);
- continue;
- }
-
- pages[ret] = folio_file_page(folio, xas.xa_index);
- folio_get(folio);
- if (++ret == nr_pages)
- break;
+ if (!folio_test_slab(folio))
+ folio_get(folio);
+ pages++;
}
- rcu_read_unlock();
return ret;
}
-static ssize_t iter_xarray_get_pages(struct iov_iter *i,
- struct page ***pages, size_t maxsize,
- unsigned maxpages, size_t *_start_offset)
-{
- unsigned nr, offset, count;
- pgoff_t index;
- loff_t pos;
-
- pos = i->xarray_start + i->iov_offset;
- index = pos >> PAGE_SHIFT;
- offset = pos & ~PAGE_MASK;
- *_start_offset = offset;
-
- count = want_pages_array(pages, maxsize, offset, maxpages);
- if (!count)
- return -ENOMEM;
- nr = iter_xarray_populate_pages(*pages, i->xarray, index, count);
- if (nr == 0)
- return 0;
-
- maxsize = min_t(size_t, nr * PAGE_SIZE - offset, maxsize);
- i->iov_offset += maxsize;
- i->count -= maxsize;
- return maxsize;
-}
-
/* must be done on non-empty ITER_UBUF or ITER_IOVEC one */
static unsigned long first_iovec_segment(const struct iov_iter *i, size_t *size)
{
@@ -1044,22 +960,6 @@ static unsigned long first_iovec_segment(const struct iov_iter *i, size_t *size)
BUG(); // if it had been empty, we wouldn't get called
}
-/* must be done on non-empty ITER_BVEC one */
-static struct page *first_bvec_segment(const struct iov_iter *i,
- size_t *size, size_t *start)
-{
- struct page *page;
- size_t skip = i->iov_offset, len;
-
- len = i->bvec->bv_len - skip;
- if (*size > len)
- *size = len;
- skip += i->bvec->bv_offset;
- page = i->bvec->bv_page + skip / PAGE_SIZE;
- *start = skip % PAGE_SIZE;
- return page;
-}
-
static ssize_t __iov_iter_get_pages_alloc(struct iov_iter *i,
struct page ***pages, size_t maxsize,
unsigned int maxpages, size_t *start)
@@ -1095,36 +995,10 @@ static ssize_t __iov_iter_get_pages_alloc(struct iov_iter *i,
iov_iter_advance(i, maxsize);
return maxsize;
}
- if (iov_iter_is_bvec(i)) {
- struct page **p;
- struct page *page;
- page = first_bvec_segment(i, &maxsize, start);
- n = want_pages_array(pages, maxsize, *start, maxpages);
- if (!n)
- return -ENOMEM;
- p = *pages;
- for (int k = 0; k < n; k++) {
- struct folio *folio = page_folio(page + k);
- p[k] = page + k;
- if (!folio_test_slab(folio))
- folio_get(folio);
- }
- maxsize = min_t(size_t, maxsize, n * PAGE_SIZE - *start);
- i->count -= maxsize;
- i->iov_offset += maxsize;
- if (i->iov_offset == i->bvec->bv_len) {
- i->iov_offset = 0;
- i->bvec++;
- i->nr_segs--;
- }
- return maxsize;
- }
- if (iov_iter_is_folioq(i))
- return iter_folioq_get_pages(i, pages, maxsize, maxpages, start);
- if (iov_iter_is_xarray(i))
- return iter_xarray_get_pages(i, pages, maxsize, maxpages, start);
- return -EFAULT;
+ if (iov_iter_is_kvec(i))
+ return -EFAULT;
+ return iter_get_kernel_pages(i, pages, maxsize, maxpages, start);
}
ssize_t iov_iter_get_pages2(struct iov_iter *i, struct page **pages,
^ permalink raw reply related
* [PATCH v5 05/21] iov_iter: Add a segmented queue of bio_vec[]
From: David Howells @ 2026-07-06 15:33 UTC (permalink / raw)
To: Christian Brauner, Matthew Wilcox, Christoph Hellwig
Cc: David Howells, Paulo Alcantara, Jens Axboe, Leon Romanovsky,
Steve French, ChenXiaoSong, Marc Dionne, Stefan Metzmacher,
Eric Van Hensbergen, Dominique Martinet, Ilya Dryomov, netfs,
linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs, linux-erofs,
linux-fsdevel, linux-kernel, linux-block
In-Reply-To: <20260706153408.1231650-1-dhowells@redhat.com>
Add the concept of a segmented queue of bio_vec[] arrays. This allows an
indefinite quantity of elements to be handled and allows things like
network filesystems and crypto drivers to glue bits on the ends without
having to reallocate the array.
The bvecq struct that defines each segment also carries capacity/usage
information along with flags indicating whether the constituent memory
regions need freeing or unpinning and the file position of the first
element in a segment. The bvecq structs are refcounted to allow a queue to
be extracted in batches and split between a number of subrequests.
The bvecq can have the bio_vec[] it manages allocated in with it, but this
is not required. A flag is provided for if this is the case as comparing
->bv to ->__bv is not sufficient to detect this case.
Add an iterator type ITER_BVECQ for it. This is intended to replace
ITER_FOLIOQ (and ITER_XARRAY).
Note that the prev pointer is only really needed for iov_iter_revert() and
could be dispensed with if struct iov_iter contained the head information
as well as the current point.
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
cc: Matthew Wilcox <willy@infradead.org>
cc: Christoph Hellwig <hch@infradead.org>
cc: Jens Axboe <axboe@kernel.dk>
cc: linux-block@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
include/linux/bvecq.h | 56 +++++++
include/linux/iov_iter.h | 74 ++++++++-
include/linux/uio.h | 11 ++
lib/iov_iter.c | 324 ++++++++++++++++++++++++++++++++++++-
lib/scatterlist.c | 67 +++++++-
lib/tests/kunit_iov_iter.c | 262 ++++++++++++++++++++++++++++++
6 files changed, 788 insertions(+), 6 deletions(-)
create mode 100644 include/linux/bvecq.h
diff --git a/include/linux/bvecq.h b/include/linux/bvecq.h
new file mode 100644
index 000000000000..15f16f905877
--- /dev/null
+++ b/include/linux/bvecq.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Implementation of a segmented queue of bio_vec[].
+ *
+ * Copyright (C) 2026 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ */
+
+#ifndef _LINUX_BVECQ_H
+#define _LINUX_BVECQ_H
+
+#include <linux/bvec.h>
+
+/*
+ * The type of memory retention used by the elements in bvecq->bv[] and how to
+ * clean it up.
+ */
+enum bvecq_mem {
+ BVECQ_MEM_EXTERNAL, /* Externally retained memory - no freeing */
+ BVECQ_MEM_PAGECACHE, /* Ref'd pagecache pages - must put */
+ BVECQ_MEM_GUP, /* Pinned memory from get_user_pages() - unpin */
+ BVECQ_MEM_ALLOCED, /* Memory alloc'd by bvecq - can be freed/pooled */
+} __mode(byte);
+
+/*
+ * Segmented bio_vec queue.
+ *
+ * These can be linked together to form messages of indefinite length and
+ * iterated over with an ITER_BVECQ iterator. The list is non-circular; next
+ * and prev are NULL at the ends.
+ *
+ * The bv pointer points to the bio_vec array; this may be __bv if allocated
+ * together. The caller is responsible for determining whether or not this is
+ * the case as the array pointed to by bv may be follow on directly from the
+ * bvecq by accident of allocation (ie. ->bv == ->__bv is *not* sufficient to
+ * determine this).
+ *
+ * The file position and discontiguity flag allow non-contiguous data sets to
+ * be chained together, but still teased apart without the need to convert the
+ * info in the bio_vec back into a folio pointer.
+ */
+struct bvecq {
+ struct bvecq *next; /* Next bvec in the list or NULL */
+ struct bvecq *prev; /* Prev bvec in the list or NULL */
+ unsigned long long fpos; /* File position */
+ refcount_t ref;
+ u32 priv; /* Private data */
+ u16 nr_slots; /* Number of elements in bv[] used */
+ u16 max_slots; /* Number of elements allocated in bv[] */
+ enum bvecq_mem mem_type:3; /* What sort of memory and how to free it */
+ bool inline_bv:1; /* T if __bv[] is being used */
+ bool discontig:1; /* T if not contiguous with previous bvecq */
+ struct bio_vec *bv; /* Pointer to array of page fragments */
+ struct bio_vec __bv[]; /* Default array (if ->inline_bv) */
+};
+
+#endif /* _LINUX_BVECQ_H */
diff --git a/include/linux/iov_iter.h b/include/linux/iov_iter.h
index f9a17fbbd398..04b8a6d943fa 100644
--- a/include/linux/iov_iter.h
+++ b/include/linux/iov_iter.h
@@ -10,6 +10,7 @@
#include <linux/uio.h>
#include <linux/bvec.h>
+#include <linux/bvecq.h>
#include <linux/folio_queue.h>
typedef size_t (*iov_step_f)(void *iter_base, size_t progress, size_t len,
@@ -141,6 +142,71 @@ size_t iterate_bvec(struct iov_iter *iter, size_t len, void *priv, void *priv2,
return progress;
}
+/*
+ * Handle ITER_BVECQ.
+ */
+static __always_inline
+size_t iterate_bvecq(struct iov_iter *iter, size_t len, void *priv, void *priv2,
+ iov_step_f step)
+{
+ const struct bvecq *bq = iter->bvecq;
+ unsigned int slot = iter->bvecq_slot;
+ size_t progress = 0, skip = iter->iov_offset;
+
+ do {
+ const struct bio_vec *bvec;
+ struct page *page;
+ size_t poff, plen;
+ void *base;
+
+ if (slot >= bq->nr_slots) {
+ if (!bq->next)
+ break;
+ bq = bq->next;
+ slot = 0;
+ continue;
+ }
+
+ bvec = &bq->bv[slot];
+ /*
+ * The caller must ensure that a slot with bv_len>0 has a valid
+ * bv_page.
+ */
+ page = bvec->bv_page + (bvec->bv_offset + skip) / PAGE_SIZE;
+ poff = (bvec->bv_offset + skip) % PAGE_SIZE;
+ plen = umin(bvec->bv_len - skip, len);
+
+ while (plen > 0) {
+ size_t part, remain, consumed;
+
+ part = umin(plen, PAGE_SIZE - poff);
+ base = kmap_local_page(page) + poff;
+ remain = step(base, progress, part, priv, priv2);
+ kunmap_local(base);
+
+ consumed = part - remain;
+ progress += consumed;
+ skip += consumed;
+ len -= consumed;
+ if (!len || remain)
+ goto stop;
+ page++;
+ poff = 0;
+ plen -= consumed;
+ }
+
+ skip = 0;
+ slot++;
+ } while (len);
+
+stop:
+ iter->bvecq_slot = slot;
+ iter->bvecq = bq;
+ iter->iov_offset = skip;
+ iter->count -= progress;
+ return progress;
+}
+
/*
* Handle ITER_FOLIOQ.
*/
@@ -306,6 +372,8 @@ size_t iterate_and_advance2(struct iov_iter *iter, size_t len, void *priv,
return iterate_bvec(iter, len, priv, priv2, step);
if (iov_iter_is_kvec(iter))
return iterate_kvec(iter, len, priv, priv2, step);
+ if (iov_iter_is_bvecq(iter))
+ return iterate_bvecq(iter, len, priv, priv2, step);
if (iov_iter_is_folioq(iter))
return iterate_folioq(iter, len, priv, priv2, step);
if (iov_iter_is_xarray(iter))
@@ -342,8 +410,8 @@ size_t iterate_and_advance(struct iov_iter *iter, size_t len, void *priv,
* buffer is presented in segments, which for kernel iteration are broken up by
* physical pages and mapped, with the mapped address being presented.
*
- * [!] Note This will only handle BVEC, KVEC, FOLIOQ, XARRAY and DISCARD-type
- * iterators; it will not handle UBUF or IOVEC-type iterators.
+ * [!] Note This will only handle BVEC, KVEC, BVECQ, FOLIOQ, XARRAY and
+ * DISCARD-type iterators; it will not handle UBUF or IOVEC-type iterators.
*
* A step functions, @step, must be provided, one for handling mapped kernel
* addresses and the other is given user addresses which have the potential to
@@ -370,6 +438,8 @@ size_t iterate_and_advance_kernel(struct iov_iter *iter, size_t len, void *priv,
return iterate_bvec(iter, len, priv, priv2, step);
if (iov_iter_is_kvec(iter))
return iterate_kvec(iter, len, priv, priv2, step);
+ if (iov_iter_is_bvecq(iter))
+ return iterate_bvecq(iter, len, priv, priv2, step);
if (iov_iter_is_folioq(iter))
return iterate_folioq(iter, len, priv, priv2, step);
if (iov_iter_is_xarray(iter))
diff --git a/include/linux/uio.h b/include/linux/uio.h
index a9bc5b3067e3..f7cfa6ea8213 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -26,6 +26,7 @@ enum iter_type {
ITER_IOVEC,
ITER_BVEC,
ITER_KVEC,
+ ITER_BVECQ,
ITER_FOLIOQ,
ITER_XARRAY,
ITER_DISCARD,
@@ -68,6 +69,7 @@ struct iov_iter {
const struct iovec *__iov;
const struct kvec *kvec;
const struct bio_vec *bvec;
+ const struct bvecq *bvecq;
const struct folio_queue *folioq;
struct xarray *xarray;
void __user *ubuf;
@@ -77,6 +79,7 @@ struct iov_iter {
};
union {
unsigned long nr_segs;
+ u16 bvecq_slot;
u8 folioq_slot;
loff_t xarray_start;
};
@@ -145,6 +148,11 @@ static inline bool iov_iter_is_discard(const struct iov_iter *i)
return iov_iter_type(i) == ITER_DISCARD;
}
+static inline bool iov_iter_is_bvecq(const struct iov_iter *i)
+{
+ return iov_iter_type(i) == ITER_BVECQ;
+}
+
static inline bool iov_iter_is_folioq(const struct iov_iter *i)
{
return iov_iter_type(i) == ITER_FOLIOQ;
@@ -295,6 +303,9 @@ void iov_iter_kvec(struct iov_iter *i, unsigned int direction, const struct kvec
void iov_iter_bvec(struct iov_iter *i, unsigned int direction, const struct bio_vec *bvec,
unsigned long nr_segs, size_t count);
void iov_iter_discard(struct iov_iter *i, unsigned int direction, size_t count);
+void iov_iter_bvec_queue(struct iov_iter *i, unsigned int direction,
+ const struct bvecq *bvecq,
+ unsigned int first_slot, unsigned int offset, size_t count);
void iov_iter_folio_queue(struct iov_iter *i, unsigned int direction,
const struct folio_queue *folioq,
unsigned int first_slot, unsigned int offset, size_t count);
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 31afc9687eb6..29ec77a0d2b4 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -538,6 +538,40 @@ static void iov_iter_iovec_advance(struct iov_iter *i, size_t size)
i->__iov = iov;
}
+static void iov_iter_bvecq_advance(struct iov_iter *i, size_t by)
+{
+ const struct bvecq *bq = i->bvecq;
+ unsigned int slot = i->bvecq_slot;
+
+ if (!i->count)
+ return;
+ i->count -= by;
+
+ by += i->iov_offset; /* From beginning of current segment. */
+ do {
+ size_t len;
+
+ if (slot >= bq->nr_slots) {
+ if (!bq->next)
+ break;
+ bq = bq->next;
+ slot = 0;
+ continue;
+ }
+
+ len = bq->bv[slot].bv_len;
+
+ if (likely(by < len))
+ break;
+ by -= len;
+ slot++;
+ } while (by);
+
+ i->iov_offset = by;
+ i->bvecq_slot = slot;
+ i->bvecq = bq;
+}
+
static void iov_iter_folioq_advance(struct iov_iter *i, size_t size)
{
const struct folio_queue *folioq = i->folioq;
@@ -583,6 +617,8 @@ void iov_iter_advance(struct iov_iter *i, size_t size)
iov_iter_iovec_advance(i, size);
} else if (iov_iter_is_bvec(i)) {
iov_iter_bvec_advance(i, size);
+ } else if (iov_iter_is_bvecq(i)) {
+ iov_iter_bvecq_advance(i, size);
} else if (iov_iter_is_folioq(i)) {
iov_iter_folioq_advance(i, size);
} else if (iov_iter_is_discard(i)) {
@@ -591,6 +627,33 @@ void iov_iter_advance(struct iov_iter *i, size_t size)
}
EXPORT_SYMBOL(iov_iter_advance);
+static void iov_iter_bvecq_revert(struct iov_iter *i, size_t unroll)
+{
+ const struct bvecq *bq = i->bvecq;
+ unsigned int slot = i->bvecq_slot;
+
+ for (;;) {
+ size_t len;
+
+ if (slot == 0) {
+ bq = bq->prev;
+ slot = bq->nr_slots;
+ continue;
+ }
+ slot--;
+
+ len = bq->bv[slot].bv_len;
+ if (unroll <= len) {
+ i->iov_offset = len - unroll;
+ break;
+ }
+ unroll -= len;
+ }
+
+ i->bvecq_slot = slot;
+ i->bvecq = bq;
+}
+
static void iov_iter_folioq_revert(struct iov_iter *i, size_t unroll)
{
const struct folio_queue *folioq = i->folioq;
@@ -648,6 +711,9 @@ void iov_iter_revert(struct iov_iter *i, size_t unroll)
}
unroll -= n;
}
+ } else if (iov_iter_is_bvecq(i)) {
+ i->iov_offset = 0;
+ iov_iter_bvecq_revert(i, unroll);
} else if (iov_iter_is_folioq(i)) {
i->iov_offset = 0;
iov_iter_folioq_revert(i, unroll);
@@ -678,9 +744,24 @@ size_t iov_iter_single_seg_count(const struct iov_iter *i)
if (iov_iter_is_bvec(i))
return min(i->count, i->bvec->bv_len - i->iov_offset);
}
+ if (!i->count)
+ return 0;
+ if (unlikely(iov_iter_is_bvecq(i))) {
+ const struct bvecq *bq = i->bvecq;
+ unsigned int slot = i->bvecq_slot;
+ size_t offset = i->iov_offset;
+
+ while (slot >= bq->nr_slots) {
+ bq = bq->next;
+ if (!bq)
+ return 0;
+ slot = 0;
+ offset = 0;
+ }
+ return umin(i->count, bq->bv[slot].bv_len - offset);
+ }
if (unlikely(iov_iter_is_folioq(i)))
- return !i->count ? 0 :
- umin(folioq_folio_size(i->folioq, i->folioq_slot), i->count);
+ return umin(folioq_folio_size(i->folioq, i->folioq_slot), i->count);
return i->count;
}
EXPORT_SYMBOL(iov_iter_single_seg_count);
@@ -717,6 +798,35 @@ void iov_iter_bvec(struct iov_iter *i, unsigned int direction,
}
EXPORT_SYMBOL(iov_iter_bvec);
+/**
+ * iov_iter_bvec_queue - Initialise an I/O iterator to use a segmented bvec queue
+ * @i: The iterator to initialise.
+ * @direction: The direction of the transfer.
+ * @bvecq: The starting point in the bvec queue.
+ * @first_slot: The first slot in the bvec queue to use
+ * @offset: The offset into the bvec in the first slot to start at
+ * @count: The size of the I/O buffer in bytes.
+ *
+ * Set up an I/O iterator to either draw data out of the buffers attached to an
+ * inode or to inject data into those buffers. The pages *must* be prevented
+ * from evaporation, either by the caller.
+ */
+void iov_iter_bvec_queue(struct iov_iter *i, unsigned int direction,
+ const struct bvecq *bvecq, unsigned int first_slot,
+ unsigned int offset, size_t count)
+{
+ WARN_ON(direction & ~(READ | WRITE));
+ *i = (struct iov_iter) {
+ .iter_type = ITER_BVECQ,
+ .data_source = direction,
+ .bvecq = bvecq,
+ .bvecq_slot = first_slot,
+ .count = count,
+ .iov_offset = offset,
+ };
+}
+EXPORT_SYMBOL(iov_iter_bvec_queue);
+
/**
* iov_iter_folio_queue - Initialise an I/O iterator to use the folios in a folio queue
* @i: The iterator to initialise.
@@ -839,6 +949,37 @@ static unsigned long iov_iter_alignment_bvec(const struct iov_iter *i)
return res;
}
+static unsigned long iov_iter_alignment_bvecq(const struct iov_iter *iter)
+{
+ const struct bvecq *bq;
+ unsigned long res = 0;
+ unsigned int slot = iter->bvecq_slot;
+ size_t skip = iter->iov_offset;
+ size_t size = iter->count;
+
+ if (!size)
+ return res;
+
+ for (bq = iter->bvecq; bq; bq = bq->next) {
+ for (; slot < bq->nr_slots; slot++) {
+ const struct bio_vec *bvec = &bq->bv[slot];
+ size_t part = umin(bvec->bv_len - skip, size);
+
+ res |= bvec->bv_offset + skip;
+ res |= part;
+
+ size -= part;
+ if (size == 0)
+ return res;
+ skip = 0;
+ }
+
+ slot = 0;
+ }
+
+ return res;
+}
+
unsigned long iov_iter_alignment(const struct iov_iter *i)
{
if (likely(iter_is_ubuf(i))) {
@@ -854,6 +995,8 @@ unsigned long iov_iter_alignment(const struct iov_iter *i)
if (iov_iter_is_bvec(i))
return iov_iter_alignment_bvec(i);
+ if (iov_iter_is_bvecq(i))
+ return iov_iter_alignment_bvecq(i);
/* With both xarray and folioq types, we're dealing with whole folios. */
if (iov_iter_is_folioq(i))
@@ -1066,6 +1209,36 @@ static int bvec_npages(const struct iov_iter *i, int maxpages)
return npages;
}
+static size_t iov_npages_bvecq(const struct iov_iter *iter, size_t maxpages)
+{
+ const struct bvecq *bq;
+ unsigned int slot = iter->bvecq_slot;
+ size_t npages = 0;
+ size_t skip = iter->iov_offset;
+ size_t size = iter->count;
+
+ for (bq = iter->bvecq; bq; bq = bq->next) {
+ for (; slot < bq->nr_slots; slot++) {
+ const struct bio_vec *bvec = &bq->bv[slot];
+ size_t offs = (bvec->bv_offset + skip) % PAGE_SIZE;
+ size_t part = umin(bvec->bv_len - skip, size);
+
+ npages += DIV_ROUND_UP(offs + part, PAGE_SIZE);
+ if (npages >= maxpages)
+ goto out;
+
+ size -= part;
+ if (!size)
+ goto out;
+ skip = 0;
+ }
+
+ slot = 0;
+ }
+out:
+ return umin(npages, maxpages);
+}
+
int iov_iter_npages(const struct iov_iter *i, int maxpages)
{
if (unlikely(!i->count))
@@ -1080,6 +1253,8 @@ int iov_iter_npages(const struct iov_iter *i, int maxpages)
return iov_npages(i, maxpages);
if (iov_iter_is_bvec(i))
return bvec_npages(i, maxpages);
+ if (iov_iter_is_bvecq(i))
+ return iov_npages_bvecq(i, maxpages);
if (iov_iter_is_folioq(i)) {
unsigned offset = i->iov_offset % PAGE_SIZE;
int npages = DIV_ROUND_UP(offset + i->count, PAGE_SIZE);
@@ -1366,6 +1541,147 @@ void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state)
i->nr_segs = state->nr_segs;
}
+/*
+ * Count the number of virtually contiguous pages coming up next in an
+ * ITER_BVECQ iterator, up to the specified maxima.
+ */
+static unsigned int iter_count_bvecq_pages(const struct iov_iter *iter,
+ size_t maxsize,
+ unsigned int maxpages)
+{
+ const struct bvecq *bvecq = iter->bvecq;
+ unsigned int slot = iter->bvecq_slot;
+ ssize_t remain = umin(maxsize, iter->count);
+ size_t count = 0, offset = iter->iov_offset;
+
+ do {
+ const struct bio_vec *bv;
+ size_t boff, blen;
+
+ if (slot >= bvecq->nr_slots) {
+ if (!bvecq->next) {
+ WARN_ON_ONCE(remain > 0);
+ break;
+ }
+ bvecq = bvecq->next;
+ slot = 0;
+ offset = 0;
+ continue;
+ }
+
+ bv = &bvecq->bv[slot++];
+ boff = bv->bv_offset;
+ blen = bv->bv_len;
+
+ if (unlikely(!bv->bv_page)) {
+ if (blen && count > 0)
+ break;
+ continue;
+ }
+ if (!PAGE_ALIGNED(boff) && count > 0)
+ break;
+
+ boff += offset;
+ blen -= offset;
+ offset = 0;
+ if (!blen)
+ continue;
+
+ blen = umin(blen, remain);
+ remain -= blen;
+ blen += offset_in_page(boff);
+ count += DIV_ROUND_UP(blen, PAGE_SIZE);
+
+ if (!PAGE_ALIGNED(blen))
+ break;
+ } while (remain > 0 && count < maxpages);
+
+ return umin(count, maxpages);
+}
+
+/*
+ * Extract a list of virtually contiguous pages from an ITER_BVECQ iterator.
+ * This does not get references on the pages, nor does it get a pin on them.
+ */
+static ssize_t iov_iter_extract_bvecq_pages(struct iov_iter *iter,
+ struct page ***pages, size_t maxsize,
+ unsigned int maxpages,
+ iov_iter_extraction_t extraction_flags,
+ size_t *offset0)
+{
+ const struct bvecq *bvecq;
+ struct page **p;
+ unsigned int slot, nr = 0;
+ size_t extracted = 0, offset;
+
+ /* Count the next run of virtually contiguous pages. */
+ maxpages = iter_count_bvecq_pages(iter, maxsize, maxpages);
+
+ if (!*pages) {
+ *pages = kvmalloc_array(maxpages, sizeof(struct page *), GFP_KERNEL);
+ if (!*pages)
+ return -ENOMEM;
+ }
+
+ p = *pages;
+
+ /* Now transcribe the page pointers. */
+ extracted = 0;
+ bvecq = iter->bvecq;
+ offset = iter->iov_offset;
+ slot = iter->bvecq_slot;
+
+ do {
+ const struct bio_vec *bv;
+ size_t boff, blen;
+
+ if (slot >= bvecq->nr_slots) {
+ if (!bvecq->next) {
+ WARN_ON_ONCE(extracted < iter->count);
+ break;
+ }
+ bvecq = bvecq->next;
+ slot = 0;
+ offset = 0;
+ continue;
+ }
+
+ bv = &bvecq->bv[slot];
+ boff = bv->bv_offset;
+ blen = bv->bv_len;
+
+ if (!bv->bv_page)
+ blen = 0;
+
+ if (offset < blen) {
+ size_t part = umin(maxsize - extracted, blen - offset);
+ size_t poff = (boff + offset) % PAGE_SIZE;
+ size_t pix = (boff + offset) / PAGE_SIZE;
+
+ if (poff + part > PAGE_SIZE)
+ part = PAGE_SIZE - poff;
+
+ if (!extracted)
+ *offset0 = poff;
+
+ p[nr++] = bv->bv_page + pix;
+ offset += part;
+ extracted += part;
+ }
+
+ if (offset >= blen) {
+ offset = 0;
+ slot++;
+ }
+ } while (nr < maxpages && extracted < maxsize);
+
+ iter->bvecq = bvecq;
+ iter->bvecq_slot = slot;
+ iter->iov_offset = offset;
+ iter->count -= extracted;
+ return extracted;
+}
+
/*
* Extract a list of contiguous pages from an ITER_FOLIOQ iterator. This does
* not get references on the pages, nor does it get a pin on them.
@@ -1726,6 +2042,10 @@ ssize_t iov_iter_extract_pages(struct iov_iter *i,
return iov_iter_extract_bvec_pages(i, pages, maxsize,
maxpages, extraction_flags,
offset0);
+ if (iov_iter_is_bvecq(i))
+ return iov_iter_extract_bvecq_pages(i, pages, maxsize,
+ maxpages, extraction_flags,
+ offset0);
if (iov_iter_is_folioq(i))
return iov_iter_extract_folioq_pages(i, pages, maxsize,
maxpages, extraction_flags,
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 6ea40d2e6247..23e5a180103b 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -10,6 +10,7 @@
#include <linux/highmem.h>
#include <linux/kmemleak.h>
#include <linux/bvec.h>
+#include <linux/bvecq.h>
#include <linux/uio.h>
#include <linux/folio_queue.h>
@@ -1267,6 +1268,65 @@ static ssize_t extract_kvec_to_sg(struct iov_iter *iter,
return ret;
}
+/*
+ * Extract up to sg_max folios from an BVECQ-type iterator and add them to
+ * the scatterlist. The pages are not pinned.
+ */
+static ssize_t extract_bvecq_to_sg(struct iov_iter *iter,
+ ssize_t maxsize,
+ struct sg_table *sgtable,
+ unsigned int sg_max,
+ iov_iter_extraction_t extraction_flags)
+{
+ const struct bvecq *bvecq = iter->bvecq;
+ struct scatterlist *sg = sgtable->sgl + sgtable->nents;
+ unsigned int slot = iter->bvecq_slot;
+ ssize_t ret = 0;
+ size_t offset = iter->iov_offset;
+
+ maxsize = umin(maxsize, iov_iter_count(iter));
+
+ while (sg_max > 0 && ret < maxsize) {
+ const struct bio_vec *bv;
+ size_t blen, part;
+
+ if (slot >= bvecq->nr_slots) {
+ if (!bvecq->next) {
+ WARN_ON_ONCE(ret < iter->count);
+ break;
+ }
+ bvecq = bvecq->next;
+ slot = 0;
+ offset = 0;
+ continue;
+ }
+
+ bv = &bvecq->bv[slot];
+ blen = bv->bv_len;
+
+ if (offset >= blen) {
+ offset = 0;
+ slot++;
+ continue;
+ }
+
+ part = umin(maxsize - ret, blen - offset);
+
+ sg_set_page(sg, bv->bv_page, part, bv->bv_offset + offset);
+ sgtable->nents++;
+ sg++;
+ sg_max--;
+ offset += part;
+ ret += part;
+ }
+
+ iter->bvecq = bvecq;
+ iter->bvecq_slot = slot;
+ iter->iov_offset = offset;
+ iter->count -= ret;
+ return ret;
+}
+
/*
* Extract up to sg_max folios from an FOLIOQ-type iterator and add them to
* the scatterlist. The pages are not pinned.
@@ -1391,8 +1451,8 @@ static ssize_t extract_xarray_to_sg(struct iov_iter *iter,
* addition of @sg_max elements.
*
* The pages referred to by UBUF- and IOVEC-type iterators are extracted and
- * pinned; BVEC-, KVEC-, FOLIOQ- and XARRAY-type are extracted but aren't
- * pinned; DISCARD-type is not supported.
+ * pinned; BVEC-, BVECQ-, KVEC-, FOLIOQ- and XARRAY-type are extracted but
+ * aren't pinned; DISCARD-type is not supported.
*
* No end mark is placed on the scatterlist; that's left to the caller.
*
@@ -1424,6 +1484,9 @@ ssize_t extract_iter_to_sg(struct iov_iter *iter, size_t maxsize,
case ITER_KVEC:
return extract_kvec_to_sg(iter, maxsize, sgtable, sg_max,
extraction_flags);
+ case ITER_BVECQ:
+ return extract_bvecq_to_sg(iter, maxsize, sgtable, sg_max,
+ extraction_flags);
case ITER_FOLIOQ:
return extract_folioq_to_sg(iter, maxsize, sgtable, sg_max,
extraction_flags);
diff --git a/lib/tests/kunit_iov_iter.c b/lib/tests/kunit_iov_iter.c
index d9690ba1db88..8426c33e48a4 100644
--- a/lib/tests/kunit_iov_iter.c
+++ b/lib/tests/kunit_iov_iter.c
@@ -12,6 +12,7 @@
#include <linux/mm.h>
#include <linux/uio.h>
#include <linux/bvec.h>
+#include <linux/bvecq.h>
#include <linux/folio_queue.h>
#include <linux/scatterlist.h>
#include <linux/minmax.h>
@@ -552,6 +553,185 @@ static void __init iov_kunit_copy_from_folioq(struct kunit *test)
KUNIT_SUCCEED(test);
}
+static void iov_kunit_destroy_bvecq(void *data)
+{
+ struct bvecq *bq, *next;
+
+ for (bq = data; bq; bq = next) {
+ next = bq->next;
+ for (int i = 0; i < bq->nr_slots; i++)
+ if (bq->bv[i].bv_page)
+ put_page(bq->bv[i].bv_page);
+ kfree(bq);
+ }
+}
+
+static struct bvecq *iov_kunit_alloc_bvecq(struct kunit *test, unsigned int max_slots)
+{
+ struct bvecq *bq;
+
+ bq = kzalloc(struct_size(bq, __bv, max_slots), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bq);
+ bq->max_slots = max_slots;
+ bq->bv = bq->__bv;
+ bq->inline_bv = true;
+ return bq;
+}
+
+static struct bvecq *iov_kunit_create_bvecq(struct kunit *test, unsigned int max_slots)
+{
+ struct bvecq *bq;
+
+ bq = iov_kunit_alloc_bvecq(test, max_slots);
+ kunit_add_action_or_reset(test, iov_kunit_destroy_bvecq, bq);
+ return bq;
+}
+
+static void __init iov_kunit_load_bvecq(struct kunit *test,
+ struct iov_iter *iter, int dir,
+ struct bvecq *bq_head,
+ struct page **pages, size_t npages)
+{
+ struct bvecq *bq = bq_head;
+ size_t size = 0;
+
+ for (int i = 0; i < npages; i++) {
+ if (bq->nr_slots >= bq->max_slots) {
+ bq->next = iov_kunit_alloc_bvecq(test, 13);
+ bq->next->prev = bq;
+ bq = bq->next;
+ }
+ bvec_set_page(&bq->bv[bq->nr_slots], pages[i], PAGE_SIZE, 0);
+ bq->nr_slots++;
+ size += PAGE_SIZE;
+ }
+ iov_iter_bvec_queue(iter, dir, bq_head, 0, 0, size);
+}
+
+/*
+ * Test copying to a ITER_BVECQ-type iterator.
+ */
+static void __init iov_kunit_copy_to_bvecq(struct kunit *test)
+{
+ const struct kvec_test_range *pr;
+ struct iov_iter iter;
+ struct bvecq *bq;
+ struct page **spages, **bpages;
+ u8 *scratch, *buffer;
+ size_t bufsize, npages, size, copied;
+ int i, patt;
+
+ bufsize = 0x100000;
+ npages = bufsize / PAGE_SIZE;
+
+ bq = iov_kunit_create_bvecq(test, 13);
+
+ scratch = iov_kunit_create_buffer(test, &spages, npages);
+ for (i = 0; i < bufsize; i++)
+ scratch[i] = pattern(i);
+
+ buffer = iov_kunit_create_buffer(test, &bpages, npages);
+ memset(buffer, 0, bufsize);
+
+ iov_kunit_load_bvecq(test, &iter, READ, bq, bpages, npages);
+
+ i = 0;
+ for (pr = kvec_test_ranges; pr->from >= 0; pr++) {
+ size = pr->to - pr->from;
+ KUNIT_ASSERT_LE(test, pr->to, bufsize);
+
+ iov_iter_bvec_queue(&iter, READ, bq, 0, 0, pr->to);
+ iov_iter_advance(&iter, pr->from);
+ copied = copy_to_iter(scratch + i, size, &iter);
+
+ KUNIT_EXPECT_EQ(test, copied, size);
+ KUNIT_EXPECT_EQ(test, iter.count, 0);
+ i += size;
+ if (test->status == KUNIT_FAILURE)
+ goto stop;
+ }
+
+ /* Build the expected image in the scratch buffer. */
+ patt = 0;
+ memset(scratch, 0, bufsize);
+ for (pr = kvec_test_ranges; pr->from >= 0; pr++)
+ for (i = pr->from; i < pr->to; i++)
+ scratch[i] = pattern(patt++);
+
+ /* Compare the images */
+ for (i = 0; i < bufsize; i++) {
+ KUNIT_EXPECT_EQ_MSG(test, buffer[i], scratch[i], "at i=%x", i);
+ if (buffer[i] != scratch[i])
+ return;
+ }
+
+stop:
+ KUNIT_SUCCEED(test);
+}
+
+/*
+ * Test copying from a ITER_BVECQ-type iterator.
+ */
+static void __init iov_kunit_copy_from_bvecq(struct kunit *test)
+{
+ const struct kvec_test_range *pr;
+ struct iov_iter iter;
+ struct bvecq *bq;
+ struct page **spages, **bpages;
+ u8 *scratch, *buffer;
+ size_t bufsize, npages, size, copied;
+ int i, j;
+
+ bufsize = 0x100000;
+ npages = bufsize / PAGE_SIZE;
+
+ bq = iov_kunit_create_bvecq(test, 13);
+
+ buffer = iov_kunit_create_buffer(test, &bpages, npages);
+ for (i = 0; i < bufsize; i++)
+ buffer[i] = pattern(i);
+
+ scratch = iov_kunit_create_buffer(test, &spages, npages);
+ memset(scratch, 0, bufsize);
+
+ iov_kunit_load_bvecq(test, &iter, READ, bq, bpages, npages);
+
+ i = 0;
+ for (pr = kvec_test_ranges; pr->from >= 0; pr++) {
+ size = pr->to - pr->from;
+ KUNIT_ASSERT_LE(test, pr->to, bufsize);
+
+ iov_iter_bvec_queue(&iter, WRITE, bq, 0, 0, pr->to);
+ iov_iter_advance(&iter, pr->from);
+ copied = copy_from_iter(scratch + i, size, &iter);
+
+ KUNIT_EXPECT_EQ(test, copied, size);
+ KUNIT_EXPECT_EQ(test, iter.count, 0);
+ i += size;
+ }
+
+ /* Build the expected image in the main buffer. */
+ i = 0;
+ memset(buffer, 0, bufsize);
+ for (pr = kvec_test_ranges; pr->from >= 0; pr++) {
+ for (j = pr->from; j < pr->to; j++) {
+ buffer[i++] = pattern(j);
+ if (i >= bufsize)
+ goto stop;
+ }
+ }
+stop:
+
+ /* Compare the images */
+ for (i = 0; i < bufsize; i++) {
+ KUNIT_EXPECT_EQ_MSG(test, scratch[i], buffer[i], "at i=%x", i);
+ if (scratch[i] != buffer[i])
+ return;
+ }
+
+ KUNIT_SUCCEED(test);
+}
+
static void iov_kunit_destroy_xarray(void *data)
{
struct xarray *xarray = data;
@@ -867,6 +1047,85 @@ static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
KUNIT_SUCCEED(test);
}
+/*
+ * Test the extraction of ITER_BVECQ-type iterators.
+ */
+static void __init iov_kunit_extract_pages_bvecq(struct kunit *test)
+{
+ const struct kvec_test_range *pr;
+ struct iov_iter iter;
+ struct bvecq *bq;
+ struct page **bpages, *pagelist[8], **pages = pagelist;
+ ssize_t len;
+ size_t bufsize, size = 0, npages;
+ int i, from;
+
+ bufsize = 0x100000;
+ npages = bufsize / PAGE_SIZE;
+
+ bq = iov_kunit_create_bvecq(test, 13);
+
+ iov_kunit_create_buffer(test, &bpages, npages);
+ iov_kunit_load_bvecq(test, &iter, READ, bq, bpages, npages);
+
+ for (pr = kvec_test_ranges; pr->from >= 0; pr++) {
+ from = pr->from;
+ size = pr->to - from;
+ KUNIT_ASSERT_LE(test, pr->to, bufsize);
+
+ iov_iter_bvec_queue(&iter, WRITE, bq, 0, 0, pr->to);
+ iov_iter_advance(&iter, from);
+
+ do {
+ size_t offset0 = LONG_MAX;
+
+ for (i = 0; i < ARRAY_SIZE(pagelist); i++)
+ pagelist[i] = (void *)(unsigned long)0xaa55aa55aa55aa55ULL;
+
+ len = iov_iter_extract_pages(&iter, &pages, 100 * 1024,
+ ARRAY_SIZE(pagelist), 0, &offset0);
+ KUNIT_EXPECT_GE(test, len, 0);
+ if (len < 0)
+ break;
+ KUNIT_EXPECT_LE(test, len, size);
+ KUNIT_EXPECT_EQ(test, iter.count, size - len);
+ if (len == 0)
+ break;
+ size -= len;
+ KUNIT_EXPECT_GE(test, (ssize_t)offset0, 0);
+ KUNIT_EXPECT_LT(test, offset0, PAGE_SIZE);
+
+ for (i = 0; i < ARRAY_SIZE(pagelist); i++) {
+ struct page *p;
+ ssize_t part = min_t(ssize_t, len, PAGE_SIZE - offset0);
+ int ix;
+
+ KUNIT_ASSERT_GE(test, part, 0);
+ ix = from / PAGE_SIZE;
+ KUNIT_ASSERT_LT(test, ix, npages);
+ p = bpages[ix];
+ KUNIT_EXPECT_PTR_EQ(test, pagelist[i], p);
+ KUNIT_EXPECT_EQ(test, offset0, from % PAGE_SIZE);
+ from += part;
+ len -= part;
+ KUNIT_ASSERT_GE(test, len, 0);
+ if (len == 0)
+ break;
+ offset0 = 0;
+ }
+
+ if (test->status == KUNIT_FAILURE)
+ goto stop;
+ } while (iov_iter_count(&iter) > 0);
+
+ KUNIT_EXPECT_EQ(test, size, 0);
+ KUNIT_EXPECT_EQ(test, iter.count, 0);
+ }
+
+stop:
+ KUNIT_SUCCEED(test);
+}
+
/*
* Test the extraction of ITER_FOLIOQ-type iterators.
*/
@@ -1226,12 +1485,15 @@ static struct kunit_case __refdata iov_kunit_cases[] = {
KUNIT_CASE(iov_kunit_copy_from_kvec),
KUNIT_CASE(iov_kunit_copy_to_bvec),
KUNIT_CASE(iov_kunit_copy_from_bvec),
+ KUNIT_CASE(iov_kunit_copy_to_bvecq),
+ KUNIT_CASE(iov_kunit_copy_from_bvecq),
KUNIT_CASE(iov_kunit_copy_to_folioq),
KUNIT_CASE(iov_kunit_copy_from_folioq),
KUNIT_CASE(iov_kunit_copy_to_xarray),
KUNIT_CASE(iov_kunit_copy_from_xarray),
KUNIT_CASE(iov_kunit_extract_pages_kvec),
KUNIT_CASE(iov_kunit_extract_pages_bvec),
+ KUNIT_CASE(iov_kunit_extract_pages_bvecq),
KUNIT_CASE(iov_kunit_extract_pages_folioq),
KUNIT_CASE(iov_kunit_extract_pages_xarray),
KUNIT_CASE(iov_kunit_iter_to_sg_kvec),
^ permalink raw reply related
* [PATCH] nbd: reclassify socket lock in nbd_reconnect_socket
From: Yun Zhou @ 2026-07-06 15:57 UTC (permalink / raw)
To: josef, axboe; +Cc: linux-block, nbd, linux-kernel, yun.zhou
nbd_reconnect_socket() obtains a new socket via nbd_get_socket() but
does not call nbd_reclassify_socket(), leaving the socket's sk_lock in
the default AF_INET6 lockdep class. This causes false circular lock
dependency warnings because lockdep merges the NBD socket's sk_lock
with unrelated sockets (e.g., rawv6_sendmsg) that establish a
sk_lock -> fs_reclaim dependency in normal operation.
nbd_add_socket() already reclassifies correctly on the initial connect
path, but the reconnect path was missed.
Add the missing nbd_reclassify_socket() call after nbd_get_socket() in
nbd_reconnect_socket().
Reported-by: syzbot+576095eed5658cbd9b63@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=576095eed5658cbd9b63
Fixes: d532cddb6c60 ("nbd: Reclassify sockets to avoid lockdep circular dependency")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
drivers/block/nbd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 8c6028e71e83..70a04d541ea4 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1386,6 +1386,8 @@ static int nbd_reconnect_socket(struct nbd_device *nbd, unsigned long arg)
if (!sock)
return err;
+ nbd_reclassify_socket(sock);
+
args = kzalloc_obj(*args);
if (!args) {
sockfd_put(sock);
--
2.43.0
^ permalink raw reply related
* [RFC PATCH 4/6] blk-mq: add a shared zone to tag fairness
From: Keith Busch @ 2026-07-06 17:34 UTC (permalink / raw)
To: linux-block; +Cc: linux-scsi, axboe, hch, bvanassche, sumit.saxena, Keith Busch
In-Reply-To: <20260706173438.3537347-1-kbusch@meta.com>
From: Keith Busch <kbusch@kernel.org>
Private slices of the sbitmap can strand an under utilized queue's tags.
Allow a driver to request some percentage of tags to be reserved into a
common shared pool that anyone may allocate from. The default 0 means
the entire tag set is divided up among its users; 100 means no one has
an exclusive window and can therefore allocate from the entire tag
space.
Shared tags being outside the private zone makes it so we can't do a
sbitmap range weight to find its active commands, so the debugfs count
may no longer accurate. Count the hctx's actual tags via
__blk_mq_hctx_tag_busy_iter() to provide accuracy over performance for
debugfs.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
block/blk-mq-tag.c | 94 +++++++++++++++++++++++++++++++-----------
block/blk-mq.c | 3 ++
include/linux/blk-mq.h | 5 +++
3 files changed, 79 insertions(+), 23 deletions(-)
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 58cf480df9c69..aa7dbceb60dcd 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -144,54 +144,79 @@ static inline bool blk_mq_tag_is_windowed(struct blk_mq_alloc_data *data)
(data->hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED);
}
+struct blk_mq_tag_win {
+ unsigned int priv_min;
+ unsigned int priv_max;
+ unsigned int shared_min;
+ unsigned int shared_max;
+};
+
static bool blk_mq_tag_active_window(struct blk_mq_hw_ctx *hctx,
- unsigned int depth, unsigned int *min,
- unsigned int *max)
+ unsigned int depth,
+ struct blk_mq_tag_win *w)
{
unsigned int users = READ_ONCE(hctx->tags->active_queues);
int slot = READ_ONCE(*blk_mq_tag_win_slot(hctx));
- unsigned int p, rem, pos;
+ unsigned int fair, priv, pct, pos;
+
+ if (users <= 1 || depth <= 1)
+ return false;
- if (users <= 1 || slot < 0 || depth <= 1)
+ fair = depth / users;
+ if (!fair)
return false;
- p = depth / users;
- if (!p)
+ pct = 100 - min(hctx->queue->tag_set->shared_pct, 100u);
+ priv = fair * pct / 100;
+ if (!priv)
return false;
/*
* Slots are allocated from a bitmap and freed out of order, so a slot
* index can exceed the active-user count once lower slots are freed.
* Convert it to a position based on how many active slots precede it.
+ * If a slot could not be assigned, the window is confined to the
+ * shared range.
*/
- pos = bitmap_weight(hctx->tags->active_slots, slot);
- rem = depth - p * users;
- *min = pos * p;
- *max = pos * p + p + rem;
- if (*max > depth)
- *max = depth;
+ pos = slot >= 0 ? bitmap_weight(hctx->tags->active_slots, slot) : users;
+ if (pos < users) {
+ w->priv_min = pos * priv;
+ w->priv_max = w->priv_min + priv;
+ } else {
+ w->priv_min = w->priv_max = 0;
+ }
+ w->shared_min = min(users * priv, depth);
+ w->shared_max = depth;
+
+ if (w->priv_min == w->priv_max && w->shared_min == w->shared_max)
+ return false;
return true;
}
-int blk_mq_get_tag_window(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue *bt)
+static int __blk_mq_get_tag_window(struct sbitmap_queue *bt,
+ struct blk_mq_tag_win *w)
{
- unsigned int min, max;
+ if (w->priv_max > w->priv_min) {
+ int tag = sbitmap_queue_get_range(bt, w->priv_min, w->priv_max);
- if (blk_mq_tag_active_window(hctx, bt->sb.depth, &min, &max))
- return sbitmap_queue_get_range(bt, min, max);
- return __sbitmap_queue_get(bt);
+ if (tag >= 0)
+ return tag;
+ }
+
+ if (w->shared_max > w->shared_min)
+ return sbitmap_queue_get_range(bt, w->shared_min, w->shared_max);
+ return BLK_MQ_NO_TAG;
}
-unsigned int blk_mq_hctx_active(struct blk_mq_hw_ctx *hctx)
+int blk_mq_get_tag_window(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue *bt)
{
- struct sbitmap *sb = &hctx->tags->bitmap_tags.sb;
- unsigned int min, max;
+ struct blk_mq_tag_win w;
- if (!blk_mq_tag_active_window(hctx, sb->depth, &min, &max))
- return sbitmap_weight(sb);
- return sbitmap_weight_range(sb, min, max - min);
+ if (blk_mq_tag_active_window(hctx, bt->sb.depth, &w))
+ return __blk_mq_get_tag_window(bt, &w);
+ return __sbitmap_queue_get(bt);
}
static int __blk_mq_get_tag(struct blk_mq_alloc_data *data,
@@ -604,6 +629,29 @@ static void __blk_mq_hctx_tag_busy_iter(struct blk_mq_hw_ctx *hctx,
bt_for_each(hctx, hctx->queue, &tags->bitmap_tags, fn, priv, false);
}
+static bool blk_mq_count_active(struct request *rq, void *priv)
+{
+ (*(unsigned int *)priv)++;
+ return true;
+}
+
+unsigned int blk_mq_hctx_active(struct blk_mq_hw_ctx *hctx)
+{
+ struct request_queue *q = hctx->queue;
+ unsigned int count = 0;
+ int srcu_idx;
+
+ if (!percpu_ref_tryget(&q->q_usage_counter))
+ return 0;
+
+ srcu_idx = srcu_read_lock(&q->tag_set->tags_srcu);
+ __blk_mq_hctx_tag_busy_iter(hctx, blk_mq_count_active, &count);
+ srcu_read_unlock(&q->tag_set->tags_srcu, srcu_idx);
+
+ blk_queue_exit(q);
+ return count;
+}
+
/**
* blk_mq_queue_tag_busy_iter - iterate over all requests with a driver tag
* @q: Request queue to examine.
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 56ab6ac5ec696..cc56baee74fe8 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4845,6 +4845,9 @@ int blk_mq_alloc_tag_set(struct blk_mq_tag_set *set)
set->queue_depth = BLK_MQ_MAX_DEPTH;
}
+ if (set->shared_pct > 100)
+ set->shared_pct = 100;
+
if (!set->nr_maps)
set->nr_maps = 1;
else if (set->nr_maps > HCTX_MAX_TYPES)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 22cc09d5ef320..59847ac7d319c 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -515,6 +515,10 @@ enum hctx_type {
* @numa_node: NUMA node the storage adapter has been connected to.
* @timeout: Request processing timeout in jiffies.
* @flags: Zero or more BLK_MQ_F_* flags.
+ * @shared_pct: Percent of tags dedicated to non-exclusive use. 0 means
+ * the entire tagset is divided among the users for exclusive
+ * per-user access, and 100 means the entire tag space is
+ * available to all queue contexts that share it.
* @driver_data: Pointer to data owned by the block driver that created this
* tag set.
* @tags: Tag sets. One tag set per hardware queue. Has @nr_hw_queues
@@ -544,6 +548,7 @@ struct blk_mq_tag_set {
int numa_node;
unsigned int timeout;
unsigned int flags;
+ unsigned int shared_pct;
void *driver_data;
struct blk_mq_tags **tags;
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 2/6] blk-mq: replace shared-tag fairness counter with allocation windows
From: Keith Busch @ 2026-07-06 17:34 UTC (permalink / raw)
To: linux-block; +Cc: linux-scsi, axboe, hch, bvanassche, sumit.saxena, Keith Busch
In-Reply-To: <20260706173438.3537347-1-kbusch@meta.com>
From: Keith Busch <kbusch@kernel.org>
The per-request atomic active counter that hctx_may_queue() checks on
every shared-tag allocation is costly under contention. Instead, confine
each active queue to a slice of the shared bitmap and allocate from
that, so one busy queue cannot starve the others. This removes per-IO
atomic on the hot path. Fairness becomes structural to the sbitmap
allocation range, so the counter and hctx_may_queue are removed.
Link: https://lore.kernel.org/linux-block/20240529213921.3166462-1-bvanassche@acm.org/
Link: https://lore.kernel.org/linux-block/20260609121806.2121755-1-sumit.saxena@broadcom.com/
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
block/blk-core.c | 2 +-
block/blk-mq-debugfs.c | 2 +-
block/blk-mq-tag.c | 113 +++++++++++++++++++++++++++++++++++++++--
block/blk-mq.c | 21 ++------
block/blk-mq.h | 102 +------------------------------------
include/linux/blk-mq.h | 15 ++++--
include/linux/blkdev.h | 6 ++-
7 files changed, 135 insertions(+), 126 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 365641266c9e8..7e719b90d8a66 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -464,7 +464,7 @@ struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id)
q->node = node_id;
- atomic_set(&q->nr_active_requests_shared_tags, 0);
+ q->tag_win_slot = -1;
timer_setup(&q->timeout, blk_rq_timed_out_timer, 0);
INIT_WORK(&q->timeout_work, blk_timeout_work);
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 6754d8f9449c1..4b47a3322ff72 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -480,7 +480,7 @@ static int hctx_active_show(void *data, struct seq_file *m)
{
struct blk_mq_hw_ctx *hctx = data;
- seq_printf(m, "%d\n", __blk_mq_active_requests(hctx));
+ seq_printf(m, "%u\n", blk_mq_hctx_active(hctx));
return 0;
}
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 35deee5bbc739..0dd497225c74a 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -33,6 +33,18 @@ static void blk_mq_update_wake_batch(struct blk_mq_tags *tags,
users);
}
+/*
+ * Where this queue's fairness allocation-window slot is stored. HCTX_SHARED
+ * tag sets divide one global bitmap among request_queues (slot per queue);
+ * QUEUE_SHARED tag sets divide each per-hw-queue bitmap among the hctxs
+ * sharing it (slot per hctx).
+ */
+static int *blk_mq_tag_win_slot(struct blk_mq_hw_ctx *hctx)
+{
+ return blk_mq_is_shared_tags(hctx->flags) ?
+ &hctx->queue->tag_win_slot : &hctx->tag_win_slot;
+}
+
/*
* If a previously inactive queue goes active, bump the active user count.
* We need to do this before try to allocate driver tag, then even if fail
@@ -65,6 +77,15 @@ void __blk_mq_tag_busy(struct blk_mq_hw_ctx *hctx)
users = tags->active_queues + 1;
WRITE_ONCE(tags->active_queues, users);
blk_mq_update_wake_batch(tags, users);
+ if (tags->active_slots) {
+ unsigned int nbits = tags->bitmap_tags.sb.depth;
+ unsigned int slot = find_first_zero_bit(tags->active_slots, nbits);
+
+ if (slot < nbits) {
+ set_bit(slot, tags->active_slots);
+ WRITE_ONCE(*blk_mq_tag_win_slot(hctx), slot);
+ }
+ }
spin_unlock_irqrestore(&tags->lock, flags);
}
@@ -102,17 +123,82 @@ void __blk_mq_tag_idle(struct blk_mq_hw_ctx *hctx)
users = tags->active_queues - 1;
WRITE_ONCE(tags->active_queues, users);
blk_mq_update_wake_batch(tags, users);
+ if (tags->active_slots) {
+ int *slotp = blk_mq_tag_win_slot(hctx);
+ int slot = READ_ONCE(*slotp);
+
+ if (slot >= 0) {
+ clear_bit(slot, tags->active_slots);
+ WRITE_ONCE(*slotp, -1);
+ }
+ }
spin_unlock_irq(&tags->lock);
blk_mq_tag_wakeup_all(tags, false);
}
+static inline bool blk_mq_tag_is_windowed(struct blk_mq_alloc_data *data)
+{
+ return !data->q->elevator &&
+ !(data->flags & BLK_MQ_REQ_RESERVED) &&
+ (data->hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED);
+}
+
+static bool blk_mq_tag_active_window(struct blk_mq_hw_ctx *hctx,
+ unsigned int depth, unsigned int *min,
+ unsigned int *max)
+{
+ unsigned int users = READ_ONCE(hctx->tags->active_queues);
+ int slot = READ_ONCE(*blk_mq_tag_win_slot(hctx));
+ unsigned int p, rem, pos;
+
+ if (users <= 1 || slot < 0 || depth <= 1)
+ return false;
+
+ p = depth / users;
+ if (!p)
+ return false;
+
+ /*
+ * Slots are allocated from a bitmap and freed out of order, so a slot
+ * index can exceed the active-user count once lower slots are freed.
+ * Convert it to a position based on how many active slots precede it.
+ */
+ pos = bitmap_weight(hctx->tags->active_slots, slot);
+ rem = depth - p * users;
+ *min = pos * p;
+ *max = pos * p + p + rem;
+ if (*max > depth)
+ *max = depth;
+
+ return true;
+}
+
+int blk_mq_get_tag_window(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue *bt)
+{
+ unsigned int min, max;
+
+ if (blk_mq_tag_active_window(hctx, bt->sb.depth, &min, &max))
+ return sbitmap_queue_get_range(bt, min, max);
+ return __sbitmap_queue_get(bt);
+
+}
+
+unsigned int blk_mq_hctx_active(struct blk_mq_hw_ctx *hctx)
+{
+ struct sbitmap *sb = &hctx->tags->bitmap_tags.sb;
+ unsigned int min, max;
+
+ if (!blk_mq_tag_active_window(hctx, sb->depth, &min, &max))
+ return sbitmap_weight(sb);
+ return sbitmap_weight_range(sb, min, max - min);
+}
+
static int __blk_mq_get_tag(struct blk_mq_alloc_data *data,
struct sbitmap_queue *bt)
{
- if (!data->q->elevator && !(data->flags & BLK_MQ_REQ_RESERVED) &&
- !hctx_may_queue(data->hctx, bt))
- return BLK_MQ_NO_TAG;
+ if (blk_mq_tag_is_windowed(data))
+ return blk_mq_get_tag_window(data->hctx, bt);
if (data->shallow_depth)
return sbitmap_queue_get_shallow(bt, data->shallow_depth);
@@ -142,6 +228,7 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
struct sbq_wait_state *ws;
DEFINE_SBQ_WAIT(wait);
unsigned int tag_offset;
+ bool slept = false;
int tag;
if (data->flags & BLK_MQ_REQ_RESERVED) {
@@ -193,8 +280,17 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
if (tag != BLK_MQ_NO_TAG)
break;
+ /*
+ * We were previously woken but still cannot allocate within our
+ * window: the freed bit belonged to another queue's window.
+ * Relay the wakeup so a waiter that can use it gets to run.
+ */
+ if (slept && blk_mq_tag_is_windowed(data))
+ sbitmap_queue_wake_up_relay(bt, ws);
+
bt_prev = bt;
io_schedule();
+ slept = true;
sbitmap_finish_wait(bt, ws, &wait);
@@ -574,8 +670,14 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
spin_lock_init(&tags->lock);
INIT_LIST_HEAD(&tags->page_list);
+ if (depth) {
+ tags->active_slots = bitmap_zalloc(depth, GFP_KERNEL);
+ if (!tags->active_slots)
+ goto out_free_tags;
+ }
+
if (bt_alloc(&tags->bitmap_tags, depth, round_robin, node))
- goto out_free_tags;
+ goto out_free_slots;
if (bt_alloc(&tags->breserved_tags, reserved_tags, round_robin, node))
goto out_free_bitmap_tags;
@@ -583,6 +685,8 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
out_free_bitmap_tags:
sbitmap_queue_free(&tags->bitmap_tags);
+out_free_slots:
+ bitmap_free(tags->active_slots);
out_free_tags:
kfree(tags);
return NULL;
@@ -611,6 +715,7 @@ void blk_mq_free_tags(struct blk_mq_tag_set *set, struct blk_mq_tags *tags)
{
sbitmap_queue_free(&tags->bitmap_tags);
sbitmap_queue_free(&tags->breserved_tags);
+ bitmap_free(tags->active_slots);
/* if tags pages is not allocated yet, free tags directly */
if (list_empty(&tags->page_list)) {
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c850330a32bc..56ab6ac5ec696 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -489,8 +489,6 @@ __blk_mq_alloc_requests_batch(struct blk_mq_alloc_data *data)
}
} while (data->nr_tags > nr);
- if (!(data->rq_flags & RQF_SCHED_TAGS))
- blk_mq_add_active_requests(data->hctx, nr);
/* caller already holds a reference, add for remainder */
percpu_ref_get_many(&data->q->q_usage_counter, nr - 1);
data->nr_tags -= nr;
@@ -587,8 +585,6 @@ static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
goto retry;
}
- if (!(data->rq_flags & RQF_SCHED_TAGS))
- blk_mq_inc_active_requests(data->hctx);
rq = blk_mq_rq_ctx_init(data, blk_mq_tags_from_data(data), tag);
blk_mq_rq_time_init(rq, alloc_time_ns);
return rq;
@@ -763,8 +759,6 @@ struct request *blk_mq_alloc_request_hctx(struct request_queue *q,
tag = blk_mq_get_tag(&data);
if (tag == BLK_MQ_NO_TAG)
goto out_queue_exit;
- if (!(data.rq_flags & RQF_SCHED_TAGS))
- blk_mq_inc_active_requests(data.hctx);
rq = blk_mq_rq_ctx_init(&data, blk_mq_tags_from_data(&data), tag);
blk_mq_rq_time_init(rq, alloc_time_ns);
rq->__data_len = 0;
@@ -807,10 +801,8 @@ static void __blk_mq_free_request(struct request *rq)
blk_pm_mark_last_busy(rq);
rq->mq_hctx = NULL;
- if (rq->tag != BLK_MQ_NO_TAG) {
- blk_mq_dec_active_requests(hctx);
+ if (rq->tag != BLK_MQ_NO_TAG)
blk_mq_put_tag(hctx->tags, ctx, rq->tag);
- }
if (sched_tag != BLK_MQ_NO_TAG)
blk_mq_put_tag(hctx->sched_tags, ctx, sched_tag);
blk_mq_sched_restart(hctx);
@@ -1157,8 +1149,6 @@ static inline void blk_mq_flush_tag_batch(struct blk_mq_hw_ctx *hctx,
{
struct request_queue *q = hctx->queue;
- blk_mq_sub_active_requests(hctx, nr_tags);
-
blk_mq_put_tags(hctx->tags, tag_array, nr_tags);
percpu_ref_put_many(&q->q_usage_counter, nr_tags);
}
@@ -1844,17 +1834,16 @@ bool __blk_mq_alloc_driver_tag(struct request *rq)
if (blk_mq_tag_is_reserved(rq->mq_hctx->sched_tags, rq->internal_tag)) {
bt = &rq->mq_hctx->tags->breserved_tags;
tag_offset = 0;
+ tag = __sbitmap_queue_get(bt);
} else {
- if (!hctx_may_queue(rq->mq_hctx, bt))
- return false;
+ /* Fairness on the shared driver tags via the allocation window. */
+ tag = blk_mq_get_tag_window(rq->mq_hctx, bt);
}
- tag = __sbitmap_queue_get(bt);
if (tag == BLK_MQ_NO_TAG)
return false;
rq->tag = tag + tag_offset;
- blk_mq_inc_active_requests(rq->mq_hctx);
return true;
}
@@ -4028,7 +4017,7 @@ blk_mq_alloc_hctx(struct request_queue *q, struct blk_mq_tag_set *set,
if (!zalloc_cpumask_var_node(&hctx->cpumask, gfp, node))
goto free_hctx;
- atomic_set(&hctx->nr_active, 0);
+ hctx->tag_win_slot = -1;
if (node == NUMA_NO_NODE)
node = set->numa_node;
hctx->numa_node = node;
diff --git a/block/blk-mq.h b/block/blk-mq.h
index aa15d31aaae9b..920bef5e7ce6e 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -205,6 +205,8 @@ static inline struct sbq_wait_state *bt_wait_ptr(struct sbitmap_queue *bt,
void __blk_mq_tag_busy(struct blk_mq_hw_ctx *);
void __blk_mq_tag_idle(struct blk_mq_hw_ctx *);
+unsigned int blk_mq_hctx_active(struct blk_mq_hw_ctx *hctx);
+int blk_mq_get_tag_window(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue *bt);
static inline void blk_mq_tag_busy(struct blk_mq_hw_ctx *hctx)
{
@@ -291,70 +293,9 @@ static inline int blk_mq_get_rq_budget_token(struct request *rq)
return -1;
}
-static inline void __blk_mq_add_active_requests(struct blk_mq_hw_ctx *hctx,
- int val)
-{
- if (blk_mq_is_shared_tags(hctx->flags))
- atomic_add(val, &hctx->queue->nr_active_requests_shared_tags);
- else
- atomic_add(val, &hctx->nr_active);
-}
-
-static inline void __blk_mq_inc_active_requests(struct blk_mq_hw_ctx *hctx)
-{
- __blk_mq_add_active_requests(hctx, 1);
-}
-
-static inline void __blk_mq_sub_active_requests(struct blk_mq_hw_ctx *hctx,
- int val)
-{
- if (blk_mq_is_shared_tags(hctx->flags))
- atomic_sub(val, &hctx->queue->nr_active_requests_shared_tags);
- else
- atomic_sub(val, &hctx->nr_active);
-}
-
-static inline void __blk_mq_dec_active_requests(struct blk_mq_hw_ctx *hctx)
-{
- __blk_mq_sub_active_requests(hctx, 1);
-}
-
-static inline void blk_mq_add_active_requests(struct blk_mq_hw_ctx *hctx,
- int val)
-{
- if (hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)
- __blk_mq_add_active_requests(hctx, val);
-}
-
-static inline void blk_mq_inc_active_requests(struct blk_mq_hw_ctx *hctx)
-{
- if (hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)
- __blk_mq_inc_active_requests(hctx);
-}
-
-static inline void blk_mq_sub_active_requests(struct blk_mq_hw_ctx *hctx,
- int val)
-{
- if (hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)
- __blk_mq_sub_active_requests(hctx, val);
-}
-
-static inline void blk_mq_dec_active_requests(struct blk_mq_hw_ctx *hctx)
-{
- if (hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED)
- __blk_mq_dec_active_requests(hctx);
-}
-
-static inline int __blk_mq_active_requests(struct blk_mq_hw_ctx *hctx)
-{
- if (blk_mq_is_shared_tags(hctx->flags))
- return atomic_read(&hctx->queue->nr_active_requests_shared_tags);
- return atomic_read(&hctx->nr_active);
-}
static inline void __blk_mq_put_driver_tag(struct blk_mq_hw_ctx *hctx,
struct request *rq)
{
- blk_mq_dec_active_requests(hctx);
blk_mq_put_tag(hctx->tags, rq->mq_ctx, rq->tag);
rq->tag = BLK_MQ_NO_TAG;
}
@@ -396,45 +337,6 @@ static inline void blk_mq_free_requests(struct list_head *list)
}
}
-/*
- * For shared tag users, we track the number of currently active users
- * and attempt to provide a fair share of the tag depth for each of them.
- */
-static inline bool hctx_may_queue(struct blk_mq_hw_ctx *hctx,
- struct sbitmap_queue *bt)
-{
- unsigned int depth, users;
-
- if (!hctx || !(hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED))
- return true;
-
- /*
- * Don't try dividing an ant
- */
- if (bt->sb.depth == 1)
- return true;
-
- if (blk_mq_is_shared_tags(hctx->flags)) {
- struct request_queue *q = hctx->queue;
-
- if (!test_bit(QUEUE_FLAG_HCTX_ACTIVE, &q->queue_flags))
- return true;
- } else {
- if (!test_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state))
- return true;
- }
-
- users = READ_ONCE(hctx->tags->active_queues);
- if (!users)
- return true;
-
- /*
- * Allow at least some tags
- */
- depth = max((bt->sb.depth + users - 1) / users, 4U);
- return __blk_mq_active_requests(hctx) < depth;
-}
-
/* run the code block in @dispatch_ops with rcu/srcu read lock held */
#define __blk_mq_run_dispatch_ops(q, check_sleep, dispatch_ops) \
do { \
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index af878597afb8c..22cc09d5ef320 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -433,10 +433,12 @@ struct blk_mq_hw_ctx {
unsigned int queue_num;
/**
- * @nr_active: Number of active requests. Only used when a tag set is
- * shared across request queues.
+ * @tag_win_slot: Assigned fairness allocation-window slot within this
+ * hctx's tags, or -1 if none. Used for non-HCTX_SHARED shared tag sets
+ * (the per-hctx analogue of request_queue.tag_win_slot). Protected by
+ * the tags->lock.
*/
- atomic_t nr_active;
+ int tag_win_slot;
/** @cpuhp_online: List to store request if CPU is going to die */
struct hlist_node cpuhp_online;
@@ -776,6 +778,13 @@ struct blk_mq_tags {
unsigned int nr_reserved_tags;
unsigned int active_queues;
+ /*
+ * Bitmap of assigned per-queue fairness window slots, @nr_tags -
+ * @nr_reserved_tags bits wide (windowing is moot once active users
+ * exceed that). Protected by @lock.
+ */
+ unsigned long *active_slots;
+
struct sbitmap_queue bitmap_tags;
struct sbitmap_queue breserved_tags;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9213a5716f95a..86b16bd8b9c17 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -573,7 +573,11 @@ struct request_queue {
struct timer_list timeout;
struct work_struct timeout_work;
- atomic_t nr_active_requests_shared_tags;
+ /*
+ * Assigned allocation-window slot for shared (HCTX_SHARED) tag-set
+ * fairness, or -1 if none. Protected by the shared tags->lock.
+ */
+ int tag_win_slot;
struct blk_mq_tags *sched_shared_tags;
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 5/6] blk-mq: cache shared-tag fairness windows
From: Keith Busch @ 2026-07-06 17:34 UTC (permalink / raw)
To: linux-block; +Cc: linux-scsi, axboe, hch, bvanassche, sumit.saxena, Keith Busch
In-Reply-To: <20260706173438.3537347-1-kbusch@meta.com>
From: Keith Busch <kbusch@kernel.org>
The windowed allocation path recomputed each queue's window on every tag
allocation, which includes costly division and bitmap weight operations.
Compute the window only when the active set changes and cache it per
queue/hctx, packed into a single u64 so the allocation fast path reads
it with one READ_ONCE and no arithmetic. A lock-free rebalance
recomputes the windows at each busy/idle transition for all contexts
using that tagset, assigning positions in tag_list order. This also
drops the need for the per-queue/hctx slot bitmap.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
block/blk-core.c | 2 +-
block/blk-mq-tag.c | 180 ++++++++++++++++++-----------------------
block/blk-mq.c | 2 +-
include/linux/blk-mq.h | 16 +---
include/linux/blkdev.h | 19 ++++-
5 files changed, 102 insertions(+), 117 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 7e719b90d8a66..85f7ad22f6461 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -464,7 +464,7 @@ struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id)
q->node = node_id;
- q->tag_win_slot = -1;
+ q->tag_win = (struct blk_mq_tag_win){ .shared_max = U16_MAX };
timer_setup(&q->timeout, blk_rq_timed_out_timer, 0);
INIT_WORK(&q->timeout_work, blk_timeout_work);
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index aa7dbceb60dcd..d92496378a834 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -34,15 +34,77 @@ static void blk_mq_update_wake_batch(struct blk_mq_tags *tags,
}
/*
- * Where this queue's fairness allocation-window slot is stored. HCTX_SHARED
- * tag sets divide one global bitmap among request_queues (slot per queue);
- * QUEUE_SHARED tag sets divide each per-hw-queue bitmap among the hctxs
- * sharing it (slot per hctx).
+ * Where this queue's cached fairness window lives. HCTX_SHARED tag sets divide
+ * one global bitmap among request_queues (window per queue); QUEUE_SHARED tag
+ * sets divide each per-hw-queue bitmap among the hctxs sharing it (window per
+ * hctx). The window packs into a u64 only because every bound fits in a u16.
*/
-static int *blk_mq_tag_win_slot(struct blk_mq_hw_ctx *hctx)
+static_assert(BLK_MQ_MAX_DEPTH <= U16_MAX);
+
+static struct blk_mq_tag_win *blk_mq_tag_win_ptr(struct blk_mq_hw_ctx *hctx)
{
return blk_mq_is_shared_tags(hctx->flags) ?
- &hctx->queue->tag_win_slot : &hctx->tag_win_slot;
+ &hctx->queue->tag_win : &hctx->tag_win;
+}
+
+static void __blk_mq_rebalance(struct blk_mq_tag_set *set,
+ struct blk_mq_tags *tags, bool shared,
+ unsigned int idx)
+{
+ unsigned int depth = tags->bitmap_tags.sb.depth;
+ unsigned int users, priv, smin, pos;
+ struct request_queue *q;
+
+ users = READ_ONCE(tags->active_queues);
+ priv = smin = pos = 0;
+ if (users > 1 && depth > 1) {
+ unsigned int fair = depth / users;
+ unsigned int pct = 100 - min(set->shared_pct, 100u);
+
+ priv = fair * pct / 100;
+ smin = min(users * priv, depth);
+ }
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(q, &set->tag_list, tag_set_list) {
+ struct blk_mq_tag_win *w, new_w;
+ bool active;
+
+ if (shared) {
+ w = &q->tag_win;
+ active = test_bit(QUEUE_FLAG_HCTX_ACTIVE, &q->queue_flags);
+ } else {
+ struct blk_mq_hw_ctx *hctx = queue_hctx(q, idx);
+
+ if (!hctx)
+ continue;
+ w = &hctx->tag_win;
+ active = test_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state);
+ }
+
+ if (!priv || !active) {
+ new_w = (struct blk_mq_tag_win) {
+ .shared_max = U16_MAX
+ };
+ } else {
+ new_w = (struct blk_mq_tag_win) {
+ .priv_min = min(pos * priv, depth),
+ .priv_max = min(pos * priv + priv, depth),
+ .shared_min = smin,
+ .shared_max = depth,
+ };
+ pos++;
+ }
+ WRITE_ONCE(w->v, new_w.v);
+ }
+ rcu_read_unlock();
+}
+
+/* Rebalance the pool @hctx draws from after its active set changed. */
+static void blk_mq_rebalance(struct blk_mq_hw_ctx *hctx)
+{
+ __blk_mq_rebalance(hctx->queue->tag_set, hctx->tags,
+ blk_mq_is_shared_tags(hctx->flags), hctx->queue_num);
}
/*
@@ -77,16 +139,9 @@ void __blk_mq_tag_busy(struct blk_mq_hw_ctx *hctx)
users = tags->active_queues + 1;
WRITE_ONCE(tags->active_queues, users);
blk_mq_update_wake_batch(tags, users);
- if (tags->active_slots) {
- unsigned int nbits = tags->bitmap_tags.sb.depth;
- unsigned int slot = find_first_zero_bit(tags->active_slots, nbits);
-
- if (slot < nbits) {
- set_bit(slot, tags->active_slots);
- WRITE_ONCE(*blk_mq_tag_win_slot(hctx), slot);
- }
- }
spin_unlock_irqrestore(&tags->lock, flags);
+
+ blk_mq_rebalance(hctx);
}
/*
@@ -123,17 +178,9 @@ void __blk_mq_tag_idle(struct blk_mq_hw_ctx *hctx)
users = tags->active_queues - 1;
WRITE_ONCE(tags->active_queues, users);
blk_mq_update_wake_batch(tags, users);
- if (tags->active_slots) {
- int *slotp = blk_mq_tag_win_slot(hctx);
- int slot = READ_ONCE(*slotp);
-
- if (slot >= 0) {
- clear_bit(slot, tags->active_slots);
- WRITE_ONCE(*slotp, -1);
- }
- }
spin_unlock_irq(&tags->lock);
+ blk_mq_rebalance(hctx);
blk_mq_tag_wakeup_all(tags, false);
}
@@ -144,81 +191,22 @@ static inline bool blk_mq_tag_is_windowed(struct blk_mq_alloc_data *data)
(data->hctx->flags & BLK_MQ_F_TAG_QUEUE_SHARED);
}
-struct blk_mq_tag_win {
- unsigned int priv_min;
- unsigned int priv_max;
- unsigned int shared_min;
- unsigned int shared_max;
-};
-
-static bool blk_mq_tag_active_window(struct blk_mq_hw_ctx *hctx,
- unsigned int depth,
- struct blk_mq_tag_win *w)
-{
- unsigned int users = READ_ONCE(hctx->tags->active_queues);
- int slot = READ_ONCE(*blk_mq_tag_win_slot(hctx));
- unsigned int fair, priv, pct, pos;
-
- if (users <= 1 || depth <= 1)
- return false;
-
- fair = depth / users;
- if (!fair)
- return false;
-
- pct = 100 - min(hctx->queue->tag_set->shared_pct, 100u);
- priv = fair * pct / 100;
- if (!priv)
- return false;
-
- /*
- * Slots are allocated from a bitmap and freed out of order, so a slot
- * index can exceed the active-user count once lower slots are freed.
- * Convert it to a position based on how many active slots precede it.
- * If a slot could not be assigned, the window is confined to the
- * shared range.
- */
- pos = slot >= 0 ? bitmap_weight(hctx->tags->active_slots, slot) : users;
- if (pos < users) {
- w->priv_min = pos * priv;
- w->priv_max = w->priv_min + priv;
- } else {
- w->priv_min = w->priv_max = 0;
- }
-
- w->shared_min = min(users * priv, depth);
- w->shared_max = depth;
-
- if (w->priv_min == w->priv_max && w->shared_min == w->shared_max)
- return false;
- return true;
-}
-
-static int __blk_mq_get_tag_window(struct sbitmap_queue *bt,
- struct blk_mq_tag_win *w)
+int blk_mq_get_tag_window(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue *bt)
{
- if (w->priv_max > w->priv_min) {
- int tag = sbitmap_queue_get_range(bt, w->priv_min, w->priv_max);
+ struct blk_mq_tag_win w = { .v = READ_ONCE(blk_mq_tag_win_ptr(hctx)->v) };
+ int tag;
+ if (w.priv_max > w.priv_min) {
+ tag = sbitmap_queue_get_range(bt, w.priv_min, w.priv_max);
if (tag >= 0)
return tag;
}
- if (w->shared_max > w->shared_min)
- return sbitmap_queue_get_range(bt, w->shared_min, w->shared_max);
-
+ if (w.shared_max > w.shared_min)
+ return sbitmap_queue_get_range(bt, w.shared_min, w.shared_max);
return BLK_MQ_NO_TAG;
}
-int blk_mq_get_tag_window(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue *bt)
-{
- struct blk_mq_tag_win w;
-
- if (blk_mq_tag_active_window(hctx, bt->sb.depth, &w))
- return __blk_mq_get_tag_window(bt, &w);
- return __sbitmap_queue_get(bt);
-}
-
static int __blk_mq_get_tag(struct blk_mq_alloc_data *data,
struct sbitmap_queue *bt)
{
@@ -726,14 +714,8 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
spin_lock_init(&tags->lock);
INIT_LIST_HEAD(&tags->page_list);
- if (depth) {
- tags->active_slots = bitmap_zalloc(depth, GFP_KERNEL);
- if (!tags->active_slots)
- goto out_free_tags;
- }
-
if (bt_alloc(&tags->bitmap_tags, depth, round_robin, node))
- goto out_free_slots;
+ goto out_free_tags;
if (bt_alloc(&tags->breserved_tags, reserved_tags, round_robin, node))
goto out_free_bitmap_tags;
@@ -741,8 +723,6 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
out_free_bitmap_tags:
sbitmap_queue_free(&tags->bitmap_tags);
-out_free_slots:
- bitmap_free(tags->active_slots);
out_free_tags:
kfree(tags);
return NULL;
@@ -771,7 +751,6 @@ void blk_mq_free_tags(struct blk_mq_tag_set *set, struct blk_mq_tags *tags)
{
sbitmap_queue_free(&tags->bitmap_tags);
sbitmap_queue_free(&tags->breserved_tags);
- bitmap_free(tags->active_slots);
/* if tags pages is not allocated yet, free tags directly */
if (list_empty(&tags->page_list)) {
@@ -786,6 +765,7 @@ void blk_mq_tag_resize_shared_tags(struct blk_mq_tag_set *set, unsigned int size
{
struct blk_mq_tags *tags = set->shared_tags;
+ __blk_mq_rebalance(set, tags, true, 0);
sbitmap_queue_resize(&tags->bitmap_tags, size - set->reserved_tags);
}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index cc56baee74fe8..ef8699735aa95 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4017,7 +4017,7 @@ blk_mq_alloc_hctx(struct request_queue *q, struct blk_mq_tag_set *set,
if (!zalloc_cpumask_var_node(&hctx->cpumask, gfp, node))
goto free_hctx;
- hctx->tag_win_slot = -1;
+ hctx->tag_win = (struct blk_mq_tag_win){ .shared_max = U16_MAX };
if (node == NUMA_NO_NODE)
node = set->numa_node;
hctx->numa_node = node;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 59847ac7d319c..0b1e1697e1736 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -433,12 +433,11 @@ struct blk_mq_hw_ctx {
unsigned int queue_num;
/**
- * @tag_win_slot: Assigned fairness allocation-window slot within this
- * hctx's tags, or -1 if none. Used for non-HCTX_SHARED shared tag sets
- * (the per-hctx analogue of request_queue.tag_win_slot). Protected by
- * the tags->lock.
+ * @tag_win: Cached fairness allocation window for non-HCTX_SHARED shared
+ * tag sets (each per-hw-queue bitmap divided among its hctxs). Written by
+ * the rebalance walk, read locklessly on the allocation path.
*/
- int tag_win_slot;
+ struct blk_mq_tag_win tag_win;
/** @cpuhp_online: List to store request if CPU is going to die */
struct hlist_node cpuhp_online;
@@ -783,13 +782,6 @@ struct blk_mq_tags {
unsigned int nr_reserved_tags;
unsigned int active_queues;
- /*
- * Bitmap of assigned per-queue fairness window slots, @nr_tags -
- * @nr_reserved_tags bits wide (windowing is moot once active users
- * exceed that). Protected by @lock.
- */
- unsigned long *active_slots;
-
struct sbitmap_queue bitmap_tags;
struct sbitmap_queue breserved_tags;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 86b16bd8b9c17..7b55ce6a3fd2b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -487,6 +487,18 @@ struct blk_independent_access_ranges {
struct blk_independent_access_range ia_range[];
};
+struct blk_mq_tag_win {
+ union {
+ struct {
+ u16 priv_min;
+ u16 priv_max;
+ u16 shared_min;
+ u16 shared_max;
+ };
+ u64 v;
+ };
+};
+
struct request_queue {
/*
* The queue owner gets to use this for whatever they like.
@@ -574,10 +586,11 @@ struct request_queue {
struct work_struct timeout_work;
/*
- * Assigned allocation-window slot for shared (HCTX_SHARED) tag-set
- * fairness, or -1 if none. Protected by the shared tags->lock.
+ * Cached fairness allocation window for HCTX_SHARED tag sets (the one
+ * global bitmap divided among request_queues). Written by the rebalance
+ * walk, read locklessly on the allocation path.
*/
- int tag_win_slot;
+ struct blk_mq_tag_win tag_win;
struct blk_mq_tags *sched_shared_tags;
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 1/6] lib/sbitmap: add ranged allocation, bounded wakeup relay, and ranged weight
From: Keith Busch @ 2026-07-06 17:34 UTC (permalink / raw)
To: linux-block; +Cc: linux-scsi, axboe, hch, bvanassche, sumit.saxena, Keith Busch
In-Reply-To: <20260706173438.3537347-1-kbusch@meta.com>
From: Keith Busch <kbusch@kernel.org>
Add some helpers that let a caller carve a shared bitmap into per-user
windows.
* sbitmap_queue_get_range() allocates a free bit only within requested range.
* sbitmap_queue_wake_up_relay() hands a wakeup to another waiter when the
woken one cannot use the freed bit (it fell outside that waiter's window).
* sbitmap_weight_range() counts set (and not cleared) bits in a sub-range.
No callers yet. The intention is for blk-mq users for shared tag
fairness without requiring per-io atomic accounting operations.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
include/linux/sbitmap.h | 53 +++++++++++
lib/sbitmap.c | 200 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 253 insertions(+)
diff --git a/include/linux/sbitmap.h b/include/linux/sbitmap.h
index cc7ad189caa5c..9fceda5d9de51 100644
--- a/include/linux/sbitmap.h
+++ b/include/linux/sbitmap.h
@@ -86,6 +86,7 @@ struct sbitmap {
#define SBQ_WAIT_QUEUES 8
#define SBQ_WAKE_BATCH 8
+#define SBQ_RELAY_BUDGET SBQ_WAKE_BATCH
/**
* struct sbq_wait_state - Wait queue in a &struct sbitmap_queue.
@@ -149,6 +150,14 @@ struct sbitmap_queue {
* @wakeup_cnt: Number of thread wake ups issued.
*/
atomic_t wakeup_cnt;
+
+ /**
+ * @relay_credits: Remaining range-relay wakeups allowed before the next
+ * genuine completion. Refilled by sbitmap_queue_wake_up() and consumed
+ * by sbitmap_queue_wake_up_relay(); bounds the relay chain so it cannot
+ * cycle when no waiter can use a freed bit.
+ */
+ atomic_t relay_credits;
};
/**
@@ -375,6 +384,18 @@ void sbitmap_show(struct sbitmap *sb, struct seq_file *m);
*/
unsigned int sbitmap_weight(const struct sbitmap *sb);
+/**
+ * sbitmap_weight_range() - Return how many set and not cleared bits in a
+ * sub-range of a &struct sbitmap.
+ * @sb: Bitmap to check.
+ * @off: First bit of the range.
+ * @len: Number of bits in the range.
+ *
+ * Return: How many bits in [@off, @off + @len) are set and not cleared.
+ */
+unsigned int sbitmap_weight_range(const struct sbitmap *sb, unsigned int off,
+ unsigned int len);
+
/**
* sbitmap_bitmap_show() - Write a hex dump of a &struct sbitmap to a &struct
* seq_file.
@@ -472,6 +493,22 @@ unsigned long __sbitmap_queue_get_batch(struct sbitmap_queue *sbq, int nr_tags,
int sbitmap_queue_get_shallow(struct sbitmap_queue *sbq,
unsigned int shallow_depth);
+/**
+ * sbitmap_queue_get_range() - Try to allocate a free bit from a restricted
+ * range of a &struct sbitmap_queue, with preemption already disabled.
+ * @sbq: Bitmap queue to allocate from.
+ * @min: First bit (inclusive) the caller is allowed to allocate.
+ * @max: Last bit (exclusive) the caller is allowed to allocate.
+ *
+ * Allocation is confined to the [@min, @max) window. This lets several users
+ * share one bitmap while each is bounded to its own (possibly overlapping)
+ * window, providing fairness without a separate per-user accounting counter.
+ *
+ * Return: Non-negative allocated bit number if successful, -1 otherwise.
+ */
+int sbitmap_queue_get_range(struct sbitmap_queue *sbq, unsigned int min,
+ unsigned int max);
+
/**
* sbitmap_queue_get() - Try to allocate a free bit from a &struct
* sbitmap_queue.
@@ -575,6 +612,22 @@ void sbitmap_queue_wake_all(struct sbitmap_queue *sbq);
*/
void sbitmap_queue_wake_up(struct sbitmap_queue *sbq, int nr);
+/**
+ * sbitmap_queue_wake_up_relay() - Relay a wakeup to the next waiter.
+ * @sbq: Bitmap queue to wake up.
+ * @cur: The relaying waiter's own wait queue, skipped so it does not wake
+ * itself (it is about to sleep again); may be NULL.
+ *
+ * Wake a single waiter on the next active wait queue without touching the
+ * wake_batch accounting. Intended for range-limited allocation: a waiter that
+ * was woken but could not use the freed bit (it fell outside the waiter's
+ * allowed range) calls this to pass the wakeup along to another waiter that
+ * may be able to use it. The relay is bounded by a credit budget refilled
+ * only by genuine completions, so it cannot cycle indefinitely.
+ */
+void sbitmap_queue_wake_up_relay(struct sbitmap_queue *sbq,
+ struct sbq_wait_state *cur);
+
/**
* sbitmap_queue_show() - Dump &struct sbitmap_queue information to a &struct
* seq_file.
diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index 4d188d05db153..3264085b4d533 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -338,6 +338,130 @@ static int sbitmap_get_shallow(struct sbitmap *sb, unsigned long shallow_depth)
return nr;
}
+/*
+ * Find and set a free bit in the [low, high) sub-range of a single word.
+ * Wraps back to @low once so a free bit below @hint is still found.
+ */
+static int __sbitmap_get_word_range(unsigned long *word, unsigned int low,
+ unsigned int high, unsigned int hint)
+{
+ bool wrap;
+ int nr;
+
+ if (hint < low || hint >= high)
+ hint = low;
+ /* don't wrap if already starting from the bottom of the range */
+ wrap = hint > low;
+
+ while (1) {
+ nr = find_next_zero_bit(word, high, hint);
+ if (unlikely(nr >= high)) {
+ if (wrap) {
+ hint = low;
+ wrap = false;
+ continue;
+ }
+ return -1;
+ }
+ if (!test_and_set_bit_lock(nr, word))
+ break;
+ hint = nr + 1;
+ if (hint >= high) {
+ if (!wrap)
+ return -1;
+ hint = low;
+ wrap = false;
+ }
+ }
+
+ return nr;
+}
+
+static int sbitmap_find_bit_in_word_range(struct sbitmap_word *map,
+ unsigned int low, unsigned int high,
+ unsigned int hint)
+{
+ int nr;
+
+ do {
+ nr = __sbitmap_get_word_range(&map->word, low, high, hint);
+ if (nr != -1)
+ break;
+ if (!sbitmap_deferred_clear(map, high, low, false))
+ break;
+ } while (1);
+
+ return nr;
+}
+
+/*
+ * Allocate a bit restricted to the [min, max) range of the bitmap. Used to
+ * carve the shared depth into (possibly overlapping) per-user windows without
+ * a separate accounting counter: a user simply cannot allocate outside its
+ * window.
+ */
+static int __sbitmap_get_range(struct sbitmap *sb, unsigned int min,
+ unsigned int max, unsigned int hint)
+{
+ unsigned int min_index, max_index, index;
+ unsigned int nwords, i;
+ int nr = -1;
+
+ if (max > sb->depth)
+ max = sb->depth;
+ if (min >= max)
+ return -1;
+
+ min_index = SB_NR_TO_INDEX(sb, min);
+ max_index = SB_NR_TO_INDEX(sb, max - 1);
+ nwords = max_index - min_index + 1;
+
+ if (hint < min || hint >= max)
+ hint = min;
+ index = SB_NR_TO_INDEX(sb, hint);
+
+ for (i = 0; i < nwords; i++) {
+ unsigned int word_base = index << sb->shift;
+ unsigned int word_depth = __map_depth(sb, index);
+ unsigned int low, high, h;
+
+ low = (index == min_index) ? SB_NR_TO_BIT(sb, min) : 0;
+ high = (index == max_index) ? SB_NR_TO_BIT(sb, max - 1) + 1 :
+ word_depth;
+ h = (hint >= word_base && hint < word_base + word_depth) ?
+ SB_NR_TO_BIT(sb, hint) : low;
+
+ nr = sbitmap_find_bit_in_word_range(&sb->map[index], low,
+ high, h);
+ if (nr != -1) {
+ nr += word_base;
+ break;
+ }
+
+ if (++index > max_index)
+ index = min_index;
+ }
+
+ return nr;
+}
+
+static int sbitmap_get_range(struct sbitmap *sb, unsigned int min,
+ unsigned int max)
+{
+ int nr;
+ unsigned int hint, depth;
+
+ if (WARN_ON_ONCE(unlikely(!sb->alloc_hint)))
+ return -1;
+
+ depth = READ_ONCE(sb->depth);
+ hint = update_alloc_hint_before_get(sb, depth);
+ nr = __sbitmap_get_range(sb, min, max, hint);
+ update_alloc_hint_after_get(sb, depth, hint, nr);
+
+ return nr;
+}
+
bool sbitmap_any_bit_set(const struct sbitmap *sb)
{
unsigned int i;
@@ -377,6 +501,38 @@ unsigned int sbitmap_weight(const struct sbitmap *sb)
}
EXPORT_SYMBOL_GPL(sbitmap_weight);
+unsigned int sbitmap_weight_range(const struct sbitmap *sb, unsigned int off,
+ unsigned int len)
+{
+ unsigned int weight = 0;
+ unsigned int end = off + len;
+ unsigned int index;
+
+ if (end > sb->depth)
+ end = sb->depth;
+
+ for (index = SB_NR_TO_INDEX(sb, off); index < sb->map_nr; index++) {
+ const struct sbitmap_word *word = &sb->map[index];
+ unsigned int base = index << sb->shift;
+ unsigned int wbits = __map_depth(sb, index);
+ unsigned int lo, hi;
+ unsigned long val;
+
+ if (base >= end)
+ break;
+ val = READ_ONCE(word->word) & ~READ_ONCE(word->cleared);
+ lo = (off > base) ? off - base : 0;
+ hi = (end - base < wbits) ? end - base : wbits;
+ if (lo)
+ val &= ~((1UL << lo) - 1);
+ if (hi < BITS_PER_LONG)
+ val &= (1UL << hi) - 1;
+ weight += hweight_long(val);
+ }
+ return weight;
+}
+EXPORT_SYMBOL_GPL(sbitmap_weight_range);
+
void sbitmap_show(struct sbitmap *sb, struct seq_file *m)
{
seq_printf(m, "depth=%u\n", sb->depth);
@@ -462,6 +618,7 @@ int sbitmap_queue_init_node(struct sbitmap_queue *sbq, unsigned int depth,
atomic_set(&sbq->ws_active, 0);
atomic_set(&sbq->completion_cnt, 0);
atomic_set(&sbq->wakeup_cnt, 0);
+ atomic_set(&sbq->relay_credits, 0);
sbq->ws = kzalloc_node(SBQ_WAIT_QUEUES * sizeof(*sbq->ws), flags, node);
if (!sbq->ws) {
@@ -573,6 +730,13 @@ int sbitmap_queue_get_shallow(struct sbitmap_queue *sbq,
}
EXPORT_SYMBOL_GPL(sbitmap_queue_get_shallow);
+int sbitmap_queue_get_range(struct sbitmap_queue *sbq, unsigned int min,
+ unsigned int max)
+{
+ return sbitmap_get_range(&sbq->sb, min, max);
+}
+EXPORT_SYMBOL_GPL(sbitmap_queue_get_range);
+
void sbitmap_queue_min_shallow_depth(struct sbitmap_queue *sbq,
unsigned int min_shallow_depth)
{
@@ -629,10 +793,46 @@ void sbitmap_queue_wake_up(struct sbitmap_queue *sbq, int nr)
} while (!atomic_try_cmpxchg(&sbq->wakeup_cnt,
&wakeups, wakeups + wake_batch));
+ atomic_set(&sbq->relay_credits, SBQ_RELAY_BUDGET);
__sbitmap_queue_wake_up(sbq, wake_batch);
}
EXPORT_SYMBOL_GPL(sbitmap_queue_wake_up);
+/*
+ * Wake a single waiter on the next active wait queue, bypassing the
+ * wake_batch accounting (no bit was freed). This "relays" a wakeup that the
+ * current waiter consumed but could not use - e.g. the freed bit fell outside
+ * its allowed allocation range - so a waiter that *can* use a free bit gets a
+ * chance to run. Relaying continues, one hop per failed waiter, until a usable
+ * waiter is found or the chain credits drain.
+ */
+void sbitmap_queue_wake_up_relay(struct sbitmap_queue *sbq,
+ struct sbq_wait_state *cur)
+{
+ int wake_index, i;
+
+ if (!atomic_read(&sbq->ws_active))
+ return;
+
+ if (atomic_dec_if_positive(&sbq->relay_credits) < 0)
+ return;
+
+ wake_index = atomic_read(&sbq->wake_index);
+ for (i = 0; i < SBQ_WAIT_QUEUES; i++) {
+ struct sbq_wait_state *ws = &sbq->ws[wake_index];
+
+ wake_index = sbq_index_inc(wake_index);
+ if (ws != cur && waitqueue_active(&ws->wait)) {
+ wake_up_nr(&ws->wait, 1);
+ break;
+ }
+ }
+
+ if (wake_index != atomic_read(&sbq->wake_index))
+ atomic_set(&sbq->wake_index, wake_index);
+}
+EXPORT_SYMBOL_GPL(sbitmap_queue_wake_up_relay);
+
static inline void sbitmap_update_cpu_hint(struct sbitmap *sb, int cpu, int tag)
{
if (likely(!sb->round_robin && tag < sb->depth))
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 0/6] sbitmap enforced fairness for blk-mq
From: Keith Busch @ 2026-07-06 17:34 UTC (permalink / raw)
To: linux-block; +Cc: linux-scsi, axboe, hch, bvanassche, sumit.saxena, Keith Busch
From: Keith Busch <kbusch@kernel.org>
There have been a few proposals to remove the blk-mq tag fairness
algorithm:
https://lore.kernel.org/linux-block/20240529213921.3166462-1-bvanassche@acm.org/
https://lore.kernel.org/linux-block/20260609121806.2121755-1-sumit.saxena@broadcom.com/
Both abandon blk-mq's attempt to enforce tag allocation limits on the
per-queue/per-hctx users that share tag space. This can harm resource
allocation for lesser devices sharing the space, potentially starving,
them from fair progress by a highly utilized device.
This series proposes an entirely different fairness mechanism that
doesn't require per-IO atomic accounting:
First, the sbitmap API is augmented with a ranged allocator. This
allows a client to carve the depth into exclusive ranges for specific
users.
Second, you can optionally declare a percentage of that pool to be
fair game for anyone to allocate from. This provides a way to
guarantee minimum tag space for each client while allowing a user to
over-allocate its fair budget on demand into the shared zone.
For testing, I used scsi_debug for the TAG_HCTX_SHARED case and a nvme
multi-namespace device for TAG_QUEUE_SHARED. Workloads emphasized greedy
vs. passive jobs. No performance regressions were observed.
There's a couple difficult things to deal with here:
After a completion releases a tag, there isn't an easy way to wake up
specific waiters for a range with that tag. This series handles that
by introducing a bounded wakeup relay: a waiter that was woken but
couldn't use the freed bit (it fell outside its allowed window)
forwards the wakeup to another waiter. The relay is bounded by a
credit budget refilled only by genuine completions, so it cannot cycle
indefinitely. I think this overhead is acceptable as we're already in
the slower path after exhausting the tag space.
The degenerate case when the number of users sharing the tag space
exceeds the number of tags is not specially handled. If that happens,
those users compete for the full tag space without fairness. The
existing implementation lets each user get any 4 tags in this
scenario, which is arbitrary. Duplicating that behavior would require
re-introducing atomic counting, which this series aims to remove.
The 5th patch is a performance optimization to avoid recalculating the
windows on every I/O (division + bitmap_weight). It computes the window
only when the active set changes and caches it per queue/hctx, packed
into a single u64 for a lockless fast-path read. It removes much of the
infrastructure introduced in patches 1-3, but the series is presented as
a 1:1 replacement first, then the optimization for easier review.
I am aware there are race windows with the purposefully lockless updates,
but those are temporary and harmless. Windows are recomputed on every
busy/idle transition and settle once the active set stabilizes. A torn
u64 read on 32-bit is possible but results in a temporarily mis-sized
window that self-corrects as tags get recycled.
The last patch enables scsi_debug to test the new fairness framework for
various shared vs. private splits, and exposes `shared_pct` for
host_tagset drivers where sharing may be harmful to performance. I
understand based on the previous proposals there is use for such
mechanisms for UFS.
Keith Busch (6):
lib/sbitmap: add ranged allocation, bounded wakeup relay, and ranged
weight
blk-mq: replace shared-tag fairness counter with allocation windows
blk-mq: factor out a per-hctx tag busy iterator
blk-mq: add a shared zone to tag fairness
blk-mq: cache shared-tag fairness windows
scsi: add shared-tag fairness to host_tagset drivers
block/blk-core.c | 2 +-
block/blk-mq-debugfs.c | 2 +-
block/blk-mq-tag.c | 179 ++++++++++++++++++++++++++++++----
block/blk-mq.c | 24 ++---
block/blk-mq.h | 102 +------------------
drivers/scsi/scsi_debug.c | 8 +-
drivers/scsi/scsi_lib.c | 4 +-
include/linux/blk-mq.h | 12 ++-
include/linux/blkdev.h | 19 +++-
include/linux/sbitmap.h | 53 ++++++++++
include/scsi/scsi_host.h | 8 ++
lib/sbitmap.c | 200 ++++++++++++++++++++++++++++++++++++++
12 files changed, 470 insertions(+), 143 deletions(-)
--
2.52.0
^ permalink raw reply
* [RFC PATCH 6/6] scsi: add shared-tag fairness to host_tagset drivers
From: Keith Busch @ 2026-07-06 17:34 UTC (permalink / raw)
To: linux-block; +Cc: linux-scsi, axboe, hch, bvanassche, sumit.saxena, Keith Busch
In-Reply-To: <20260706173438.3537347-1-kbusch@meta.com>
From: Keith Busch <kbusch@kernel.org>
Introduce a per-host shared_pct into the scsi block tag set so
host_tagset drivers can choose how much of the shared pool stays carved
into per-LUN exclusivity versus shared by everyone. This may be useful
for UFS where the sharing is harmful to performance.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/scsi/scsi_debug.c | 8 +++++++-
drivers/scsi/scsi_lib.c | 4 +++-
include/scsi/scsi_host.h | 8 ++++++++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 9d1c9c41d0f99..cb2bec76ba3de 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -905,6 +905,7 @@ static int sdebug_every_nth = DEF_EVERY_NTH;
static int sdebug_fake_rw = DEF_FAKE_RW;
static unsigned int sdebug_guard = DEF_GUARD;
static int sdebug_host_max_queue; /* per host */
+static int sdebug_shared_pct; /* host_tagset shared tag pool % */
static int sdebug_lowest_aligned = DEF_LOWEST_ALIGNED;
static int sdebug_max_luns = DEF_MAX_LUNS;
static int sdebug_max_queue = SDEBUG_CANQUEUE; /* per submit queue */
@@ -7343,6 +7344,7 @@ module_param_named(fake_rw, sdebug_fake_rw, int, S_IRUGO | S_IWUSR);
module_param_named(guard, sdebug_guard, uint, S_IRUGO);
module_param_named(host_lock, sdebug_host_lock, bool, S_IRUGO | S_IWUSR);
module_param_named(host_max_queue, sdebug_host_max_queue, int, S_IRUGO);
+module_param_named(shared_pct, sdebug_shared_pct, int, S_IRUGO);
module_param_string(inq_product, sdebug_inq_product_id,
sizeof(sdebug_inq_product_id), S_IRUGO | S_IWUSR);
module_param_string(inq_rev, sdebug_inq_product_rev,
@@ -7427,6 +7429,8 @@ MODULE_PARM_DESC(guard, "protection checksum: 0=crc, 1=ip (def=0)");
MODULE_PARM_DESC(host_lock, "host_lock is ignored (def=0)");
MODULE_PARM_DESC(host_max_queue,
"host max # of queued cmds (0 to max(def) [max_queue fixed equal for !0])");
+MODULE_PARM_DESC(shared_pct,
+ "host_tagset: %% of the shared tag pool shared by all LUNs (0=exclusive, 100=no fairness, def=0)");
MODULE_PARM_DESC(inq_product, "SCSI INQUIRY product string (def=\"scsi_debug\")");
MODULE_PARM_DESC(inq_rev, "SCSI INQUIRY revision string (def=\""
SDEBUG_VERSION "\")");
@@ -9569,8 +9573,10 @@ static int sdebug_driver_probe(struct device *dev)
* following should give the same answer for each host.
*/
hpnt->nr_hw_queues = submit_queues;
- if (sdebug_host_max_queue)
+ if (sdebug_host_max_queue) {
hpnt->host_tagset = 1;
+ hpnt->shared_pct = sdebug_shared_pct;
+ }
/* poll queues are possible for nr_hw_queues > 1 */
if (hpnt->nr_hw_queues == 1 || (poll_queues < 1)) {
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 22e2e3223440d..efcb49af899a2 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2152,8 +2152,10 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
if (shost->queuecommand_may_block)
tag_set->flags |= BLK_MQ_F_BLOCKING;
tag_set->driver_data = shost;
- if (shost->host_tagset)
+ if (shost->host_tagset) {
tag_set->flags |= BLK_MQ_F_TAG_HCTX_SHARED;
+ tag_set->shared_pct = shost->shared_pct;
+ }
return blk_mq_alloc_tag_set(tag_set);
}
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 7e2011830ba4b..68c99bc7ad865 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -694,6 +694,14 @@ struct Scsi_Host {
/* The queuecommand callback may block. See also BLK_MQ_F_BLOCKING. */
unsigned queuecommand_may_block:1;
+ /*
+ * For host_tagset hosts: percent of the shared tag pool made available
+ * to all LUNs rather than reserved as per-LUN exclusive floors. See
+ * blk_mq_tag_set.shared_pct. 0 (default) divides the pool exclusively;
+ * 100 lets every LUN allocate from the whole pool with no fairness.
+ */
+ unsigned int shared_pct;
+
/* Host responded with short (<36 bytes) INQUIRY result */
unsigned short_inquiry:1;
--
2.52.0
^ permalink raw reply related
* [RFC PATCH 3/6] blk-mq: factor out a per-hctx tag busy iterator
From: Keith Busch @ 2026-07-06 17:34 UTC (permalink / raw)
To: linux-block; +Cc: linux-scsi, axboe, hch, bvanassche, sumit.saxena, Keith Busch
In-Reply-To: <20260706173438.3537347-1-kbusch@meta.com>
From: Keith Busch <kbusch@kernel.org>
Make a helper for walking the busy tags of specific hardware queue based
on the per-hctx walk into __blk_mq_hctx_tag_busy_iter() so it can be
reused to iterate a single hardware queue's tags. No functional change.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
block/blk-mq-tag.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 0dd497225c74a..58cf480df9c69 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -582,6 +582,28 @@ void blk_mq_tagset_wait_completed_request(struct blk_mq_tag_set *tagset)
}
EXPORT_SYMBOL(blk_mq_tagset_wait_completed_request);
+static void __blk_mq_hctx_tag_busy_iter(struct blk_mq_hw_ctx *hctx,
+ busy_tag_iter_fn *fn, void *priv)
+{
+ struct blk_mq_tags *tags = hctx->tags;
+
+ /*
+ * If no software queues are currently mapped to this hardware queue,
+ * there's nothing to check
+ */
+ if (!blk_mq_hw_queue_mapped(hctx))
+ return;
+
+ if (tags->nr_reserved_tags)
+ bt_for_each(hctx, hctx->queue, &tags->breserved_tags, fn, priv, true);
+ bt_for_each(hctx, hctx->queue, &tags->bitmap_tags, fn, priv, false);
+}
+
/**
* blk_mq_queue_tag_busy_iter - iterate over all requests with a driver tag
* @q: Request queue to examine.
@@ -621,22 +643,8 @@ void blk_mq_queue_tag_busy_iter(struct request_queue *q, busy_tag_iter_fn *fn,
struct blk_mq_hw_ctx *hctx;
unsigned long i;
- queue_for_each_hw_ctx(q, hctx, i) {
- struct blk_mq_tags *tags = hctx->tags;
- struct sbitmap_queue *bresv = &tags->breserved_tags;
- struct sbitmap_queue *btags = &tags->bitmap_tags;
-
- /*
- * If no software queues are currently mapped to this
- * hardware queue, there's nothing to check
- */
- if (!blk_mq_hw_queue_mapped(hctx))
- continue;
-
- if (tags->nr_reserved_tags)
- bt_for_each(hctx, q, bresv, fn, priv, true);
- bt_for_each(hctx, q, btags, fn, priv, false);
- }
+ queue_for_each_hw_ctx(q, hctx, i)
+ __blk_mq_hctx_tag_busy_iter(hctx, fn, priv);
}
srcu_read_unlock(&q->tag_set->tags_srcu, srcu_idx);
blk_queue_exit(q);
--
2.52.0
^ permalink raw reply related
* Re: [RFC PATCH 0/6] sbitmap enforced fairness for blk-mq
From: Bart Van Assche @ 2026-07-06 17:56 UTC (permalink / raw)
To: Keith Busch, linux-block
Cc: linux-scsi, axboe, hch, sumit.saxena, Keith Busch
In-Reply-To: <20260706173438.3537347-1-kbusch@meta.com>
On 7/6/26 10:34 AM, Keith Busch wrote:
> There have been a few proposals to remove the blk-mq tag fairness
> algorithm:
>
> https://lore.kernel.org/linux-block/20240529213921.3166462-1-bvanassche@acm.org/
> https://lore.kernel.org/linux-block/20260609121806.2121755-1-sumit.saxena@broadcom.com/
There have been more proposals to remove the fairness code from other
kernel contributors. I proposed to remove the tag fairness code because
prior attempts to solve UFS performance issues were not good enough for
the upstream kernel.
> Both abandon blk-mq's attempt to enforce tag allocation limits on the
> per-queue/per-hctx users that share tag space. This can harm resource
> allocation for lesser devices sharing the space, potentially starving,
> them from fair progress by a highly utilized device.
Has this starving phenomenon ever been observed? I think that the
measurement data that I published shows that the fairness properties of
the current sbitmap implementation are good enough.
> This series proposes an entirely different fairness mechanism that
> doesn't require per-IO atomic accounting:
>
> First, the sbitmap API is augmented with a ranged allocator. This
> allows a client to carve the depth into exclusive ranges for specific
> users.
>
> Second, you can optionally declare a percentage of that pool to be
> fair game for anyone to allocate from. This provides a way to
> guarantee minimum tag space for each client while allowing a user to
> over-allocate its fair budget on demand into the shared zone.
What software layer is expected to set these percentages? My patch
series intentionally removes the fairness mechanism because that's
exactly what the legacy code path in the UFS driver needs. The UFS
WLUN and data logical units share a single tag set. Any activity on
the WLUN increases the number of active queues and hence reduces the
number of tags available for any data logical units.
Additionally, what is the performance impact of this patch series?
Removing the tag fairness code increases IOPS so this patch series
probably has a performance impact.
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH v19 22/40] dept: track PG_locked with dept
From: Matthew Wilcox @ 2026-07-06 18:05 UTC (permalink / raw)
To: Byungchul Park
Cc: linux-kernel, max.byungchul.park, kernel_team, torvalds,
damien.lemoal, linux-ide, adilger.kernel, linux-ext4, mingo,
peterz, will, tglx, rostedt, joel, sashal, daniel.vetter,
duyuyang, johannes.berg, tj, tytso, david, amir73il, gregkh,
kernel-team, linux-mm, akpm, mhocko, minchan, hannes,
vdavydov.dev, sj, jglisse, dennis, cl, penberg, rientjes, vbabka,
ngupta, linux-block, josef, linux-fsdevel, jack, jlayton,
dan.j.williams, hch, djwong, dri-devel, rodrigosiqueiramelo,
melissa.srw, hamohammed.sa, harry.yoo, chris.p.wilson,
gwan-gyeong.mun, boqun.feng, longman, yunseong.kim, ysk,
yeoreum.yun, netdev, matthew.brost, her0gyugyu, corbet,
catalin.marinas, bp, x86, hpa, luto, sumit.semwal, gustavo,
christian.koenig, andi.shyti, arnd, lorenzo.stoakes, Liam.Howlett,
rppt, surenb, mcgrof, petr.pavlu, da.gomez, samitolvanen, paulmck,
frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy, anna, kees,
bigeasy, clrkwllms, mark.rutland, ada.coupriediaz,
kristina.martsenko, wangkefeng.wang, broonie, kevin.brodsky, dwmw,
shakeel.butt, ast, ziy, yuzhao, baolin.wang, usamaarif642,
joel.granados, richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel,
2407018371, dakr, miguel.ojeda.sandonis, neilb, bagasdotme,
wsa+renesas, dave.hansen, geert, ojeda, alex.gaynor, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, rust-for-linux
In-Reply-To: <20260706061928.66713-23-byungchul@sk.com>
On Mon, Jul 06, 2026 at 03:19:10PM +0900, Byungchul Park wrote:
> Makes dept able to track PG_locked waits and events, which will be
> useful in practice. See the following link that shows dept worked with
> PG_locked and detected real issues in practice:
>
> https://lore.kernel.org/lkml/1674268856-31807-1-git-send-email-byungchul.park@lge.com/
> @@ -219,6 +220,7 @@ struct page {
> struct page *kmsan_shadow;
> struct page *kmsan_origin;
> #endif
> + struct dept_ext_wgen pg_locked_wgen;
> } _struct_page_alignment;
I may not understand this quite correctly, but I think that tracking
PG_locked dependencies in the struct page has both false positive and
false negative problems.
Imagine we have a file mapping M1 containing folio F1 at index 0 and F2
at index 1. It is correct locking order to lock F1 before locking F2
(for example when doing writeback). Later, M1 has its folios reclaimed
and returned to the free pool. Then each is added to mapping M2, this
time with folio F2 at index 8 and F1 at index 9. Now the correct order
to lock these folios in the order F2 followed by F1.
I don't see a part of this patch where we clear pg_locked_wgen when the
page is returned to the page allocator. Maybe I missed that.
I think we should be tracking PG_locked dependencies in the owner
of the folio. For files, that would be in the struct address_space.
For anon memory, I think that's in the anon_vma, but if somebody told
me it was in some other structure, I wouldn't argue with them.
This requires slightly more complexity than lockdep currently has.
We don't want to use a lockdep class for each folio, obviously. So we
need something to say "I already have folio F1 locked, is it OK to lock
folio F2?". Essentially figuring out how we can track all folios in a
given mapping the same way, and making sure that we don't deadlock on
folios in the same mapping.
If F1 and F2 are in different mappings, it's not a deadlock if F1 is in a
filesystem mapping and F2 is in its backing dev. It's also not a deadlock
if F1 and F2 are both filesystem folios and the inodes are both locked.
See vfs_lock_two_folios() in fs/remap_range.c.
I have much less knowledge about anonymous memory locking order.
Maybe it doesn't happen. Or about locking one anon and one file folio.
For slab memory, we don't sleep on PG_locked (it's used as a spinlock bit).
For other kinds of memory ... I don't know. Page migration is fun.
^ permalink raw reply
* Re: [RFC PATCH 0/6] sbitmap enforced fairness for blk-mq
From: Keith Busch @ 2026-07-06 18:26 UTC (permalink / raw)
To: Bart Van Assche
Cc: Keith Busch, linux-block, linux-scsi, axboe, hch, sumit.saxena
In-Reply-To: <6db8dcc3-6f79-4407-a5de-ec80915bc73a@acm.org>
On Mon, Jul 06, 2026 at 10:56:05AM -0700, Bart Van Assche wrote:
> On 7/6/26 10:34 AM, Keith Busch wrote:
> > There have been a few proposals to remove the blk-mq tag fairness
> > algorithm:
> >
> > https://lore.kernel.org/linux-block/20240529213921.3166462-1-bvanassche@acm.org/
> > https://lore.kernel.org/linux-block/20260609121806.2121755-1-sumit.saxena@broadcom.com/
>
> There have been more proposals to remove the fairness code from other
> kernel contributors. I proposed to remove the tag fairness code because
> prior attempts to solve UFS performance issues were not good enough for
> the upstream kernel.
>
> > Both abandon blk-mq's attempt to enforce tag allocation limits on the
> > per-queue/per-hctx users that share tag space. This can harm resource
> > allocation for lesser devices sharing the space, potentially starving,
> > them from fair progress by a highly utilized device.
>
> Has this starving phenomenon ever been observed?
If you're asking if a production workload sees the phenomenon, no
because they run the code that has enforced fairness.
> I think that the
> measurement data that I published shows that the fairness properties of
> the current sbitmap implementation are good enough.
I can trivially wire up a test where one queue allocates 100% of the
tags for long running commands, and all the other queues that want to
start a short running command can't proceed.
> > Second, you can optionally declare a percentage of that pool to be
> > fair game for anyone to allocate from. This provides a way to
> > guarantee minimum tag space for each client while allowing a user to
> > over-allocate its fair budget on demand into the shared zone.
>
> What software layer is expected to set these percentages?
It's a tagset parameter that the LLD can set to whatever it needs.
> My patch
> series intentionally removes the fairness mechanism because that's
> exactly what the legacy code path in the UFS driver needs. The UFS
> WLUN and data logical units share a single tag set. Any activity on
> the WLUN increases the number of active queues and hence reduces the
> number of tags available for any data logical units.
>
> Additionally, what is the performance impact of this patch series?
It removes the per-io atomics, so impact is it goes faster if that was
your bottle-neck.
> Removing the tag fairness code increases IOPS so this patch series
> probably has a performance impact.
You can declare the entire tagset is shared if you want. That should get
you what you were going for, but we still have an option to spatially
constrain others that want fairness with this series.
^ permalink raw reply
* Re: [PATCH 2/2] dm-raid1: don't fail the mirror for invalid I/O errors
From: Keith Busch @ 2026-07-06 20:52 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Benjamin Marzinski, Keith Busch, dm-devel, linux-block,
Dr. David Alan Gilbert, Vjaceslavs Klimovs
In-Reply-To: <0bd687cf-82ac-eacf-844b-c179a52dc72c@redhat.com>
On Wed, Jun 24, 2026 at 01:14:03PM +0200, Mikulas Patocka wrote:
> This approach is OK, I will stage the patches when 7.2-rc1 comes out and
> when I'll fork the dm git branches.
>
> I suggest one change - it is kind of hacky when multiple I/O completion
> callbacks write into io->orig_bio->bi_status concurrently - so it would be
> better to not do it and maintain and return separate bit mask for
> non-retryable errors.
>
> For example:
>
> static void complete_io(struct io *io)
> {
> unsigned long error_bits = io->error_bits;
> unsigned long nonretryable_error_bits = io->nonretryable_error_bits;
> io_notify_fn fn = io->callback;
> void *context = io->context;
>
> if (io->vma_invalidate_size)
> invalidate_kernel_vmap_range(io->vma_invalidate_address,
> io->vma_invalidate_size);
>
> mempool_free(io, &io->client->pool);
> fn(error_bits, nonretryable_error_bits, context);
> }
>
> static void dec_count(struct io *io, unsigned int region, blk_status_t error)
> {
> if (unlikely(error == BLK_STS_NOTSUPP) || unlikely(error == BLK_STS_INVAL))
> set_bit(region, &io->nonretryable_error_bits);
> else if (unlikely(error != BLK_STS_OK))
> set_bit(region, &io->error_bits);
>
> if (atomic_dec_and_test(&io->count))
> complete_io(io);
> }
>
> Please send the updated patch that uses this approach.
Sure thing, I can get started on that. Though I think it's largely
obviated if we get the block layer to handle things early rather than
submit malformed bio's, and this will accomplish that:
https://lore.kernel.org/dm-devel/20260624170905.3972095-1-kbusch@meta.com/
But I can certainly respin this series as it provides a more indepth
defence.
I also owe an update on the relaxed dm-crypt direct-io memory alignment
as well, as that series fell through the cracks on me for the previous
merge window.
> BTW. I think that blk_path_error should also test for BLK_STS_INVAL and
> return false, otherwise, dm-multipath would be suffering from this bug
> too. Ben, could you test it?
Good point.
^ permalink raw reply
* Re: [PATCH] blk-iolatency: flush enable work after policy deactivation
From: Cen Zhang @ 2026-07-07 2:28 UTC (permalink / raw)
To: Tejun Heo, Josef Bacik, Jens Axboe
Cc: cgroups, linux-block, linux-kernel, baijiaju1990
In-Reply-To: <20260621135916.2657247-1-zzzccc427@gmail.com>
Hi all,
Sorry for the noise, but I wanted to gently follow up on this patch in
case it got buried.
I would really appreciate any feedback when you have a chance. Please
let me know if I should make any changes or provide more testing
results.
Thanks a lot.
Best regards,
Cen Zhang
Cen Zhang <zzzccc427@gmail.com> 于2026年6月21日周日 21:59写道：
>
> A blk-iolatency rq-qos teardown can free struct blk_iolatency while a
> freshly queued enable_work callback still references it. The observed
> failure is:
>
> blkcg_iolatency_exit() flushes enable_work before deactivating the
> iolatency policy. However, blkcg_deactivate_policy() calls
> iolatency_pd_offline() for online policy data, and iolatency_pd_offline()
> clears min_lat_nsec through iolatency_set_min_lat_nsec(). If this clears
> the last nonzero latency target, enable_cnt reaches zero and schedules
> enable_work again after the flush has already returned.
>
> The buggy scenario involves two paths, with each column showing the order
> within that path:
>
> blkcg_iolatency_exit() path: system_wq worker path:
> 1. Flush old enable_work. 1. enable_work is idle.
> 2. Deactivate the policy. 2. no worker owns it.
> 3. Offline queues new enable_work. 3. work item becomes pending.
> 4. Free blkiolat. 4. worker later runs the item.
> 5. Owner storage is gone. 5. worker dereferences blkiolat.
>
> Flush enable_work again after blkcg_deactivate_policy() returns and before
> freeing blkiolat. Policy offline callbacks have completed at that point,
> so the second drain covers the late queueing path without changing the
> normal enable/disable accounting rules.
>
> Validation reproduced this kernel report:
> BUG: KASAN: slab-use-after-free in assign_work+0x2a/0x150
>
> Call Trace:
> <TASK>
> dump_stack_lvl+0x53/0x70
> print_report+0xd0/0x630
> ? __pfx__raw_spin_lock_irqsave+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __virt_addr_valid+0xea/0x1a0
> ? assign_work+0x2a/0x150
> kasan_report+0xce/0x100
> ? assign_work+0x2a/0x150
> assign_work+0x2a/0x150
> worker_thread+0x1b7/0x500
> ? __pfx_worker_thread+0x10/0x10
> kthread+0x192/0x1d0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork+0x2ac/0x3c0
> ? __pfx_ret_from_fork+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __switch_to+0x2d5/0x6e0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> Allocated by task 470:
> kasan_save_stack+0x33/0x60
> kasan_save_track+0x14/0x30
> __kasan_kmalloc+0x8f/0xa0
> iolatency_set_limit+0x301/0x450
> cgroup_file_write+0x178/0x2e0
> kernfs_fop_write_iter+0x1ef/0x290
> vfs_write+0x446/0x6f0
> ksys_write+0xc7/0x160
> do_syscall_64+0xf9/0x540
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Freed by task 611:
> kasan_save_stack+0x33/0x60
> kasan_save_track+0x14/0x30
> kasan_save_free_info+0x3b/0x60
> __kasan_slab_free+0x43/0x70
> kfree+0x131/0x390
> rq_qos_exit+0x5d/0x90
> __del_gendisk+0x394/0x490
> del_gendisk+0xa1/0xe0
> virtblk_remove+0x41/0xd0
> virtio_dev_remove+0x63/0xe0
> device_release_driver_internal+0x246/0x2e0
> unbind_store+0xa9/0xb0
> kernfs_fop_write_iter+0x1ef/0x290
> vfs_write+0x446/0x6f0
> ksys_write+0xc7/0x160
> do_syscall_64+0xf9/0x540
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Last potentially related work creation:
> kasan_save_stack+0x33/0x60
> kasan_record_aux_stack+0x8c/0xa0
> __queue_work+0x42a/0x800
> queue_work_on+0x5d/0x70
> iolatency_set_min_lat_nsec+0x196/0x230
> iolatency_pd_offline+0x1f/0x40
> blkcg_deactivate_policy+0x194/0x270
> blkcg_iolatency_exit+0x33/0x40
> rq_qos_exit+0x5d/0x90
> __del_gendisk+0x394/0x490
> del_gendisk+0xa1/0xe0
> virtblk_remove+0x41/0xd0
> virtio_dev_remove+0x63/0xe0
> device_release_driver_internal+0x246/0x2e0
> unbind_store+0xa9/0xb0
> kernfs_fop_write_iter+0x1ef/0x290
> vfs_write+0x446/0x6f0
> ksys_write+0xc7/0x160
> do_syscall_64+0xf9/0x540
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Second to last potentially related work creation:
> kasan_save_stack+0x33/0x60
> kasan_record_aux_stack+0x8c/0xa0
> __queue_work+0x42a/0x800
> queue_work_on+0x5d/0x70
> iolatency_set_min_lat_nsec+0x196/0x230
> iolatency_set_limit+0x3f1/0x450
> cgroup_file_write+0x178/0x2e0
> kernfs_fop_write_iter+0x1ef/0x290
> vfs_write+0x446/0x6f0
> ksys_write+0xc7/0x160
> do_syscall_64+0xf9/0x540
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Fixes: 8a177a36da6c ("blk-iolatency: Fix inflight count imbalances and IO hangs on offline")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> block/blk-iolatency.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
> index 1aaee6fb0f59..a0bdd8a5c94c 100644
> --- a/block/blk-iolatency.c
> +++ b/block/blk-iolatency.c
> @@ -639,6 +639,11 @@ static void blkcg_iolatency_exit(struct rq_qos *rqos)
> timer_shutdown_sync(&blkiolat->timer);
> flush_work(&blkiolat->enable_work);
> blkcg_deactivate_policy(rqos->disk, &blkcg_policy_iolatency);
> + /*
> + * blkcg_deactivate_policy() invokes iolatency_pd_offline(), which may
> + * queue enable_work again when it clears the last latency target.
> + */
> + flush_work(&blkiolat->enable_work);
> kfree(blkiolat);
> }
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v19 22/40] dept: track PG_locked with dept
From: Byungchul Park @ 2026-07-07 2:35 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-kernel, max.byungchul.park, kernel_team, torvalds,
damien.lemoal, linux-ide, adilger.kernel, linux-ext4, mingo,
peterz, will, tglx, rostedt, joel, sashal, daniel.vetter,
duyuyang, johannes.berg, tj, tytso, david, amir73il, gregkh,
kernel-team, linux-mm, akpm, mhocko, minchan, hannes,
vdavydov.dev, sj, jglisse, dennis, cl, penberg, rientjes, vbabka,
ngupta, linux-block, josef, linux-fsdevel, jack, jlayton,
dan.j.williams, hch, djwong, dri-devel, rodrigosiqueiramelo,
melissa.srw, hamohammed.sa, harry.yoo, chris.p.wilson,
gwan-gyeong.mun, boqun.feng, longman, yunseong.kim, ysk,
yeoreum.yun, netdev, matthew.brost, her0gyugyu, corbet,
catalin.marinas, bp, x86, hpa, luto, sumit.semwal, gustavo,
christian.koenig, andi.shyti, arnd, lorenzo.stoakes, Liam.Howlett,
rppt, surenb, mcgrof, petr.pavlu, da.gomez, samitolvanen, paulmck,
frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy, anna, kees,
bigeasy, clrkwllms, mark.rutland, ada.coupriediaz,
kristina.martsenko, wangkefeng.wang, broonie, kevin.brodsky, dwmw,
shakeel.butt, ast, ziy, yuzhao, baolin.wang, usamaarif642,
joel.granados, richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel,
2407018371, dakr, miguel.ojeda.sandonis, neilb, bagasdotme,
wsa+renesas, dave.hansen, geert, ojeda, alex.gaynor, gary,
bjorn3_gh, lossin, a.hindborg, aliceryhl, tmgross, rust-for-linux
In-Reply-To: <akvueAxPl8aoLvMR@casper.infradead.org>
On Mon, Jul 06, 2026 at 07:05:44PM +0100, Matthew Wilcox wrote:
> On Mon, Jul 06, 2026 at 03:19:10PM +0900, Byungchul Park wrote:
> > Makes dept able to track PG_locked waits and events, which will be
> > useful in practice. See the following link that shows dept worked with
> > PG_locked and detected real issues in practice:
> >
> > https://lore.kernel.org/lkml/1674268856-31807-1-git-send-email-byungchul.park@lge.com/
>
> > @@ -219,6 +220,7 @@ struct page {
> > struct page *kmsan_shadow;
> > struct page *kmsan_origin;
> > #endif
> > + struct dept_ext_wgen pg_locked_wgen;
> > } _struct_page_alignment;
>
> I may not understand this quite correctly, but I think that tracking
> PG_locked dependencies in the struct page has both false positive and
> false negative problems.
>
> Imagine we have a file mapping M1 containing folio F1 at index 0 and F2
> at index 1. It is correct locking order to lock F1 before locking F2
> (for example when doing writeback). Later, M1 has its folios reclaimed
> and returned to the free pool. Then each is added to mapping M2, this
> time with folio F2 at index 8 and F1 at index 9. Now the correct order
> to lock these folios in the order F2 followed by F1.
First of all, I appreciate your feedback. Thanks!
That case doesn't generate any dependency unless any other waits are
involved in. That should be handled in xxx_nested manner e.g.
folio_lock_nested() that I need to introduce. The work is in progress.
> I don't see a part of this patch where we clear pg_locked_wgen when the
> page is returned to the page allocator. Maybe I missed that.
You are right. pg_locked_wgen doesn't get cleared. However, DEPT works
this way:
folio_lock()
wait_for_pg_locked_cleared()
set_pg_locked() // (1) update pg_locked_wgen to the current wgen
... // there might be other waits
folio_unlock()
clear_pg_locked() // (2) check if there have been any waits since (1)
In other words, it's guranteed that pg_locked_wgen has been updated e.i.
(1) when DEPT refers to pg_locked_wgen e.i. (2). So I don't think it's
a problem.
> I think we should be tracking PG_locked dependencies in the owner
> of the folio. For files, that would be in the struct address_space.
> For anon memory, I think that's in the anon_vma, but if somebody told
> me it was in some other structure, I wouldn't argue with them.
I think it's a good point but it's a classification issue. folios owned
by struct address_space should be classified to e.g. address_space_class
and ones owned by struct anon_vma should be classified to e.g.
anon_vma_class. I will work on it to apply the insight you just gave
but better do it as follow-up patches since the initial patchset is
already too big to get reviewed.
> This requires slightly more complexity than lockdep currently has.
> We don't want to use a lockdep class for each folio, obviously. So we
> need something to say "I already have folio F1 locked, is it OK to lock
From DEPT's perspective, folio_lock(F1) and folio_lock(F2) are waits and
folio_unlock(F1) and folio_unlock(F2) are events. Since DEPT tracks
dependencies with specified classes between waits and events, DEPT's
interest in the following example is to detect a situation like:
< context X >
folio_lock(address_space_class'ed F1)
...
folio_lock(anon_vma_class'ed F2)
...
folio_unlock(anon_vma_class'ed F2)
...
folio_unlock(address_space_class'ed F1)
< context Y >
folio_lock(anon_vma_class'ed any folio)
...
folio_lock(address_space_class'ed any folio)
...
folio_unlock(address_space_class'ed any folio)
...
folio_unlock(anon_vma_class'ed any folio)
However, the following pattern should be manually annotated by
developers like using folio_lock_nested() or something. DEPT cannot
work with it automatically:
folio_lock(address_space_class'ed F1)
...
folio_lock(address_space_class'ed F2)
...
folio_unlock(address_space_class'ed F2)
...
folio_unlock(address_space_class'ed F1)
or
folio_lock(anon_vma_class'ed F1)
...
folio_lock(anon_vma_class'ed F2)
...
folio_unlock(anon_vma_class'ed F2)
...
folio_unlock(anon_vma_class'ed F1)
These should be explicitly annotated by developers if it's intended:
folio_lock(address_space_class'ed F1)
...
folio_lock_nested(address_space_class'ed F2)
...
folio_unlock(address_space_class'ed F2)
...
folio_unlock(address_space_class'ed F1)
or
folio_lock(anon_vma_class'ed F1)
...
folio_lock_nested(anon_vma_class'ed F2)
...
folio_unlock(anon_vma_class'ed F2)
...
folio_unlock(anon_vma_class'ed F1)
> folio F2?". Essentially figuring out how we can track all folios in a
> given mapping the same way, and making sure that we don't deadlock on
> folios in the same mapping.
At the moment, as I told you, DEPT cannot work with dependencies between
the same class'ed folios. However, it'd be much better if DEPT can work
with even those cases. Could you provide a scenario where a deadlock
happens between the same class'ed ones? Any idea how to detect for the
cases?
> If F1 and F2 are in different mappings, it's not a deadlock if F1 is in a
> filesystem mapping and F2 is in its backing dev. It's also not a deadlock
> if F1 and F2 are both filesystem folios and the inodes are both locked.
> See vfs_lock_two_folios() in fs/remap_range.c.
Yeah.. DEPT is a tracker to track dependencies between waits and events
even across different contexts, but not a magic unfortunately. That
lock ordering issue - with the same class'ed ones - should be resolved
in the manual manner as vfs_lock_two_folios() does.
> I have much less knowledge about anonymous memory locking order.
> Maybe it doesn't happen. Or about locking one anon and one file folio.
> For slab memory, we don't sleep on PG_locked (it's used as a spinlock bit).
> For other kinds of memory ... I don't know. Page migration is fun.
Anyway, the sophisticated classification you mentioned is necessary for
DEPT to be better especially for folio locking mechanism.
Thanks again!
Byungchul
^ permalink raw reply
* [PATCH V2 0/6] null_blk: fix init/exit races, leaks and cleanup
From: Zizhi Wo @ 2026-07-07 2:55 UTC (permalink / raw)
To: axboe, dlemoal, nilay, linux-block, kch, johannes.thumshirn,
kbusch, bvanassche
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
This series fixes several issues in null_blk around lock initialization,
concurrent configfs access, and module init/exit cleanup.
Patches 1-2 fix the uninitialized mutex. Following Bart's suggestion, the
fix now uses DEFINE_MUTEX() and renames the lock.
Patch 3 fixes configfs registration concurrency.
Patch 4 reorders resource release in null_exit() to match null_init().
Patch 5 fixes a data race where the NULLB_DEVICE_ATTR macro locklessly
overwrote dev state already set under the lock by apply_fn().
Patch 6 fixes a global tag_set leak on the null_init() error path.
See the individual patch descriptions for details.
Changes since v1:
- Added patches 4-6, and modify the lock name in patch 2.
https://lore.kernel.org/all/20260706123507.3809871-1-wozizhi@huaweicloud.com/
Zizhi Wo (6):
null_blk: use DEFINE_MUTEX for the file-scope mutex
null_blk: give the file-scope mutex a descriptive name
null_blk: register configfs subsystem after creating default devices
null_blk: move unregister_blkdev() after destroying dev in null_exit()
null_blk: don't locklessly overwrite dev state after apply_fn
null_blk: free global tag_set on init error path
drivers/block/null_blk/main.c | 70 +++++++++++++++++------------------
1 file changed, 34 insertions(+), 36 deletions(-)
--
2.52.0
^ permalink raw reply
* [PATCH V2 1/6] null_blk: use DEFINE_MUTEX for the file-scope mutex
From: Zizhi Wo @ 2026-07-07 2:55 UTC (permalink / raw)
To: axboe, dlemoal, nilay, linux-block, kch, johannes.thumshirn,
kbusch, bvanassche
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260707025542.1299859-1-wozizhi@huaweicloud.com>
From: Zizhi Wo <wozizhi@huawei.com>
In null_init(), mutex_init(&lock) currently happens after
configfs_register_subsystem(), which exposes the nullb subsystem to
userspace. A racing mkdir() into /sys/kernel/config/nullb/ can reach
null_find_dev_by_name() -> mutex_lock(&lock) before the mutex is
initialized, trigger warning:
[ 123.137788] DEBUG_LOCKS_WARN_ON(lock->magic != lock)
[ 123.137796] WARNING: kernel/locking/mutex.c:159 at mutex_lock+0x171/0x1c0, CPU#13: mkdir/1301
[ 123.140090] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4
......
[ 123.154926] Call Trace:
[ 123.155172] <TASK>
[ 123.155419] ? __pfx_mutex_lock+0x10/0x10
[ 123.156181] ? __pfx__raw_spin_lock+0x10/0x10
[ 123.156571] nullb_group_make_group+0x20/0x100 [null_blk]
[ 123.157011] configfs_mkdir+0x47b/0xc70
[ 123.157337] ? __pfx_configfs_mkdir+0x10/0x10
[ 123.157719] ? may_create_dentry+0x242/0x2e0
[ 123.158061] vfs_mkdir+0x2a9/0x6c0
[ 123.158352] filename_mkdirat+0x3dc/0x500
[ 123.158710] ? __pfx_filename_mkdirat+0x10/0x10
[ 123.159070] ? strncpy_from_user+0x3a/0x1d0
[ 123.159413] __x64_sys_mkdir+0x6b/0x90
[ 123.159760] do_syscall_64+0xea/0x600
Replace the runtime mutex_init(&lock) with a static DEFINE_MUTEX(lock)
declaration to fix this issue.
Fixes: 49c3b9266a71 ("block: null_blk: Improve device creation with configfs")
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/block/null_blk/main.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index f8c0fd57e041..eba204b27785 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -66,7 +66,7 @@ struct nullb_page {
#define NULLB_PAGE_FREE (MAP_SZ - 2)
static LIST_HEAD(nullb_list);
-static struct mutex lock;
+static DEFINE_MUTEX(lock);
static int null_major;
static DEFINE_IDA(nullb_indexes);
static struct blk_mq_tag_set tag_set;
@@ -2166,8 +2166,6 @@ static int __init null_init(void)
if (ret)
return ret;
- mutex_init(&lock);
-
null_major = register_blkdev(0, "nullb");
if (null_major < 0) {
ret = null_major;
--
2.52.0
^ permalink raw reply related
* [PATCH V2 2/6] null_blk: give the file-scope mutex a descriptive name
From: Zizhi Wo @ 2026-07-07 2:55 UTC (permalink / raw)
To: axboe, dlemoal, nilay, linux-block, kch, johannes.thumshirn,
kbusch, bvanassche
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260707025542.1299859-1-wozizhi@huaweicloud.com>
From: Zizhi Wo <wozizhi@huawei.com>
The file-scope lock mutex serializes access to global null_blk state,
including the nullb_list and device creation/removal. Rename it to
"nullb_global_lock" to make its purpose clear. No functional change.
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index eba204b27785..98f6935bb502 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -66,7 +66,7 @@ struct nullb_page {
#define NULLB_PAGE_FREE (MAP_SZ - 2)
static LIST_HEAD(nullb_list);
-static DEFINE_MUTEX(lock);
+static DEFINE_MUTEX(nullb_global_lock);
static int null_major;
static DEFINE_IDA(nullb_indexes);
static struct blk_mq_tag_set tag_set;
@@ -423,9 +423,9 @@ static int nullb_apply_submit_queues(struct nullb_device *dev,
{
int ret;
- mutex_lock(&lock);
+ mutex_lock(&nullb_global_lock);
ret = nullb_update_nr_hw_queues(dev, submit_queues, dev->poll_queues);
- mutex_unlock(&lock);
+ mutex_unlock(&nullb_global_lock);
return ret;
}
@@ -435,9 +435,9 @@ static int nullb_apply_poll_queues(struct nullb_device *dev,
{
int ret;
- mutex_lock(&lock);
+ mutex_lock(&nullb_global_lock);
ret = nullb_update_nr_hw_queues(dev, dev->submit_queues, poll_queues);
- mutex_unlock(&lock);
+ mutex_unlock(&nullb_global_lock);
return ret;
}
@@ -493,7 +493,7 @@ static ssize_t nullb_device_power_store(struct config_item *item,
return ret;
ret = count;
- mutex_lock(&lock);
+ mutex_lock(&nullb_global_lock);
if (!dev->power && newp) {
if (test_and_set_bit(NULLB_DEV_FL_UP, &dev->flags))
goto out;
@@ -516,7 +516,7 @@ static ssize_t nullb_device_power_store(struct config_item *item,
}
out:
- mutex_unlock(&lock);
+ mutex_unlock(&nullb_global_lock);
return ret;
}
@@ -707,10 +707,10 @@ nullb_group_drop_item(struct config_group *group, struct config_item *item)
struct nullb_device *dev = to_nullb_device(item);
if (test_and_clear_bit(NULLB_DEV_FL_UP, &dev->flags)) {
- mutex_lock(&lock);
+ mutex_lock(&nullb_global_lock);
dev->power = false;
null_del_dev(dev->nullb);
- mutex_unlock(&lock);
+ mutex_unlock(&nullb_global_lock);
}
nullb_del_fault_config(dev);
config_item_put(item);
@@ -2081,14 +2081,14 @@ static struct nullb *null_find_dev_by_name(const char *name)
{
struct nullb *nullb = NULL, *nb;
- mutex_lock(&lock);
+ mutex_lock(&nullb_global_lock);
list_for_each_entry(nb, &nullb_list, list) {
if (strcmp(nb->disk_name, name) == 0) {
nullb = nb;
break;
}
}
- mutex_unlock(&lock);
+ mutex_unlock(&nullb_global_lock);
return nullb;
}
@@ -2102,9 +2102,9 @@ static int null_create_dev(void)
if (!dev)
return -ENOMEM;
- mutex_lock(&lock);
+ mutex_lock(&nullb_global_lock);
ret = null_add_dev(dev);
- mutex_unlock(&lock);
+ mutex_unlock(&nullb_global_lock);
if (ret) {
null_free_dev(dev);
return ret;
@@ -2200,17 +2200,17 @@ static void __exit null_exit(void)
unregister_blkdev(null_major, "nullb");
- mutex_lock(&lock);
+ mutex_lock(&nullb_global_lock);
while (!list_empty(&nullb_list)) {
nullb = list_entry(nullb_list.next, struct nullb, list);
null_destroy_dev(nullb);
}
- mutex_unlock(&lock);
+ mutex_unlock(&nullb_global_lock);
if (tag_set.ops)
blk_mq_free_tag_set(&tag_set);
- mutex_destroy(&lock);
+ mutex_destroy(&nullb_global_lock);
}
module_init(null_init);
--
2.52.0
^ permalink raw reply related
* [PATCH V2 3/6] null_blk: register configfs subsystem after creating default devices
From: Zizhi Wo @ 2026-07-07 2:55 UTC (permalink / raw)
To: axboe, dlemoal, nilay, linux-block, kch, johannes.thumshirn,
kbusch, bvanassche
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260707025542.1299859-1-wozizhi@huaweicloud.com>
From: Zizhi Wo <wozizhi@huawei.com>
In null_init(), configfs_register_subsystem() currently runs before
register_blkdev(), so when null_blk is built as a module, a racing mkdir()
+ poweron from userspace can reach null_add_dev() while null_major is still
0. __add_disk() then hits WARN_ON(disk->minors) (major=0 with minors!=0)
and fails:
[root@fedora ~]# [ 2366.521436] WARNING: block/genhd.c:476 at __add_disk+0x8a7/0xde0,
[ 2366.523552] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
[ 2366.529081] CPU: 26 UID: 0 PID: 1600 Comm: sh Not tainted 7.2.0-rc1+ #66 PREEMPT(full)
......
[ 2366.547251] Call Trace:
[ 2366.547575] <TASK>
[ 2366.547831] ? _raw_spin_lock+0x84/0xe0
[ 2366.548260] add_disk_fwnode+0x114/0x560
[ 2366.548739] null_add_dev+0x102d/0x1b80 [null_blk]
[ 2366.549310] ? __pfx_null_add_dev+0x10/0x10 [null_blk]
[ 2366.549906] ? mutex_lock+0xde/0x1c0
[ 2366.550361] ? __pfx_mutex_lock+0x10/0x10
[ 2366.550827] nullb_device_power_store+0x1e7/0x280 [null_blk]
[ 2366.551499] ? __pfx_nullb_device_power_store+0x10/0x10 [null_blk]
[ 2366.552177] ? __kmalloc_cache_noprof+0x1f5/0x470
[ 2366.552748] ? configfs_write_iter+0x35c/0x4e0
[ 2366.553242] configfs_write_iter+0x286/0x4e0
[ 2366.553787] vfs_write+0x52d/0xd00
[ 2366.554169] ? __pfx_vfs_write+0x10/0x10
[ 2366.554679] ? __pfx___css_rstat_updated+0x10/0x10
[ 2366.555196] ? fdget_pos+0x1cf/0x4c0
[ 2366.555649] ksys_write+0xfc/0x1d0
......
Additionally, the err_dev path destroys all devices on nullb_list while
configfs is still registered. If a racing mkdir() + poweron puts a user
device on the list, null_destroy_dev()->null_free_dev() kfrees the user
device's nullb_device but /sys/kernel/config/nullb/<name> is still
reachable. Any userspace access to the item will trigger a UAF.
For simplicity, move configfs_register_subsystem() to the end to solve
the problems above. This also mirrors null_exit().
Fixes: 3bf2bd20734e ("nullb: add configfs interface")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 98f6935bb502..024c89b36ddc 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2162,15 +2162,9 @@ static int __init null_init(void)
config_group_init(&nullb_subsys.su_group);
mutex_init(&nullb_subsys.su_mutex);
- ret = configfs_register_subsystem(&nullb_subsys);
- if (ret)
- return ret;
-
null_major = register_blkdev(0, "nullb");
- if (null_major < 0) {
- ret = null_major;
- goto err_conf;
- }
+ if (null_major < 0)
+ return null_major;
for (i = 0; i < nr_devices; i++) {
ret = null_create_dev();
@@ -2178,6 +2172,10 @@ static int __init null_init(void)
goto err_dev;
}
+ ret = configfs_register_subsystem(&nullb_subsys);
+ if (ret)
+ goto err_dev;
+
pr_info("module loaded\n");
return 0;
@@ -2187,8 +2185,6 @@ static int __init null_init(void)
null_destroy_dev(nullb);
}
unregister_blkdev(null_major, "nullb");
-err_conf:
- configfs_unregister_subsystem(&nullb_subsys);
return ret;
}
--
2.52.0
^ permalink raw reply related
* [PATCH V2 4/6] null_blk: move unregister_blkdev() after destroying dev in null_exit()
From: Zizhi Wo @ 2026-07-07 2:55 UTC (permalink / raw)
To: axboe, dlemoal, nilay, linux-block, kch, johannes.thumshirn,
kbusch, bvanassche
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260707025542.1299859-1-wozizhi@huaweicloud.com>
In null_exit(), unregister_blkdev() was called before the null_blk
instances were destroyed, which is inconsistent with the cleanup order in
null_init(). Move it after null_destroy_dev() so that teardown happens in
the reverse order of initialization.
No functional change intended.
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huaweicloud.com>
---
drivers/block/null_blk/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 024c89b36ddc..cab51301560e 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2194,8 +2194,6 @@ static void __exit null_exit(void)
configfs_unregister_subsystem(&nullb_subsys);
- unregister_blkdev(null_major, "nullb");
-
mutex_lock(&nullb_global_lock);
while (!list_empty(&nullb_list)) {
nullb = list_entry(nullb_list.next, struct nullb, list);
@@ -2203,6 +2201,8 @@ static void __exit null_exit(void)
}
mutex_unlock(&nullb_global_lock);
+ unregister_blkdev(null_major, "nullb");
+
if (tag_set.ops)
blk_mq_free_tag_set(&tag_set);
--
2.52.0
^ permalink raw reply related
* [PATCH V2 6/6] null_blk: free global tag_set on init error path
From: Zizhi Wo @ 2026-07-07 2:55 UTC (permalink / raw)
To: axboe, dlemoal, nilay, linux-block, kch, johannes.thumshirn,
kbusch, bvanassche
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260707025542.1299859-1-wozizhi@huaweicloud.com>
From: Zizhi Wo <wozizhi@huawei.com>
If shared_tags is enabled, null_setup_tagset() allocates the global tag_set
via null_init_global_tag_set(). If device creation later fails, err_dev
destroys the default devices and calls unregister_blkdev(), but never frees
the global tag_set. Since module init failed, null_exit() is never invoked,
so the global tag_set's tags and maps are permanently leaked.
Free the global tag_set in err_dev, matching null_exit() which does
if (tag_set.ops) blk_mq_free_tag_set(&tag_set).
Fixes: 82f402fefa50 ("null_blk: add support for shared tags")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index e7555c47b671..1e24c7e23524 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -2187,6 +2187,8 @@ static int __init null_init(void)
null_destroy_dev(nullb);
}
unregister_blkdev(null_major, "nullb");
+ if (tag_set.ops)
+ blk_mq_free_tag_set(&tag_set);
return ret;
}
--
2.52.0
^ permalink raw reply related
* [PATCH V2 5/6] null_blk: don't locklessly overwrite dev state after apply_fn
From: Zizhi Wo @ 2026-07-07 2:55 UTC (permalink / raw)
To: axboe, dlemoal, nilay, linux-block, kch, johannes.thumshirn,
kbusch, bvanassche
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260707025542.1299859-1-wozizhi@huaweicloud.com>
From: Zizhi Wo <wozizhi@huawei.com>
The NULLB_DEVICE_ATTR macro unconditionally writes dev->NAME = new_value
after apply_fn() returns. For attributes with an apply_fn (submit_queues,
poll_queues), apply_fn already sets dev->NAME under &nullb_list_lock.
configfs serializes writes via a per-open-file mutex (buffer->mutex), so
two threads writing to the same attribute through separate open file
descriptions run the store callback concurrently. The macro's write is
redundant and lockless, so a concurrent store's losing thread can overwrite
the winner's value after apply_fn set it, making dev->submit_queues
mismatch the hardware state. null_map_queues() then hits a WARN_ON_ONCE and
falls back to a single queue.
Restructure the macro so that apply_fn attributes return directly after
apply_fn, and only non-apply_fn attributes write dev->NAME -- those are
only changeable while not CONFIGURED and have no live hardware state to
mismatch.
Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
---
drivers/block/null_blk/main.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index cab51301560e..e7555c47b671 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -360,13 +360,15 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page, \
ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\
if (ret < 0) \
return ret; \
- if (apply_fn) \
+ if (apply_fn) { \
ret = apply_fn(dev, new_value); \
- else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) \
- ret = -EBUSY; \
- if (ret < 0) \
- return ret; \
- dev->NAME = new_value; \
+ if (ret < 0) \
+ return ret; \
+ } else { \
+ if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) \
+ return -EBUSY; \
+ dev->NAME = new_value; \
+ } \
return count; \
} \
CONFIGFS_ATTR(nullb_device_, NAME);
--
2.52.0
^ permalink raw reply related
* Re: [PATCH V2 5/6] null_blk: don't locklessly overwrite dev state after apply_fn
From: Zizhi Wo @ 2026-07-07 3:38 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, nilay, linux-block, kch,
johannes.thumshirn, kbusch, bvanassche
Cc: linux-kernel, yangerkun, chengzhihao1
In-Reply-To: <20260707025542.1299859-6-wozizhi@huaweicloud.com>
在 2026/7/7 10:55, Zizhi Wo 写道:
> From: Zizhi Wo <wozizhi@huawei.com>
>
> The NULLB_DEVICE_ATTR macro unconditionally writes dev->NAME = new_value
> after apply_fn() returns. For attributes with an apply_fn (submit_queues,
> poll_queues), apply_fn already sets dev->NAME under &nullb_list_lock.
>
> configfs serializes writes via a per-open-file mutex (buffer->mutex), so
> two threads writing to the same attribute through separate open file
> descriptions run the store callback concurrently. The macro's write is
> redundant and lockless, so a concurrent store's losing thread can overwrite
> the winner's value after apply_fn set it, making dev->submit_queues
> mismatch the hardware state. null_map_queues() then hits a WARN_ON_ONCE and
> falls back to a single queue.
>
> Restructure the macro so that apply_fn attributes return directly after
> apply_fn, and only non-apply_fn attributes write dev->NAME -- those are
> only changeable while not CONFIGURED and have no live hardware state to
> mismatch.
>
> Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> ---
> drivers/block/null_blk/main.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index cab51301560e..e7555c47b671 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -360,13 +360,15 @@ nullb_device_##NAME##_store(struct config_item *item, const char *page, \
> ret = nullb_device_##TYPE##_attr_store(&new_value, page, count);\
> if (ret < 0) \
> return ret; \
> - if (apply_fn) \
> + if (apply_fn) { \
> ret = apply_fn(dev, new_value); \
> - else if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) \
> - ret = -EBUSY; \
> - if (ret < 0) \
> - return ret; \
> - dev->NAME = new_value; \
> + if (ret < 0) \
> + return ret; \
> + } else { \
> + if (test_bit(NULLB_DEV_FL_CONFIGURED, &dev->flags)) \
> + return -EBUSY; \
> + dev->NAME = new_value; \
> + } \
> return count; \
> } \
> CONFIGFS_ATTR(nullb_device_, NAME);
Sorry for the noise. I missed that nullb_update_nr_hw_queues() returns
early without storing the value when !dev->nullb, so dropping the
macro's trailing write would silently discard pre-power-on
configuration. I'll add the update logic on the !dev->nullb path in v3.
Any further comments are welcome.
Thanks,
Zizhi Wo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox