slab: Fix obj_ext is mistakenly considered NULL due to race condition

PATCH Superseded Hao Ge 2 participants 8 messages Oct 23, 2025

Subsystems (2)

Memory management Slab allocator

lore

Share & tools

lore

Hao Ge

author

2025-10-23 01:22 UTC #0 lore raw

PATCH 1/1 slab: Fix obj_ext is mistakenly considered NULL due to race condition

From: Hao Ge <[email protected]>

If two competing threads enter alloc_slab_obj_exts(), and the thread that failed to allocate the object extension vector exits after the one that succeeded, it will mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.

Therefore, let's add an additional check when alloc_slab_obj_exts fails.

Signed-off-by Hao Ge <[email protected]>

---
 mm/slub.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

mm/slub.c +6 -3

index d4403341c9df..42276f0cc920 100644
@@ -2227,9 +2227,12 @@ prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p)
 	slab = virt_to_slab(p);
 	if (!slab_obj_exts(slab) &&
 	    alloc_slab_obj_exts(slab, s, flags, false)) {
-		pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
-			     __func__, s->name);
-		return NULL;
+		/* Recheck if a racing thread has successfully allocated slab->obj_exts. */
+		if (!slab_obj_exts(slab)) {
+			pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
+				     __func__, s->name);
+			return NULL;
+		}
 	}
 
 	return slab_obj_exts(slab) + obj_to_index(s, slab, p);

Signature · 1 line

2.25.1

Harry Yoo

2025-10-23 02:25 UTC #1 lore raw

Quoted 24 lines · Hao Ge

From: Hao Ge <[email protected]>
If two competing threads enter alloc_slab_obj_exts(), and the thread that failed to allocate the object extension vector exits after the one that succeeded, it will mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's add an additional check when alloc_slab_obj_exts fails.
Signed-off-by Hao Ge <[email protected]>
---
 mm/slub.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
mm/slub.c +0 -0
index d4403341c9df..42276f0cc920 100644
@@ -2227,9 +2227,12 @@ prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p)
 	slab = virt_to_slab(p);
 	if (!slab_obj_exts(slab) &&
 	    alloc_slab_obj_exts(slab, s, flags, false)) {

mm/slub.c prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p) · line 2236

Quoted 10 lines

mm/slub.c +6 -3

-		pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
-			     __func__, s->name);
-		return NULL;
+		/* Recheck if a racing thread has successfully allocated slab->obj_exts. */
+		if (!slab_obj_exts(slab)) {
+			pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
+				     __func__, s->name);
+			return NULL;
+		}
 	}

Maybe this patch is a bit paranoid... since if mark_failed_objexts_alloc() win cmpxchg() and then someone else allocates the object extension vector, the warning will still be printed anyway.

But anyway, I think there is a better way to do this:

mm/slub.c +6 -4

index dd4c85ea1038..d08d7580349d 100644
@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts)
 	}
 }
 
-static inline void mark_failed_objexts_alloc(struct slab *slab)
+static inline bool mark_failed_objexts_alloc(struct slab *slab)
 {
-	cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
+	return cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL) == 0;
 }
 
 static inline void handle_failed_objexts_alloc(unsigned long obj_exts,

@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
 #else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */
 
 static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) {}
-static inline void mark_failed_objexts_alloc(struct slab *slab) {}
+static inline bool mark_failed_objexts_alloc(struct slab *slab) { return true; }
 static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
 			struct slabobj_ext *vec, unsigned int objects) {}

@@ -2125,7 +2125,9 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
 	}
 	if (!vec) {
 		/* Mark vectors which failed to allocate */
-		mark_failed_objexts_alloc(slab);
+		if (!mark_failed_objexts_alloc(slab) &&
+			slab_obj_exts(slab))
+			return 0;
 
 		return -ENOMEM;
 	}

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Hao Ge

author

2025-10-23 03:12 UTC #2 lore raw

Hi Harry

Quoted 30 lines · Harry Yoo

Quoted 29 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter alloc_slab_obj_exts(), and the thread that failed to allocate the object extension vector exits after the one that succeeded, it will mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's add an additional check when alloc_slab_obj_exts fails.
Signed-off-by Hao Ge <[email protected]>
---
  mm/slub.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)
