@@ -965,10 +965,12 @@ static inline int __must_check xa_alloc_irq(struct xarray *xa, u32 *id,
* Must only be operated on an xarray initialized with flag XA_FLAGS_ALLOC set
* in xa_init_flags().
*
+ * Note that callers interested in whether wrapping has occurred should
+ * use __xa_alloc_cyclic() instead.
+ *
* Context: Any context. Takes and releases the xa_lock. May sleep if
* the @gfp flags permit.
- * Return: 0 if the allocation succeeded without wrapping. 1 if the
- * allocation succeeded after wrapping, -ENOMEM if memory could not be
+ * Return: 0 if the allocation succeeded, -ENOMEM if memory could not be
* allocated or -EBUSY if there are no free entries in @limit.
*/
static inline int xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry,
@@ -981,7 +983,7 @@ static inline int xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry,
err = __xa_alloc_cyclic(xa, id, entry, limit, next, gfp);
xa_unlock(xa);
- return err;
+ return err < 0 ? err : 0;
}
/**
@@ -1002,10 +1004,12 @@ static inline int xa_alloc_cyclic(struct xarray *xa, u32 *id, void *entry,
* Must only be operated on an xarray initialized with flag XA_FLAGS_ALLOC set
* in xa_init_flags().
*
+ * Note that callers interested in whether wrapping has occurred should
+ * use __xa_alloc_cyclic() instead.
+ *
* Context: Any context. Takes and releases the xa_lock while
* disabling softirqs. May sleep if the @gfp flags permit.
- * Return: 0 if the allocation succeeded without wrapping. 1 if the
- * allocation succeeded after wrapping, -ENOMEM if memory could not be
+ * Return: 0 if the allocation succeeded, -ENOMEM if memory could not be
* allocated or -EBUSY if there are no free entries in @limit.
*/
static inline int xa_alloc_cyclic_bh(struct xarray *xa, u32 *id, void *entry,
@@ -1018,7 +1022,7 @@ static inline int xa_alloc_cyclic_bh(struct xarray *xa, u32 *id, void *entry,
err = __xa_alloc_cyclic(xa, id, entry, limit, next, gfp);
xa_unlock_bh(xa);
- return err;
+ return err < 0 ? err : 0;
}
/**
@@ -1039,10 +1043,12 @@ static inline int xa_alloc_cyclic_bh(struct xarray *xa, u32 *id, void *entry,
* Must only be operated on an xarray initialized with flag XA_FLAGS_ALLOC set
* in xa_init_flags().
*
+ * Note that callers interested in whether wrapping has occurred should
+ * use __xa_alloc_cyclic() instead.
+ *
* Context: Process context. Takes and releases the xa_lock while
* disabling interrupts. May sleep if the @gfp flags permit.
- * Return: 0 if the allocation succeeded without wrapping. 1 if the
- * allocation succeeded after wrapping, -ENOMEM if memory could not be
+ * Return: 0 if the allocation succeeded, -ENOMEM if memory could not be
* allocated or -EBUSY if there are no free entries in @limit.
*/
static inline int xa_alloc_cyclic_irq(struct xarray *xa, u32 *id, void *entry,
@@ -1055,7 +1061,7 @@ static inline int xa_alloc_cyclic_irq(struct xarray *xa, u32 *id, void *entry,
err = __xa_alloc_cyclic(xa, id, entry, limit, next, gfp);
xa_unlock_irq(xa);
- return err;
+ return err < 0 ? err : 0;
}
/**
@@ -1040,6 +1040,7 @@ static noinline void check_xa_alloc_3(struct xarray *xa, unsigned int base)
unsigned int i, id;
unsigned long index;
void *entry;
+ int ret;
XA_BUG_ON(xa, xa_alloc_cyclic(xa, &id, xa_mk_index(1), limit,
&next, GFP_KERNEL) != 0);
@@ -1059,7 +1060,7 @@ static noinline void check_xa_alloc_3(struct xarray *xa, unsigned int base)
else
entry = xa_mk_index(i - 0x3fff);
XA_BUG_ON(xa, xa_alloc_cyclic(xa, &id, entry, limit,
- &next, GFP_KERNEL) != (id == 1));
+ &next, GFP_KERNEL) != 0);
XA_BUG_ON(xa, xa_mk_index(id) != entry);
}
@@ -1072,15 +1073,27 @@ static noinline void check_xa_alloc_3(struct xarray *xa, unsigned int base)
xa_limit_32b, &next, GFP_KERNEL) != 0);
XA_BUG_ON(xa, id != UINT_MAX);
XA_BUG_ON(xa, xa_alloc_cyclic(xa, &id, xa_mk_index(base),
- xa_limit_32b, &next, GFP_KERNEL) != 1);
+ xa_limit_32b, &next, GFP_KERNEL) != 0);
XA_BUG_ON(xa, id != base);
XA_BUG_ON(xa, xa_alloc_cyclic(xa, &id, xa_mk_index(base + 1),
xa_limit_32b, &next, GFP_KERNEL) != 0);
XA_BUG_ON(xa, id != base + 1);
xa_for_each(xa, index, entry)
xa_erase_index(xa, index);
+ XA_BUG_ON(xa, !xa_empty(xa));
+ /* check wrap-around return of __xa_alloc_cyclic() */
+ next = UINT_MAX;
+ XA_BUG_ON(xa, xa_alloc_cyclic(xa, &id, xa_mk_index(UINT_MAX),
+ xa_limit_32b, &next, GFP_KERNEL) != 0);
+ xa_lock(xa);
+ ret = __xa_alloc_cyclic(xa, &id, xa_mk_index(base), xa_limit_32b,
+ &next, GFP_KERNEL);
+ xa_unlock(xa);
+ XA_BUG_ON(xa, ret != 1);
+ xa_for_each(xa, index, entry)
+ xa_erase_index(xa, index);
XA_BUG_ON(xa, !xa_empty(xa));
}
Change xa_alloc_cyclic() to return 0 even on wrap-around. Do the same for xa_alloc_cyclic_irq() and xa_alloc_cyclic_bh(). This will prevent any future bug of treating return of 1 as an error: int ret = xa_alloc_cyclic(...) if (ret) // currently mishandles ret==1 goto failure; If there will be someone interested in when wrap-around occurs, there is still __xa_alloc_cyclic() that behaves as before. For now there is no such user. Suggested-by: Matthew Wilcox <willy@infradead.org> Link: https://lore.kernel.org/netdev/Z9gUd-5t8b5NX2wE@casper.infradead.org Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> --- CC: Michal Swiatkowski <michal.swiatkowski@linux.intel.com> CC: Pierre Riteau <pierre@stackhpc.com> CC: Andrew Morton <akpm@linux-foundation.org> CC: linux-fsdevel@vger.kernel.org CC: linux-mm@kvack.org CC: linux-kernel@vger.kernel.org Thanks to Andy and Dave for internal review feedback CC: Andy Shevchenko <andriy.shevchenko@intel.com> CC: Dave Hansen <dave.hansen@intel.com> --- include/linux/xarray.h | 24 +++++++++++++++--------- lib/test_xarray.c | 17 +++++++++++++++-- 2 files changed, 30 insertions(+), 11 deletions(-)