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 96140C433E0 for ; Tue, 2 Mar 2021 10:01:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6846164F12 for ; Tue, 2 Mar 2021 10:01:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1577764AbhCBJwh (ORCPT ); Tue, 2 Mar 2021 04:52:37 -0500 Received: from mx2.suse.de ([195.135.220.15]:36316 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238007AbhCBJgO (ORCPT ); Tue, 2 Mar 2021 04:36:14 -0500 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: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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