mm/slub.c +2 -3
index d4403341c9df..42276f0cc920 100644
@@ -2227,9 +2227,12 @@ prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p)
  	slab = virt_to_slab(p);
  	if (!slab_obj_exts(slab) &&
  	    alloc_slab_obj_exts(slab, s, flags, false)) {
-		pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
-			     __func__, s->name);
-		return NULL;
+		/* Recheck if a racing thread has successfully allocated slab->obj_exts. */
+		if (!slab_obj_exts(slab)) {

Quoted 8 lines

mm/slub.c prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p) · line 2236
Quoted 5 lines
mm/slub.c +4 -0
+			pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
+				     __func__, s->name);
+			return NULL;
+		}
  	}
Maybe this patch is a bit paranoid... since if mark_failed_objexts_alloc() win cmpxchg() and then someone else allocates the object extension vector, the warning will still be printed anyway.

The process that successfully allocates slab_exts will call handle_failed_objexts_alloc, setting ref->ct = CODETAG_EMPTY

to prevent the warning from being triggered.

Quoted 39 lines

But anyway, I think there is a better way to do this:

mm/slub.c +6 -4

index dd4c85ea1038..d08d7580349d 100644
@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts)
  	}
  }
 
-static inline void mark_failed_objexts_alloc(struct slab *slab)
+static inline bool mark_failed_objexts_alloc(struct slab *slab)
  {
-	cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
+	return cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL) == 0;
  }
 
  static inline void handle_failed_objexts_alloc(unsigned long obj_exts,

@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
  #else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */
 
  static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) {}
-static inline void mark_failed_objexts_alloc(struct slab *slab) {}
+static inline bool mark_failed_objexts_alloc(struct slab *slab) { return true; }
  static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
  			struct slabobj_ext *vec, unsigned int objects) {}

@@ -2125,7 +2125,9 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
  	}
  	if (!vec) {
  		/* Mark vectors which failed to allocate */
-		mark_failed_objexts_alloc(slab);
+		if (!mark_failed_objexts_alloc(slab) &&
+			slab_obj_exts(slab))
+			return 0;
 
  		return -ENOMEM;
  	}

Harry Yoo

2025-10-23 07:50 UTC #3 lore raw

Quoted 42 lines · Hao Ge

Hi Harry
Quoted 38 lines · Harry Yoo
Quoted 24 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter alloc_slab_obj_exts(), and the thread that failed to allocate the object extension vector exits after the one that succeeded, it will mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's add an additional check when alloc_slab_obj_exts fails.
Signed-off-by Hao Ge <[email protected]>
---
  mm/slub.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)
mm/slub.c +0 -0
index d4403341c9df..42276f0cc920 100644
@@ -2227,9 +2227,12 @@ prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p)
  	slab = virt_to_slab(p);
  	if (!slab_obj_exts(slab) &&
  	    alloc_slab_obj_exts(slab, s, flags, false)) {
mm/slub.c prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p) · line 2236
Quoted 10 lines
mm/slub.c +6 -3
-		pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
-			     __func__, s->name);
-		return NULL;
+		/* Recheck if a racing thread has successfully allocated slab->obj_exts. */
+		if (!slab_obj_exts(slab)) {
+			pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
+				     __func__, s->name);
+			return NULL;
+		}
  	}
Maybe this patch is a bit paranoid... since if mark_failed_objexts_alloc() win cmpxchg() and then someone else allocates the object extension vector, the warning will still be printed anyway.

Oh, just to be clear I was talking about the other warning:
pr_warn_once("%s, %s: Failed to create slab extension vector!", __func__, s->name);

Quoted 3 lines

The process that successfully allocates slab_exts will call handle_failed_objexts_alloc, setting ref->ct = CODETAG_EMPTY to prevent the warning from being triggered.

But yeah I see what you mean.

As you mentioned, if the process that failed to allocate the vector wins cmpxchg(), later process that successfully allocate the vector would call set_codetag_empty(), so no warning.

