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=-5.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,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 D5449C433DB for ; Tue, 2 Mar 2021 09:34:13 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6957E60200 for ; Tue, 2 Mar 2021 09:34:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6957E60200 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id DD9678D00F2; Tue, 2 Mar 2021 04:34:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D634F8D0063; Tue, 2 Mar 2021 04:34:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C2A5C8D00F2; Tue, 2 Mar 2021 04:34:12 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0032.hostedemail.com [216.40.44.32]) by kanga.kvack.org (Postfix) with ESMTP id A9C928D0063 for ; Tue, 2 Mar 2021 04:34:12 -0500 (EST) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 702CE8249980 for ; Tue, 2 Mar 2021 09:34:12 +0000 (UTC) X-FDA: 77874423144.13.B1EC8B4 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf30.hostedemail.com (Postfix) with ESMTP id 3952FE0011E6 for ; Tue, 2 Mar 2021 09:34:11 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1614677650; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=cSTLa9hZjScF2nB/fdohi0qlgKjoTXLy1AfmxxhJlBE=; b=Eh5uhBdOdjB3Tu9kATn08Pfe+yZ7BbNU6zC0pyU7JryhFYPGBCSuVjOp7QnEv7a/6Tv1mt /2ZKuLEZuGPgwKP/GwytapTUVpIjbVRvmtoEbGGTVM5d+Z1wXNL0AfENyFyNApIVZ5x0dJ Zp8SYDoZIpCuuehjzbODzi9pc7UXV7Q= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 9F880AAC5; Tue, 2 Mar 2021 09:34:10 +0000 (UTC) Date: Tue, 2 Mar 2021 10:34:09 +0100 From: Michal Hocko To: Muchun Song Cc: Roman Gushchin , Johannes Weiner , Andrew Morton , Shakeel Butt , LKML , Linux Memory Management List Subject: Re: [External] Re: [PATCH] mm: memcontrol: fix kernel stack account Message-ID: References: <20210302073733.8928-1-songmuchun@bytedance.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 3952FE0011E6 X-Stat-Signature: e14oh7h63ad3i5fnpocykidjumhfu3on Received-SPF: none (suse.com>: No applicable sender policy available) receiver=imf30; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1614677651-331518 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 Tue 02-03-21 17:23:42, Muchun Song wrote: > On Tue, Mar 2, 2021 at 4:44 PM Michal Hocko wrote: > > > > On Tue 02-03-21 15:37:33, Muchun Song wrote: > > > The alloc_thread_stack_node() cannot guarantee that allocated stack pages > > > are in the same node when CONFIG_VMAP_STACK. Because we do not specify > > > __GFP_THISNODE to __vmalloc_node_range(). Fix it by caling > > > mod_lruvec_page_state() for each page one by one. > > > > What is the actual problem you are trying to address by this patch? > > 991e7673859e has deliberately dropped the per page accounting. Can you > > explain why that was incorrect? There surely is some imprecision > > involved but does it matter and is it even observable? > > When I read the code of account_kernel_stack(), I see a comment that > says "All stack pages are in the same node". I am confused about this. > IIUC, there is no guarantee about this. Right? Yes there is no guarantee indeed. Please always make sure to describe the underlying reasoning for the patch. Subject of this patch refers to a fix without explaining the actual problem. If a change is motivated by code reading then make it explicit. Also if you are refering to a different commit by Fixes: tag then it would be really helpful to explicitly mention why that commit is incorrect or cause a visible problems. > Yeah, imprecision may > not be a problem. But if this is what we did deliberately, I think that > it is better to add a comment there. Thanks. Yes the comment is quite confusing. I suspect it meant to say /* All stack pages are accounted to the same node */ -- Michal Hocko SUSE Labs