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 20618C43334 for ; Tue, 14 Jun 2022 18:28:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 87D336B0071; Tue, 14 Jun 2022 14:28:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 805556B0072; Tue, 14 Jun 2022 14:28:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 659A96B0073; Tue, 14 Jun 2022 14:28:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 54D886B0071 for ; Tue, 14 Jun 2022 14:28:57 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay13.hostedemail.com (Postfix) with ESMTP id 31B0760DEA for ; Tue, 14 Jun 2022 18:28:57 +0000 (UTC) X-FDA: 79577677914.02.84B67B5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf31.hostedemail.com (Postfix) with ESMTP id BA0D8200CB for ; Tue, 14 Jun 2022 18:28:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655231336; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jzGDnMjiVMPl8LoDK9/jjZ3s6cfGJvhDYM+qlDxnnck=; b=gIVxn5SgB/mQfyZfKrwzY1dPkUtHzch9i+xEkUOhHGwLecAdAryogeAMp/1ZxMxPjH+pBn 2ImB6SnEQowDl28QxOj/WUmLkhbOQTtlor8KZUIWwUWCFfnttoApIfkVaC62xl4CQEIwOU yjp+InHAJAACJD6xKbGYVWzJG9vNc6g= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-1-ZFBhL40jPHCo3tCVEsDiJg-1; Tue, 14 Jun 2022 14:28:54 -0400 X-MC-Unique: ZFBhL40jPHCo3tCVEsDiJg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6A9E6101AA45; Tue, 14 Jun 2022 18:28:54 +0000 (UTC) Received: from [10.22.33.116] (unknown [10.22.33.116]) by smtp.corp.redhat.com (Postfix) with ESMTP id 330832026D64; Tue, 14 Jun 2022 18:28:54 +0000 (UTC) Message-ID: Date: Tue, 14 Jun 2022 14:28:53 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH 3/3] mm/kmemleak: Prevent soft lockup in first object iteration loop of kmemleak_scan() Content-Language: en-US From: Waiman Long To: Catalin Marinas Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20220612183301.981616-1-longman@redhat.com> <20220612183301.981616-4-longman@redhat.com> <325768cd-19bd-71ae-83d6-1ca5e84f7462@redhat.com> In-Reply-To: <325768cd-19bd-71ae-83d6-1ca5e84f7462@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655231336; a=rsa-sha256; cv=none; b=RwAPvHqnmOxgvftlwkqAo8Mzy+pf/gHlYnDm+jpJIFurx1Gy4pCNFiz2fG501QdY3S1bjk YWrwwAImW67LzSS55zENe1cc65P8vUowTicVuij7nppNbxTwfrO4/SztMGdUo6NOFz5qeo SkxjMcleUohIaNGOh3dil15sak9hEUc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655231336; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=jzGDnMjiVMPl8LoDK9/jjZ3s6cfGJvhDYM+qlDxnnck=; b=dRxShwZXr814dRCcJdyyTv+Q68Xv5e2QhzYFJNxHRQO0hyTYUPUSxP/rXJEzJfp5LjW9rW 6p6K3IJqd2AJnmklC1yZmua96kJ1/DxZyjlBma/WxLemGk5HtIgTycY/b4NljRIMqug4Sr hniC1glG/7ZHF21KsmSlZgyOGUyCtMU= ARC-Authentication-Results: i=1; imf31.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=gIVxn5Sg; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf31.hostedemail.com: domain of longman@redhat.com has no SPF policy when checking 170.10.129.124) smtp.mailfrom=longman@redhat.com Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=gIVxn5Sg; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf31.hostedemail.com: domain of longman@redhat.com has no SPF policy when checking 170.10.129.124) smtp.mailfrom=longman@redhat.com X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: BA0D8200CB X-Stat-Signature: esku93eisobn9xgofkt76iem8bttxgoo X-HE-Tag: 1655231336-697043 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/14/22 14:22, Waiman Long wrote: > On 6/14/22 13:27, Catalin Marinas wrote: > >>>> raw_spin_unlock_irq(&object->lock); >>>> + >>>> +        /* >>>> +         * With object pinned by a positive reference count, it >>>> +         * won't go away and we can safely release the RCU read >>>> +         * lock and do a cond_resched() to avoid soft lockup every >>>> +         * 64k objects. >>>> +         */ >>>> +        if (object_pinned && !(gray_list_cnt & 0xffff)) { >>>> +            rcu_read_unlock(); >>>> +            cond_resched(); >>>> +            rcu_read_lock(); >>>> +        } >>> I'm not sure this gains much. There should be very few gray objects >>> initially (those passed to kmemleak_not_leak() for example). The >>> majority should be white objects. >>> >>> If we drop the fine-grained object->lock, we could instead take >>> kmemleak_lock outside the loop with a cond_resched_lock(&kmemleak_lock) >>> within the loop. I think we can get away with not having an >>> rcu_read_lock() at all for list traversal with the big lock outside the >>> loop. >> Actually this doesn't work is the current object in the iteration is >> freed. Does list_for_each_rcu_safe() help? > > list_for_each_rcu_safe() helps if we are worrying about object being > freed. However, it won't help if object->next is freed instead. > > How about something like: > > diff --git a/mm/kmemleak.c b/mm/kmemleak.c > index 7dd64139a7c7..fd836e43cb16 100644 > --- a/mm/kmemleak.c > +++ b/mm/kmemleak.c > @@ -1417,12 +1417,16 @@ static void kmemleak_scan(void) >         struct zone *zone; >         int __maybe_unused i; >         int new_leaks = 0; > +       int loop1_cnt = 0; > >         jiffies_last_scan = jiffies; > >         /* prepare the kmemleak_object's */ >         rcu_read_lock(); >         list_for_each_entry_rcu(object, &object_list, object_list) { > +               bool obj_pinned = false; > + > +               loop1_cnt++; >                 raw_spin_lock_irq(&object->lock); >  #ifdef DEBUG >                 /* > @@ -1437,10 +1441,32 @@ static void kmemleak_scan(void) >  #endif >                 /* reset the reference count (whiten the object) */ >                 object->count = 0; > -               if (color_gray(object) && get_object(object)) > +               if (color_gray(object) && get_object(object)) { >                         list_add_tail(&object->gray_list, &gray_list); > +                       obj_pinned = true; > +               } > >                 raw_spin_unlock_irq(&object->lock); > + > +               /* > +                * Do a cond_resched() to avoid soft lockup every 64k > objects. > +                * Make sure a reference has been taken so that the > object > +                * won't go away without RCU read lock. > +                */ > +               if (loop1_cnt & 0xffff) { Sorry, should be "(!(loop1_cnt & 0xffff))". > + if (!obj_pinned && !get_object(object)) { > +                               /* Try the next object instead */ > +                               loop1_cnt--; > +                               continue; > +                       } > + > +                       rcu_read_unlock(); > +                       cond_resched(); > +                       rcu_read_lock(); > + > +                       if (!obj_pinned) > +                               put_object(object); > +               } >         } >         rcu_read_unlock(); > Cheers, Longman