But if the process that allocates the vector wins cmpxchg(), then it won't call set_codetag_empty(), so the process that was trying to set OBJEXTS_ALLOC_FAIL now needs to set the tag.

Quoted 1 line

Quoted 1 line
But anyway, I think there is a better way to do this:

What do you think about the diff I suggested below, though?

Quoted 33 lines

Quoted 32 lines

mm/slub.c +6 -4

index dd4c85ea1038..d08d7580349d 100644
@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts)
  	}
  }
-static inline void mark_failed_objexts_alloc(struct slab *slab)
+static inline bool mark_failed_objexts_alloc(struct slab *slab)
  {
-	cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
+	return cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL) == 0;
  }
  static inline void handle_failed_objexts_alloc(unsigned long obj_exts,

@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
  #else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */
  static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) {}
-static inline void mark_failed_objexts_alloc(struct slab *slab) {}
+static inline bool mark_failed_objexts_alloc(struct slab *slab) { return true; }
  static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
  			struct slabobj_ext *vec, unsigned int objects) {}

@@ -2125,7 +2125,9 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
  	}
  	if (!vec) {
  		/* Mark vectors which failed to allocate */
-		mark_failed_objexts_alloc(slab);
+		if (!mark_failed_objexts_alloc(slab) &&
+			slab_obj_exts(slab))
+			return 0;
  		return -ENOMEM;
  	}

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Hao Ge

author

2025-10-23 08:24 UTC #4 lore raw

Hi Harry

Quoted 51 lines · Harry Yoo

Quoted 42 lines · Hao Ge
Hi Harry
Quoted 38 lines · Harry Yoo
Quoted 24 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter alloc_slab_obj_exts(), and the thread that failed to allocate the object extension vector exits after the one that succeeded, it will mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's add an additional check when alloc_slab_obj_exts fails.
Signed-off-by Hao Ge <[email protected]>
---
   mm/slub.c | 9 ++++++---
   1 file changed, 6 insertions(+), 3 deletions(-)
mm/slub.c +0 -0
index d4403341c9df..42276f0cc920 100644
@@ -2227,9 +2227,12 @@ prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p)
   	slab = virt_to_slab(p);
   	if (!slab_obj_exts(slab) &&
   	    alloc_slab_obj_exts(slab, s, flags, false)) {
mm/slub.c prepare_slab_obj_exts_hook(struct kmem_cache *s, gfp_t flags, void *p) · line 2236
Quoted 10 lines
mm/slub.c +6 -3
-		pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
-			     __func__, s->name);
-		return NULL;
+		/* Recheck if a racing thread has successfully allocated slab->obj_exts. */
+		if (!slab_obj_exts(slab)) {
+			pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
+				     __func__, s->name);
+			return NULL;
+		}
   	}
Maybe this patch is a bit paranoid... since if mark_failed_objexts_alloc() win cmpxchg() and then someone else allocates the object extension vector, the warning will still be printed anyway.
Oh, just to be clear I was talking about the other warning:
pr_warn_once("%s, %s: Failed to create slab extension vector!", __func__, s->name);
Quoted 3 lines
The process that successfully allocates slab_exts will call handle_failed_objexts_alloc, setting ref->ct = CODETAG_EMPTY to prevent the warning from being triggered.
But yeah I see what you mean.

Quoted 7 lines

As you mentioned, if the process that failed to allocate the vector wins cmpxchg(), later process that successfully allocate the vector would call set_codetag_empty(), so no warning.
But if the process that allocates the vector wins cmpxchg(), then it won't call set_codetag_empty(), so the process that was trying to set OBJEXTS_ALLOC_FAIL now needs to set the tag.

Yes, the case I'm encountering is exactly this one.

Quoted 3 lines

Quoted 2 lines
Quoted 2 lines
But anyway, I think there is a better way to do this:
What do you think about the diff I suggested below, though?

Sorry for the delayed response earlier; I was trying to deduce all possible scenarios.

It makes sense to me, and I will submit the V2 version based on this suggestion.

Thank you for your help.

Quoted 33 lines

mm/slub.c +6 -4

