All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hao Ge <hao.ge@linux.dev>
To: Harry Yoo <harry.yoo@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Lameter <cl@gentwo.org>,
	David Rientjes <rientjes@google.com>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Suren Baghdasaryan <surenb@google.com>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Hao Ge <gehao@kylinos.cn>
Subject: Re: [PATCH] slab: Fix obj_ext is mistakenly considered NULL due to race condition
Date: Thu, 23 Oct 2025 11:11:56 +0800	[thread overview]
Message-ID: <b8c90552-7be6-45bb-b586-ee21f63499c8@linux.dev> (raw)
In-Reply-To: <aPmR6Fz8HxYk4rTF@hyeyoo>

Hi Harry


On 2025/10/23 10:24, Harry Yoo wrote:
> On Thu, Oct 23, 2025 at 09:21:17AM +0800, Hao Ge wrote:
>> From: Hao Ge <gehao@kylinos.cn>
>>
>> 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 <gehao@kylinos.cn>
>> ---
>>   mm/slub.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index d4403341c9df..42276f0cc920 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -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;
>> +		}
>>   	}
> 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.


> But anyway, I think there is a better way to do this:
>
> diff --git a/mm/slub.c b/mm/slub.c
> index dd4c85ea1038..d08d7580349d 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -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;
>   	}
>


  reply	other threads:[~2025-10-23  3:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-23  1:21 [PATCH] slab: Fix obj_ext is mistakenly considered NULL due to race condition Hao Ge
2025-10-23  2:24 ` Harry Yoo
2025-10-23  3:11   ` Hao Ge [this message]
2025-10-23  7:50     ` Harry Yoo
2025-10-23  8:23       ` Hao Ge
2025-10-23  8:46         ` Hao Ge
2025-10-23  9:06           ` Harry Yoo
2025-10-23  9:11             ` Hao Ge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8c90552-7be6-45bb-b586-ee21f63499c8@linux.dev \
    --to=hao.ge@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=gehao@kylinos.cn \
    --cc=harry.yoo@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.