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 X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 103ACC2D0A8 for ; Mon, 28 Sep 2020 14:09:28 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 261F821941 for ; Mon, 28 Sep 2020 14:09:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 261F821941 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6BE166B0062; Mon, 28 Sep 2020 10:09:26 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 647738E0001; Mon, 28 Sep 2020 10:09:26 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 536216B006C; Mon, 28 Sep 2020 10:09:26 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0101.hostedemail.com [216.40.44.101]) by kanga.kvack.org (Postfix) with ESMTP id 392336B0062 for ; Mon, 28 Sep 2020 10:09:26 -0400 (EDT) Received: from smtpin06.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id E17988249980 for ; Mon, 28 Sep 2020 14:09:25 +0000 (UTC) X-FDA: 77312652690.06.paint81_0a17b5227182 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin06.hostedemail.com (Postfix) with ESMTP id 4FD8B10208115 for ; Mon, 28 Sep 2020 14:08:46 +0000 (UTC) X-HE-Tag: paint81_0a17b5227182 X-Filterd-Recvd-Size: 6052 Received: from huawei.com (szxga08-in.huawei.com [45.249.212.255]) by imf35.hostedemail.com (Postfix) with ESMTP for ; Mon, 28 Sep 2020 14:08:45 +0000 (UTC) Received: from DGGEML403-HUB.china.huawei.com (unknown [172.30.72.54]) by Forcepoint Email with ESMTP id DE89442FCA1814698BE7; Mon, 28 Sep 2020 22:08:36 +0800 (CST) Received: from DGGEML529-MBS.china.huawei.com ([169.254.5.156]) by DGGEML403-HUB.china.huawei.com ([fe80::74d9:c659:fbec:21fa%31]) with mapi id 14.03.0487.000; Mon, 28 Sep 2020 22:08:29 +0800 From: "chenjun (AM)" To: Catalin Marinas CC: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "akpm@linux-foundation.org" , "Xiangrui (Euler)" , "weiyongjun (A)" Subject: Re: [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect Thread-Topic: [PATCH -next 3/5] mm/kmemleak: Add support for percpu memory leak detect Thread-Index: AQHWkMbbj+H4aShTD0uutUryxnjBNQ== Date: Mon, 28 Sep 2020 14:08:29 +0000 Message-ID: References: <20200921020007.35803-1-chenjun102@huawei.com> <20200921020007.35803-4-chenjun102@huawei.com> <20200922095736.GB15643@gaia> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.174.179.194] Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CFilter-Loop: Reflected 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: Hi Catalin=0A= =0A= Thanks for your opinions.=0A= =0A= =1B$B:_=1B(B 2020/9/22 17:58, Catalin Marinas =1B$B On Mon, Sep 21, 2020 at 02:00:05AM +0000, Chen Jun wrote:=0A= >> From: Wei Yongjun =0A= >>=0A= >> Currently the reporting of the percpu chunks leaking problem=0A= >> are not supported. This patch introduces this function.=0A= >>=0A= >> Since __percpu pointer is not pointing directly to the actual chunks,=0A= >> this patch creates an object for __percpu pointer, but marks it as no=0A= >> scan block, only check whether this pointer is referenced by other=0A= >> blocks.=0A= > =0A= > OK, so you wanted NO_SCAN to not touch the block at all, not even update= =0A= > the checksum. Maybe better add a new flag, NO_ACCESS (and we could use=0A= > it to track ioremap leaks, it's been on my wishlist for years).=0A= >=0A= =0A= I will add a new OBJECT_NO_ACCESS.=0A= The checksum of the object will not be updated and its memory block will = =0A= not be scanned if the object marked with OBJECT_NO_ACCESS.=0A= =0A= >> diff --git a/mm/kmemleak.c b/mm/kmemleak.c=0A= >> index c09c6b59eda6..feedb72f06f2 100644=0A= >> --- a/mm/kmemleak.c=0A= >> +++ b/mm/kmemleak.c=0A= >> @@ -283,6 +288,9 @@ static void hex_dump_object(struct seq_file *seq,=0A= >> const u8 *ptr =3D (const u8 *)object->pointer;=0A= >> size_t len;=0A= >> =0A= >> + if (object->flags & OBJECT_PERCPU)=0A= >> + ptr =3D this_cpu_ptr((void __percpu *)object->pointer);=0A= > =0A= > You may want to print the CPU number as well since the information is=0A= > likely different on another CPU. Also, I think this context is=0A= > preemptable, so it's better with a get_cpu/put_cpu().=0A= > =0A= =0A= I will print cpu number when dump the percpu object.=0A= =0A= >> @@ -651,6 +672,19 @@ static void create_object(unsigned long ptr, size_t= size, int min_count,=0A= >> raw_spin_unlock_irqrestore(&kmemleak_lock, flags);=0A= >> }=0A= >> =0A= >> +static void create_object(unsigned long ptr, size_t size, int min_count= ,=0A= >> + gfp_t gfp)=0A= >> +{=0A= >> + __create_object(ptr, size, min_count, 0, gfp);=0A= >> +}=0A= >> +=0A= >> +static void create_object_percpu(unsigned long ptr, size_t size, int mi= n_count,=0A= >> + gfp_t gfp)=0A= >> +{=0A= >> + __create_object(ptr, size, min_count, OBJECT_PERCPU | OBJECT_NO_SCAN,= =0A= >> + gfp);=0A= >> +}=0A= >> +=0A= >> /*=0A= >> * Mark the object as not allocated and schedule RCU freeing via put_o= bject().=0A= >> */=0A= >> @@ -912,10 +946,12 @@ void __ref kmemleak_alloc_percpu(const void __perc= pu *ptr, size_t size,=0A= >> * Percpu allocations are only scanned and not reported as leaks=0A= >> * (min_count is set to 0).=0A= >> */=0A= >> - if (kmemleak_enabled && ptr && !IS_ERR(ptr))=0A= >> + if (kmemleak_enabled && ptr && !IS_ERR(ptr)) {=0A= >> for_each_possible_cpu(cpu)=0A= >> create_object((unsigned long)per_cpu_ptr(ptr, cpu),=0A= >> size, 0, gfp);=0A= >> + create_object_percpu((unsigned long)ptr, size, 1, gfp);=0A= >> + }=0A= >> }=0A= > =0A= > A concern I have here is that ptr may overlap with an existing object=0A= > and the insertion in the rb tree will fail. For example, with !SMP,=0A= > ptr =3D=3D per_cpu_ptr(ptr, 0), so create_object() will fail and kmemleak= =0A= > gets disabled.=0A= > =0A= > An option would to figure out how to allow overlapping ranges with rb=0A= > tree (or find a replacement for it if not possible).=0A= > =0A= > Another option would be to have an additional structure to track the=0A= > __percpu pointers since they have their own range. If size is not=0A= > relevant, maybe go for an xarray, otherwise another rb tree (do we have= =0A= > any instance of pointers referring some inner member of a __percpu=0A= > object?). The scan_object() function will have to search two trees.=0A= > =0A= =0A= I would like to use CONFIG_SMP to seprate code:=0A= if SMP, we will create some objects for per_cpu_ptr(ptr, cpu) and an =0A= object with OBJECT_NO_ACCESS for ptr.=0A= if !SMP, we will not create object for per_cpu_ptr(ptr,cpu), but an =0A= object without OBJECT_NO_ACCESS for ptr will be created.=0A= What do you think about this opinion.=0A= =0A= Waiting for your reply=0A= =0A= Best wishes=0A= Jun=0A= =0A=