index dd4c85ea1038..d08d7580349d 100644
@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct slabobj_ext *obj_exts)
   	}
   }
-static inline void mark_failed_objexts_alloc(struct slab *slab)
+static inline bool mark_failed_objexts_alloc(struct slab *slab)
   {
-	cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
+	return cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL) == 0;
   }
   static inline void handle_failed_objexts_alloc(unsigned long obj_exts,

@@ -2076,7 +2076,7 @@ static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
   #else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */
   static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) {}
-static inline void mark_failed_objexts_alloc(struct slab *slab) {}
+static inline bool mark_failed_objexts_alloc(struct slab *slab) { return true; }
   static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
   			struct slabobj_ext *vec, unsigned int objects) {}

@@ -2125,7 +2125,9 @@ int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
   	}
   	if (!vec) {
   		/* Mark vectors which failed to allocate */
-		mark_failed_objexts_alloc(slab);
+		if (!mark_failed_objexts_alloc(slab) &&
+			slab_obj_exts(slab))
+			return 0;
   		return -ENOMEM;
   	}

Hao Ge

author

2025-10-23 08:47 UTC #5 lore raw

Hi Harry

Quoted 94 lines · Hao Ge

Hi Harry
Quoted 55 lines · Harry Yoo
Quoted 46 lines · Hao Ge
Hi Harry
Quoted 42 lines · Harry Yoo
Quoted 30 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter alloc_slab_obj_exts(), and the thread that failed to allocate the object extension vector exits after the one that succeeded, it will mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's add an additional check when alloc_slab_obj_exts fails.
Signed-off-by Hao Ge <[email protected]>
---
mm/slub.c | 9 ++++++---
   1 file changed, 6 insertions(+), 3 deletions(-)
mm/slub.c +0 -0
index d4403341c9df..42276f0cc920 100644
@@ -2227,9 +2227,12 @@ prepare_slab_obj_exts_hook(struct 
 kmem_cache *s, gfp_t flags, void *p)
        slab = virt_to_slab(p);
       if (!slab_obj_exts(slab) && alloc_slab_obj_exts(slab, s, flags, false)) { -        pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
-                 __func__, s->name);
-        return NULL;
+        /* Recheck if a racing thread has successfully allocated
Quoted 8 lines
slab->obj_exts. */
+        if (!slab_obj_exts(slab)) {
+            pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
+                     __func__, s->name);
+            return NULL;
+        }
       }
Maybe this patch is a bit paranoid... since if mark_failed_objexts_alloc() win cmpxchg() and then someone else allocates the object extension vector, the warning will still be printed anyway.
Oh, just to be clear I was talking about the other warning:
pr_warn_once("%s, %s: Failed to create slab extension vector!", __func__, s->name);
Quoted 3 lines
The process that successfully allocates slab_exts will call handle_failed_objexts_alloc, setting ref->ct = CODETAG_EMPTY to prevent the warning from being triggered.
But yeah I see what you mean.
Quoted 7 lines
As you mentioned, if the process that failed to allocate the vector wins cmpxchg(), later process that successfully allocate the vector would call set_codetag_empty(), so no warning.
But if the process that allocates the vector wins cmpxchg(), then it won't call set_codetag_empty(), so the process that was trying to set OBJEXTS_ALLOC_FAIL now needs to set the tag.
Yes, the case I'm encountering is exactly this one.
Quoted 3 lines
Quoted 2 lines
Quoted 2 lines
But anyway, I think there is a better way to do this:
What do you think about the diff I suggested below, though?
Sorry for the delayed response earlier; I was trying to deduce all possible scenarios.
It makes sense to me, and I will submit the V2 version based on this suggestion.
Thank you for your help.
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
mm/slub.c +2 -2
index dd4c85ea1038..d08d7580349d 100644
@@ -2052,9 +2052,9 @@ static inline void mark_objexts_empty(struct 
 slabobj_ext *obj_exts)
        }
    }
-static inline void mark_failed_objexts_alloc(struct slab *slab)
+static inline bool mark_failed_objexts_alloc(struct slab *slab)
    {
-    cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
+    return cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL) == 0;
    }

