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=-7.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, 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 ED2AECA90AF for ; Tue, 12 May 2020 17:28:24 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id AE9C9206A5 for ; Tue, 12 May 2020 17:28:24 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="GbmmbJ9P" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AE9C9206A5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 4A8169000D7; Tue, 12 May 2020 13:28:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 45834900036; Tue, 12 May 2020 13:28:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 36E3C9000D7; Tue, 12 May 2020 13:28:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0253.hostedemail.com [216.40.44.253]) by kanga.kvack.org (Postfix) with ESMTP id 1C6B3900036 for ; Tue, 12 May 2020 13:28:24 -0400 (EDT) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id D1E4A8248D52 for ; Tue, 12 May 2020 17:28:23 +0000 (UTC) X-FDA: 76808750886.27.teeth05_4944497c8e70f X-HE-Tag: teeth05_4944497c8e70f X-Filterd-Recvd-Size: 3781 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf15.hostedemail.com (Postfix) with ESMTP for ; Tue, 12 May 2020 17:28:23 +0000 (UTC) Received: from kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com (unknown [163.114.132.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 217A5205ED; Tue, 12 May 2020 17:28:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1589304502; bh=1p6BnnVdA+O7XO+1OEIQ+R7NdNbgDqC3c6zfHthSp6g=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=GbmmbJ9PguPY/A4f4QbueuezRI1+jd98ZVdzclvgkFeb6uc4RlSMcCjD5qOtNdQJb 5vspDSdmmn+dCdTXnGLRfJ+IOfciS3uR3+KBs8wtuTUFKMt3BA3ACws/5DE1Xm3FHc pS5QQ1RuxSAXbSfJl7VRA2b01Ey8EerSWtK5eb94= Date: Tue, 12 May 2020 10:28:19 -0700 From: Jakub Kicinski To: Michal Hocko Cc: akpm@linux-foundation.org, linux-mm@kvack.org, kernel-team@fb.com, tj@kernel.org, hannes@cmpxchg.org, chris@chrisdown.name, cgroups@vger.kernel.org, shakeelb@google.com Subject: Re: [PATCH mm v2 1/3] mm: prepare for swap over-high accounting and penalty calculation Message-ID: <20200512102819.4858a60a@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> In-Reply-To: <20200512070858.GO29153@dhcp22.suse.cz> References: <20200511225516.2431921-1-kuba@kernel.org> <20200511225516.2431921-2-kuba@kernel.org> <20200512070858.GO29153@dhcp22.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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, 12 May 2020 09:08:58 +0200 Michal Hocko wrote: > On Mon 11-05-20 15:55:14, Jakub Kicinski wrote: > > Slice the memory overage calculation logic a little bit so we can > > reuse it to apply a similar penalty to the swap. The logic which > > accesses the memory-specific fields (use and high values) has to > > be taken out of calculate_high_delay(). > > > > Signed-off-by: Jakub Kicinski > > Acked-by: Michal Hocko > > some recommendations below. Thank you! > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 05dcb72314b5..8a9b671c3249 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -2321,41 +2321,48 @@ static void high_work_func(struct work_struct *work) > > #define MEMCG_DELAY_PRECISION_SHIFT 20 > > #define MEMCG_DELAY_SCALING_SHIFT 14 > > > > -/* > > - * Get the number of jiffies that we should penalise a mischievous cgroup which > > - * is exceeding its memory.high by checking both it and its ancestors. > > - */ > > -static unsigned long calculate_high_delay(struct mem_cgroup *memcg, > > - unsigned int nr_pages) > > +static u64 calculate_overage(unsigned long usage, unsigned long high) > > the naming is slightly confusing. I would concider the return value > to be in memory units rather than time because I would read it as > overrage of high. calculate_throttle_penalty would be more clear to me. Hm. The unit is the fraction of high. Here is the code, it's quite hard to read in diff form (I should have used --histogram, sorry): static u64 calculate_overage(unsigned long usage, unsigned long high) { u64 overage; if (usage <= high) return 0; /* * Prevent division by 0 in overage calculation by acting as if * it was a threshold of 1 page */ high = max(high, 1UL); overage = usage - high; overage <<= MEMCG_DELAY_PRECISION_SHIFT; return div64_u64(overage, high); } calculate_throttle_penalty() sounds like it returns time. How about something like calc_overage_frac() ? Or calc_overage_perc()? (abbreviating to "calc" so the caller fits on a line)