Re: [PATCH 1/2] test_xarray: add tests for advanced multi-index use
From: Luis Chamberlain
Date: Fri Nov 17 2023 - 15:54:22 EST
On Wed, Nov 15, 2023 at 11:02:59PM +0800, kernel test robot wrote:
> [ 304.121213][ T1] WARNING: suspicious RCU usage
> [ 304.122111][ T1] 6.6.0-12894-g68f563aa7e55 #1 Tainted: G N
> [ 304.123404][ T1] -----------------------------
> [ 304.124297][ T1] include/linux/xarray.h:1200 suspicious rcu_dereference_check() usage!
> [ 304.125787][ T1]
> [ 304.125787][ T1] other info that might help us debug this:
> [ 304.125787][ T1]
> [ 304.127648][ T1]
> [ 304.127648][ T1] rcu_scheduler_active = 2, debug_locks = 1
> [ 304.129454][ T1] no locks held by swapper/1.
> [ 304.130560][ T1]
> [ 304.130560][ T1] stack backtrace:
> [ 304.131863][ T1] CPU: 0 PID: 1 Comm: swapper Tainted: G N 6.6.0-12894-g68f563aa7e55 #1
> [ 304.132791][ T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
> [ 304.132791][ T1] Call Trace:
> [ 304.132791][ T1] <TASK>
> [ 304.132791][ T1] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 1))
> [ 304.132791][ T1] lockdep_rcu_suspicious (include/linux/context_tracking.h:153 kernel/locking/lockdep.c:6712)
> [ 304.132791][ T1] xas_start (include/linux/xarray.h:1200 include/linux/xarray.h:1198 lib/xarray.c:190)
> [ 304.132791][ T1] xas_load (lib/xarray.c:237)
> [ 304.132791][ T1] xas_store (lib/xarray.c:789)
> [ 304.132791][ T1] ? xa_load (include/linux/rcupdate.h:306 include/linux/rcupdate.h:780 lib/xarray.c:1465)
> [ 304.132791][ T1] check_xa_multi_store_adv_delete+0xf0/0x120
Yup... I forgot to lock on deletion. I also decided to polish this up
some more and also address some soft lockups which can happen on low
end test machines when we test for order 20, which has 1<<20 entries
(1,048,576). The patch below fixes all this. I'll spin this series up
and send a v2.
diff --git a/lib/test_xarray.c b/lib/test_xarray.c
index 3c19d12c1bf5..ac0e887ccbd6 100644
--- a/lib/test_xarray.c
+++ b/lib/test_xarray.c
@@ -32,6 +32,16 @@ void xa_dump(const struct xarray *xa) { }
} while (0)
#endif
+/*
+ * Can be used in contexts which busy loop on large number of entries but can
+ * sleep and timing is if no importance to test correctness.
+ */
+#define XA_BUG_ON_RELAX(xa, x) do { \
+ if ((tests_run % 1000) == 0) \
+ schedule(); \
+ XA_BUG_ON(xa, x); \
+} while (0)
+
static void *xa_mk_index(unsigned long index)
{
return xa_mk_value(index & LONG_MAX);
@@ -728,12 +738,17 @@ static noinline void check_multi_store(struct xarray *xa)
}
#ifdef CONFIG_XARRAY_MULTI
+/* mimics page cache __filemap_add_folio() */
static noinline void check_xa_multi_store_adv_add(struct xarray *xa,
unsigned long index,
unsigned int order,
void *p)
{
XA_STATE(xas, xa, index);
+ unsigned int nrpages = 1UL << order;
+
+ /* users are responsible for index alignemnt to the order when adding */
+ XA_BUG_ON(xa, index & (nrpages - 1));
xas_set_order(&xas, index, order);
@@ -750,23 +765,48 @@ static noinline void check_xa_multi_store_adv_add(struct xarray *xa,
XA_BUG_ON(xa, xas_error(&xas));
}
+/* mimics page_cache_delete() */
+static noinline void check_xa_multi_store_adv_del_entry(struct xarray *xa,
+ unsigned long index,
+ unsigned int order)
+{
+ XA_STATE(xas, xa, index);
+
+ xas_set_order(&xas, index, order);
+ xas_store(&xas, NULL);
+ xas_init_marks(&xas);
+}
+
static noinline void check_xa_multi_store_adv_delete(struct xarray *xa,
unsigned long index,
unsigned int order)
{
- unsigned int nrpages = 1UL << order;
- unsigned long base = round_down(index, nrpages);
- XA_STATE(xas, xa, base);
+ xa_lock_irq(xa);
+ check_xa_multi_store_adv_del_entry(xa, index, order);
+ xa_unlock_irq(xa);
+}
- xas_set_order(&xas, base, order);
- xas_store(&xas, NULL);
- xas_init_marks(&xas);
+/* mimics page cache filemap_get_entry() */
+static noinline void *test_get_entry(struct xarray *xa, unsigned long index)
+{
+ XA_STATE(xas, xa, index);
+ void *p;
+
+ rcu_read_lock();
+repeat:
+ xas_reset(&xas);
+ p = xas_load(&xas);
+ if (xas_retry(&xas, p))
+ goto repeat;
+ rcu_read_unlock();
+
+ return p;
}
static unsigned long some_val = 0xdeadbeef;
static unsigned long some_val_2 = 0xdeaddead;
-/* mimics the page cache */
+/* mimics the page cache usage */
static noinline void check_xa_multi_store_adv(struct xarray *xa,
unsigned long pos,
unsigned int order)
@@ -783,13 +823,13 @@ static noinline void check_xa_multi_store_adv(struct xarray *xa,
check_xa_multi_store_adv_add(xa, base, order, &some_val);
for (i = 0; i < nrpages; i++)
- XA_BUG_ON(xa, xa_load(xa, base + i) != &some_val);
+ XA_BUG_ON_RELAX(xa, test_get_entry(xa, base + i) != &some_val);
- XA_BUG_ON(xa, xa_load(xa, next_index) != NULL);
+ XA_BUG_ON(xa, test_get_entry(xa, next_index) != NULL);
/* Use order 0 for the next item */
check_xa_multi_store_adv_add(xa, next_index, 0, &some_val_2);
- XA_BUG_ON(xa, xa_load(xa, next_index) != &some_val_2);
+ XA_BUG_ON(xa, test_get_entry(xa, next_index) != &some_val_2);
/* Remove the next item */
check_xa_multi_store_adv_delete(xa, next_index, 0);
@@ -798,7 +838,8 @@ static noinline void check_xa_multi_store_adv(struct xarray *xa,
check_xa_multi_store_adv_add(xa, next_index, order, &some_val_2);
for (i = 0; i < nrpages; i++)
- XA_BUG_ON(xa, xa_load(xa, next_index + i) != &some_val_2);
+ XA_BUG_ON_RELAX(xa,
+ test_get_entry(xa, next_index + i) != &some_val_2);
check_xa_multi_store_adv_delete(xa, next_index, order);
check_xa_multi_store_adv_delete(xa, base, order);
@@ -812,13 +853,15 @@ static noinline void check_xa_multi_store_adv(struct xarray *xa,
check_xa_multi_store_adv_add(xa, next_index, order, &some_val_2);
for (i = 0; i < nrpages; i++)
- XA_BUG_ON(xa, xa_load(xa, base + i) != NULL);
+ XA_BUG_ON_RELAX(xa, test_get_entry(xa, base + i) != NULL);
for (i = 0; i < nrpages; i++)
- XA_BUG_ON(xa, xa_load(xa, next_index + i) != &some_val_2);
+ XA_BUG_ON_RELAX(xa,
+ test_get_entry(xa, next_index + i) != &some_val_2);
for (i = 0; i < nrpages; i++)
- XA_BUG_ON(xa, xa_load(xa, next_next_index + i) != NULL);
+ XA_BUG_ON_RELAX(xa,
+ test_get_entry(xa, next_next_index + i) != NULL);
check_xa_multi_store_adv_delete(xa, next_index, order);
XA_BUG_ON(xa, !xa_empty(xa));
@@ -828,13 +871,15 @@ static noinline void check_xa_multi_store_adv(struct xarray *xa,
check_xa_multi_store_adv_add(xa, next_next_index, order, &some_val_2);
for (i = 0; i < nrpages; i++)
- XA_BUG_ON(xa, xa_load(xa, base + i) != NULL);
+ XA_BUG_ON_RELAX(xa,
+ test_get_entry(xa, base + i) != NULL);
for (i = 0; i < nrpages; i++)
- XA_BUG_ON(xa, xa_load(xa, next_index + i) != NULL);
+ XA_BUG_ON_RELAX(xa, test_get_entry(xa, next_index + i) != NULL);
for (i = 0; i < nrpages; i++)
- XA_BUG_ON(xa, xa_load(xa, next_next_index + i) != &some_val_2);
+ XA_BUG_ON_RELAX(xa,
+ test_get_entry(xa, next_next_index + i) != &some_val_2);
check_xa_multi_store_adv_delete(xa, next_next_index, order);
XA_BUG_ON(xa, !xa_empty(xa));