Quoted 8 lines

mm/slub.c static inline void · line 2080

Quoted 8 lines

mm/slub.c +1 -1

    static inline void handle_failed_objexts_alloc(unsigned long obj_exts,

@@ -2076,7 +2076,7 @@ static inline void 
 handle_failed_objexts_alloc(unsigned long obj_exts,
    #else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */
    static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) {}
-static inline void mark_failed_objexts_alloc(struct slab *slab) {}
+static inline bool mark_failed_objexts_alloc(struct slab *slab) { 
 return true; }

Maybe it returns false here.

When CONFIG_MEM_ALLOC_PROFILING_DEBUG is not enabled,

The following condition will never be executed:

if (!mark_failed_objexts_alloc(slab) && slab_obj_exts(slab))

if another process that allocates the vector, we will lose one count.

Quoted 14 lines

mm/slub.c +3 -1

    static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
                struct slabobj_ext *vec, unsigned int objects) {}

@@ -2125,7 +2125,9 @@ int alloc_slab_obj_exts(struct slab *slab, 
 struct kmem_cache *s,
        }
        if (!vec) {
            /* Mark vectors which failed to allocate */
-        mark_failed_objexts_alloc(slab);
+        if (!mark_failed_objexts_alloc(slab) &&
+            slab_obj_exts(slab))
+            return 0;
            return -ENOMEM;
        }

Harry Yoo

2025-10-23 09:06 UTC #6 lore raw

Quoted 118 lines · Hao Ge

Hi Harry
Quoted 103 lines · Hao Ge
Hi Harry
Quoted 60 lines · Harry Yoo
Quoted 50 lines · Hao Ge
Hi Harry
Quoted 46 lines · Harry Yoo
Quoted 32 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter alloc_slab_obj_exts(), and the thread that failed to allocate the object extension vector exits after the one that succeeded, it will mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger
warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's add an additional check when
alloc_slab_obj_exts fails.
Signed-off-by Hao Ge <[email protected]>
---
mm/slub.c | 9 ++++++---
   1 file changed, 6 insertions(+), 3 deletions(-)
mm/slub.c +0 -0
index d4403341c9df..42276f0cc920 100644
@@ -2227,9 +2227,12 @@ prepare_slab_obj_exts_hook(struct
 kmem_cache *s, gfp_t flags, void *p)
        slab = virt_to_slab(p);
       if (!slab_obj_exts(slab) && alloc_slab_obj_exts(slab, s, flags, false)) { -        pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
-                 __func__, s->name);
-        return NULL;
+        /* Recheck if a racing thread has successfully
Quoted 8 lines
allocated slab->obj_exts. */
+        if (!slab_obj_exts(slab)) {
+            pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
+                     __func__, s->name);
+            return NULL;
+        }
       }
Maybe this patch is a bit paranoid... since if mark_failed_objexts_alloc()
win cmpxchg() and then someone else allocates the object extension vector,
the warning will still be printed anyway.
Oh, just to be clear I was talking about the other warning:
pr_warn_once("%s, %s: Failed to create slab extension vector!", __func__, s->name);
Quoted 3 lines
The process that successfully allocates slab_exts will call handle_failed_objexts_alloc, setting ref->ct = CODETAG_EMPTY to prevent the warning from being triggered.
But yeah I see what you mean.
Quoted 7 lines
As you mentioned, if the process that failed to allocate the vector wins cmpxchg(), later process that successfully allocate the vector would call set_codetag_empty(), so no warning.
But if the process that allocates the vector wins cmpxchg(), then it won't call set_codetag_empty(), so the process that was trying to set OBJEXTS_ALLOC_FAIL now needs to set the tag.
Yes, the case I'm encountering is exactly this one.
Quoted 3 lines
Quoted 2 lines
Quoted 2 lines
But anyway, I think there is a better way to do this:
What do you think about the diff I suggested below, though?
Sorry for the delayed response earlier; I was trying to deduce all possible scenarios.
It makes sense to me, and I will submit the V2 version based on this suggestion.
Thank you for your help.
Quoted 17 lines
Quoted 17 lines
Quoted 17 lines
mm/slub.c +2 -2
index dd4c85ea1038..d08d7580349d 100644
@@ -2052,9 +2052,9 @@ static inline void
 mark_objexts_empty(struct slabobj_ext *obj_exts)
        }
    }
