From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA264C433EF for ; Wed, 8 Jun 2022 03:05:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 54A528D0001; Tue, 7 Jun 2022 23:05:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4F87C6B0073; Tue, 7 Jun 2022 23:05:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3E68B8D0001; Tue, 7 Jun 2022 23:05:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 2EA1D6B0072 for ; Tue, 7 Jun 2022 23:05:11 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id D40C561466 for ; Wed, 8 Jun 2022 03:05:10 +0000 (UTC) X-FDA: 79553577180.12.97751D0 Received: from out30-133.freemail.mail.aliyun.com (out30-133.freemail.mail.aliyun.com [115.124.30.133]) by imf04.hostedemail.com (Postfix) with ESMTP id 8EED94002D for ; Wed, 8 Jun 2022 03:05:09 +0000 (UTC) X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R131e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018046051;MF=rongwei.wang@linux.alibaba.com;NM=1;PH=DS;RN=11;SR=0;TI=SMTPD_---0VFj.fTT_1654657496; Received: from 30.240.97.53(mailfrom:rongwei.wang@linux.alibaba.com fp:SMTPD_---0VFj.fTT_1654657496) by smtp.aliyun-inc.com; Wed, 08 Jun 2022 11:04:58 +0800 Message-ID: <29723aaa-5e28-51d3-7f87-9edf0f7b9c33@linux.alibaba.com> Date: Wed, 8 Jun 2022 11:04:56 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.0 Subject: Re: [PATCH 1/3] mm/slub: fix the race between validate_slab and slab_free To: Christoph Lameter Cc: David Rientjes , songmuchun@bytedance.com, Hyeonggon Yoo <42.hyeyoo@gmail.com>, akpm@linux-foundation.org, vbabka@suse.cz, roman.gushchin@linux.dev, iamjoonsoo.kim@lge.com, penberg@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20220529081535.69275-1-rongwei.wang@linux.alibaba.com> <9794df4f-3ffe-4e99-0810-a1346b139ce8@linux.alibaba.com> Content-Language: en-US From: Rongwei Wang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Stat-Signature: 6x18jwshrocminmk4j1j814smiabdjo3 Authentication-Results: imf04.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=alibaba.com; spf=pass (imf04.hostedemail.com: domain of rongwei.wang@linux.alibaba.com designates 115.124.30.133 as permitted sender) smtp.mailfrom=rongwei.wang@linux.alibaba.com X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 8EED94002D X-HE-Tag: 1654657509-682446 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 6/7/22 8:14 PM, Christoph Lameter wrote: > On Fri, 3 Jun 2022, Rongwei Wang wrote: > >> Recently, I am also find other ways to solve this. That case was provided by >> Muchun is useful (Thanks Muchun!). Indeed, it seems that use n->list_lock here >> is unwise. Actually, I'm not sure if you recognize the existence of such race? >> If all agrees this race, then the next question may be: do we want to solve >> this problem? or as David said, it would be better to deprecate validate >> attribute directly. I have no idea about it, hope to rely on your experience. >> >> In fact, I mainly want to collect your views on whether or how to fix this bug >> here. Thanks! > > > Well validate_slab() is rarely used and should not cause the hot paths to > incur performance penalties. Fix it in the validation logic somehow? Or > document the issue and warn that validation may not be correct if there If available, I think document the issue and warn this incorrect behavior is OK. But it still prints a large amount of confusing messages, and disturbs us? > are current operations on the slab being validated. And I am trying to fix it in following way. In a short, these changes only works under the slub debug mode, and not affects the normal mode (I'm not sure). It looks not elegant enough. And if all approve of this way, I can submit the next version. Anyway, thanks for your time:). -wrw @@ -3304,7 +3300,7 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, { void *prior; - int was_frozen; + int was_frozen, to_take_off = 0; struct slab new; unsigned long counters; struct kmem_cache_node *n = NULL; @@ -3315,14 +3311,23 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, if (kfence_free(head)) return; - if (kmem_cache_debug(s) && - !free_debug_processing(s, slab, head, tail, cnt, addr)) - return; + n = get_node(s, slab_nid(slab)); + if (kmem_cache_debug(s)) { + int ret; - do { - if (unlikely(n)) { + spin_lock_irqsave(&n->list_lock, flags); + ret = free_debug_processing(s, slab, head, tail, cnt, addr); + if (!ret) { spin_unlock_irqrestore(&n->list_lock, flags); - n = NULL; + return; + } + } + + do { + if (unlikely(to_take_off)) { + if (!kmem_cache_debug(s)) + spin_unlock_irqrestore(&n->list_lock, flags); + to_take_off = 0; } prior = slab->freelist; counters = slab->counters; @@ -3343,8 +3348,6 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, new.frozen = 1; } else { /* Needs to be taken off a list */ - - n = get_node(s, slab_nid(slab)); /* * Speculatively acquire the list_lock. * If the cmpxchg does not succeed then we may @@ -3353,8 +3356,10 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, * Otherwise the list_lock will synchronize with * other processors updating the list of slabs. */ - spin_lock_irqsave(&n->list_lock, flags); + if (!kmem_cache_debug(s)) + spin_lock_irqsave(&n->list_lock, flags); + to_take_off = 1; } } @@ -3363,8 +3368,9 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, head, new.counters, "__slab_free")); - if (likely(!n)) { - + if (likely(!to_take_off)) { + if (kmem_cache_debug(s)) + spin_unlock_irqrestore(&n->list_lock, flags); if (likely(was_frozen)) { /* * The list lock was not taken therefore no list >