xarray: make xa_alloc_cyclic() return 0 on all success cases
authorPrzemek Kitszel <przemyslaw.kitszel@intel.com>
Thu, 20 Mar 2025 10:22:19 +0000 (11:22 +0100)
committerAndrew Morton <akpm@linux-foundation.org>
Mon, 12 May 2025 00:48:19 +0000 (17:48 -0700)
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.

Link: https://lkml.kernel.org/r/20250320102219.8101-1-przemyslaw.kitszel@intel.com
Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Suggested-by: Matthew Wilcox <willy@infradead.org>
Link: https://lore.kernel.org/netdev/Z9gUd-5t8b5NX2wE@casper.infradead.org
Cc: Andriy Shevchenko <andriy.shevchenko@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
include/linux/xarray.h
lib/test_xarray.c

index 78eede109b1a5ceed91964ad7459755d22d618d0..be850174e802e632f12190f40b73e6b5be6dce7c 100644 (file)
@@ -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;
 }
 
 /**
index 080a39d22e7362a9853d51a04f35eb640bf505fb..5ca0aefee9aa5fc854678d560fe90ab564e59e6a 100644 (file)
@@ -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,7 +1073,7 @@ 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);
@@ -1080,7 +1081,19 @@ static noinline void check_xa_alloc_3(struct xarray *xa, unsigned int base)
 
        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));
 }