-static inline void mark_failed_objexts_alloc(struct slab *slab)
+static inline bool mark_failed_objexts_alloc(struct slab *slab)
    {
-    cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
+    return cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL) == 0;
    }
static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
mm/slub.c +0 -0
@@ -2076,7 +2076,7 @@ static inline void
 handle_failed_objexts_alloc(unsigned long obj_exts,
    #else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */
static inline void mark_objexts_empty(struct slabobj_ext *obj_exts) {}
mm/slub.c +1 -1
-static inline void mark_failed_objexts_alloc(struct slab *slab) {}
+static inline bool mark_failed_objexts_alloc(struct slab
*slab) { return true; }
Maybe it returns false here.

Quoted 5 lines

When CONFIG_MEM_ALLOC_PROFILING_DEBUG is not enabled,
The following condition will never be executed:
if (!mark_failed_objexts_alloc(slab) && slab_obj_exts(slab))

Good point. But without CONFIG_MEM_ALLOC_PROFILING_DEBUG, we don't know if someone else successfully allocated the vector or not (unlike, with CONFIG_MEM_ALLOC_PROFILING_DEBUG enabled, we know that when we lose cmpxchg()). We cannot "fix" the case where a process fails to allocate the vector but another allocates the vector.

So I'm not sure if checking slab_obj_exts() once more is worth it in this case, but I'm fine with either way.

Quoted 1 line

if another process that allocates the vector, we will lose one count.

By "one count" you mean skipping accounting the object in memory profiling, right?

Quoted 15 lines

static inline void handle_failed_objexts_alloc(unsigned long obj_exts,
struct slabobj_ext *vec, unsigned int objects) {}

mm/slub.c +3 -1

@@ -2125,7 +2125,9 @@ int alloc_slab_obj_exts(struct slab
 *slab, struct kmem_cache *s,
        }
        if (!vec) {
            /* Mark vectors which failed to allocate */
-        mark_failed_objexts_alloc(slab);
+        if (!mark_failed_objexts_alloc(slab) &&
+            slab_obj_exts(slab))
+            return 0;
            return -ENOMEM;
        }

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Hao Ge

author

2025-10-23 09:11 UTC #7 lore raw

Hi Harry

Quoted 125 lines · Harry Yoo

Quoted 113 lines · Hao Ge
Hi Harry
Quoted 99 lines · Hao Ge
Hi Harry
Quoted 60 lines · Harry Yoo
Quoted 50 lines · Hao Ge
Hi Harry
Quoted 46 lines · Harry Yoo
Quoted 32 lines · Hao Ge
From: Hao Ge <[email protected]>
If two competing threads enter alloc_slab_obj_exts(), and the thread that failed to allocate the object extension vector exits after the one that succeeded, it will mistakenly assume slab->obj_ext is still empty due to its own allocation failure. This will then trigger
warnings enforced by CONFIG_MEM_ALLOC_PROFILING_DEBUG checks in the subsequent free path.
Therefore, let's add an additional check when
alloc_slab_obj_exts fails.
Signed-off-by Hao Ge <[email protected]>
---
    mm/slub.c | 9 ++++++---
    1 file changed, 6 insertions(+), 3 deletions(-)
mm/slub.c +1 -3
index d4403341c9df..42276f0cc920 100644
@@ -2227,9 +2227,12 @@ prepare_slab_obj_exts_hook(struct
 kmem_cache *s, gfp_t flags, void *p)
        slab = virt_to_slab(p);
        if (!slab_obj_exts(slab) &&
            alloc_slab_obj_exts(slab, s, flags, false)) {
-        pr_warn_once("%s, %s: Failed to create slab
 extension vector!\n",
-                 __func__, s->name);
-        return NULL;
+        /* Recheck if a racing thread has successfully
Quoted 8 lines
allocated slab->obj_exts. */
+        if (!slab_obj_exts(slab)) {
+            pr_warn_once("%s, %s: Failed to create slab extension vector!\n",
+                     __func__, s->name);
+            return NULL;
+        }
       }
Maybe this patch is a bit paranoid... since if mark_failed_objexts_alloc()
win cmpxchg() and then someone else allocates the object extension vector,
the warning will still be printed anyway.
Oh, just to be clear I was talking about the other warning:
pr_warn_once("%s, %s: Failed to create slab extension vector!", __func__, s->name);
Quoted 3 lines
The process that successfully allocates slab_exts will call handle_failed_objexts_alloc, setting ref->ct = CODETAG_EMPTY to prevent the warning from being triggered.
But yeah I see what you mean.
Quoted 7 lines
As you mentioned, if the process that failed to allocate the vector wins cmpxchg(), later process that successfully allocate the vector would call set_codetag_empty(), so no warning.
But if the process that allocates the vector wins cmpxchg(), then it won't call set_codetag_empty(), so the process that was trying to set OBJEXTS_ALLOC_FAIL now needs to set the tag.
Yes, the case I'm encountering is exactly this one.
Quoted 2 lines
Quoted 1 line
Quoted 1 line
But anyway, I think there is a better way to do this:
What do you think about the diff I suggested below, though?
Sorry for the delayed response earlier; I was trying to deduce all possible scenarios.
It makes sense to me, and I will submit the V2 version based on this suggestion.
Thank you for your help.
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
mm/slub.c +2 -2
index dd4c85ea1038..d08d7580349d 100644
@@ -2052,9 +2052,9 @@ static inline void
 mark_objexts_empty(struct slabobj_ext *obj_exts)
        }
    }
