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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 5D200C4167B for ; Fri, 4 Dec 2020 15:48:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D72C722C9C for ; Fri, 4 Dec 2020 15:48:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D72C722C9C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 433856B0068; Fri, 4 Dec 2020 10:48:19 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 40A0E6B006C; Fri, 4 Dec 2020 10:48:19 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2D2196B0072; Fri, 4 Dec 2020 10:48:19 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0217.hostedemail.com [216.40.44.217]) by kanga.kvack.org (Postfix) with ESMTP id 15D7F6B0068 for ; Fri, 4 Dec 2020 10:48:19 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id BFDC3180AD815 for ; Fri, 4 Dec 2020 15:48:18 +0000 (UTC) X-FDA: 77556031476.08.sink26_2709946273c5 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin08.hostedemail.com (Postfix) with ESMTP id 8FF7B1819E621 for ; Fri, 4 Dec 2020 15:48:18 +0000 (UTC) X-HE-Tag: sink26_2709946273c5 X-Filterd-Recvd-Size: 5399 Received: from mail-qk1-f193.google.com (mail-qk1-f193.google.com [209.85.222.193]) by imf14.hostedemail.com (Postfix) with ESMTP for ; Fri, 4 Dec 2020 15:48:17 +0000 (UTC) Received: by mail-qk1-f193.google.com with SMTP id z188so5776443qke.9 for ; Fri, 04 Dec 2020 07:48:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=nwbnY1/AV2FFPs1NQO0btBf2+S5LCFRr1hUU4wSaSQQ=; b=hCSiWy3HwYppo4VxA8ax1Gw1Vky5CN2uyplFnDzpqV5sUx45gMT5u+D0dKhPJ24uif kIt5r7osKzB/LNSMvClSCCCssvQzyFWpZQv3zDXaOEQ+TAgDUgVWwAG5Oce2rARebo/d vQURlgZ1gSdu9kqPR5StVTkj0ZweBohSOI7GmsAVPmNQA381cNF36vu69Rx274gnYPcT wv6jNjFjq+av4h9y9xgXEd9gJeMEhxlBtk/aK85wocMiN3/bu3/3pGxuv8ngHrADpn9L il4t1Kuv5lvozpqGBjLp80sk0hh4vVAU/asrOM38P16qlOEXKmQrDaSAydzjpZzXaHDo N8Eg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=nwbnY1/AV2FFPs1NQO0btBf2+S5LCFRr1hUU4wSaSQQ=; b=UxjIJXtuZXwbjzMpzf+34Bzvc0MfM7RAISuOF80NTKQiqfppYSSIwhTi9Wq6n4KNx3 NSoQEwyMQ0pLUuEtmiZVVc1gTJ7bDjTEqvRaUZddZTBtPYjvHltxKtY/g9XENeRLx3cw DPFXmSHPJGtZ2kZiwKMdM9mvV0eglVo3aI3fJl3gz7BJ2tqYa6sU7lvq7Ii+WM0LU1PE oM0vnunoVr+GGRc2leWc2IOcM11On65ebOFVoBuC29TxPLZeX3vQvNbftJEY0MasQuWZ oT+0HXoLhpaJNFxQwXiEOAKx+Q3ktGi+5yhaetfr7bR3mOIhv4Dnns2rfM6Bb5ypM91d H57Q== X-Gm-Message-State: AOAM533upTeOnvF8UJFobA/UwzwpwD+biAEvJUSdodPIjrnXEMIzVyw1 4jbxDhqovX3a51qm2Pm8FMRNuA== X-Google-Smtp-Source: ABdhPJxZFIK3nn3G4ETckmaEhVgiyEWH/TFZK8Hci3TIRjQKM3LGbZ/qCwehoOQoWqjgaO7ap1vnWA== X-Received: by 2002:a05:620a:8d9:: with SMTP id z25mr146661qkz.127.1607096897156; Fri, 04 Dec 2020 07:48:17 -0800 (PST) Received: from localhost ([2620:10d:c091:480::1:a180]) by smtp.gmail.com with ESMTPSA id o23sm5340401qkk.84.2020.12.04.07.48.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 04 Dec 2020 07:48:16 -0800 (PST) Date: Fri, 4 Dec 2020 10:46:13 -0500 From: Johannes Weiner To: Muchun Song Cc: mhocko@kernel.org, vdavydov.dev@gmail.com, akpm@linux-foundation.org, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Roman Gushchin Subject: Re: [PATCH v2] mm/memcontrol: make the slab calculation consistent Message-ID: <20201204154613.GA176901@cmpxchg.org> References: <20201203031111.3187-1-songmuchun@bytedance.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201203031111.3187-1-songmuchun@bytedance.com> X-Bogosity: Ham, tests=bogofilter, spamicity=0.000003, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Dec 03, 2020 at 11:11:11AM +0800, Muchun Song wrote: > Although the ratio of the slab is one, we also should read the ratio > from the related memory_stats instead of hard-coding. And the local > variable of size is already the value of slab_unreclaimable. So we > do not need to read again. Simplify the code here. > > Signed-off-by: Muchun Song > Acked-by: Roman Gushchin I agree that ignoring the ratio right now is not very pretty, but size = memcg_page_state(memcg, NR_SLAB_RECLAIMABLE_B) + memcg_page_state(memcg, NR_SLAB_UNRECLAIMABLE_B); seq_buf_printf(&s, "slab %llu\n", size); is way easier to understand and more robust than using idx and idx + 1 and then requiring a series of BUG_ONs to ensure these two items are actually adjacent and in the right order. There is a redundant call to memcg_page_state(), granted, but that function is extremely cheap compared with e.g. seq_buf_printf(). > mm/memcontrol.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) IMO this really just complicates the code with little discernible upside. It's going to be a NAK from me, unfortunately. In retrospect, I think that memory_stats[] table was a mistake. It would probably be easier to implement this using a wrapper for memcg_page_state() that has a big switch() for unit conversion. Something like this: /* Translate stat items to the correct unit for memory.stat output */ static unsigned long memcg_page_state_output(memcg, item) { unsigned long value = memcg_page_state(memcg, item); int unit = PAGE_SIZE; switch (item) { case NR_SLAB_RECLAIMABLE_B: case NR_SLAB_UNRECLAIMABLE_B: case WORKINGSET_REFAULT_ANON: case WORKINGSET_REFAULT_FILE: case WORKINGSET_ACTIVATE_ANON: case WORKINGSET_ACTIVATE_FILE: case WORKINGSET_RESTORE_ANON: case WORKINGSET_RESTORE_FILE: case MEMCG_PERCPU_B: unit = 1; break; case NR_SHMEM_THPS: case NR_FILE_THPS: case NR_ANON_THPS: unit = HPAGE_PMD_SIZE; break; case NR_KERNEL_STACK_KB: unit = 1024; break; } return value * unit; } This would fix the ratio inconsistency, get rid of the awkward mix of static and runtime initialization of the table, is probably about the same amount of code, but simpler and more obvious overall.