-static inline void mark_failed_objexts_alloc(struct slab *slab)
+static inline bool mark_failed_objexts_alloc(struct slab *slab)
    {
-    cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL);
+    return cmpxchg(&slab->obj_exts, 0, OBJEXTS_ALLOC_FAIL) == 0;
    }
    static inline void handle_failed_objexts_alloc(unsigned
 long obj_exts,
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
mm/slub.c static inline void · line 2081
Quoted 8 lines
mm/slub.c +1 -1
@@ -2076,7 +2076,7 @@ static inline void
 handle_failed_objexts_alloc(unsigned long obj_exts,
    #else /* CONFIG_MEM_ALLOC_PROFILING_DEBUG */
    static inline void mark_objexts_empty(struct slabobj_ext
 *obj_exts) {}
-static inline void mark_failed_objexts_alloc(struct slab *slab) {}
+static inline bool mark_failed_objexts_alloc(struct slab
 *slab) { return true; }
Maybe it returns false here.
Quoted 5 lines
When CONFIG_MEM_ALLOC_PROFILING_DEBUG is not enabled,
The following condition will never be executed:
if (!mark_failed_objexts_alloc(slab) && slab_obj_exts(slab))
Good point. But without CONFIG_MEM_ALLOC_PROFILING_DEBUG, we don't know if someone else successfully allocated the vector or not (unlike, with CONFIG_MEM_ALLOC_PROFILING_DEBUG enabled, we know that when we lose cmpxchg()). We cannot "fix" the case where a process fails to allocate the vector but another allocates the vector.

Quoted 6 lines

So I'm not sure if checking slab_obj_exts() once more is worth it in this case, but I'm fine with either way.
Quoted 1 line
if another process that allocates the vector, we will lose one count.
By "one count" you mean skipping accounting the object in memory profiling, right?

Yes.

Quoted 16 lines

mm/slub.c +0 -0

    static inline void handle_failed_objexts_alloc(unsigned

long obj_exts,
struct slabobj_ext *vec, unsigned int objects) {}

mm/slub.c +3 -1

@@ -2125,7 +2125,9 @@ int alloc_slab_obj_exts(struct slab
 *slab, struct kmem_cache *s,
        }
        if (!vec) {
            /* Mark vectors which failed to allocate */
-        mark_failed_objexts_alloc(slab);
+        if (!mark_failed_objexts_alloc(slab) &&
+            slab_obj_exts(slab))
+            return 0;
            return -ENOMEM;
        }