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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 81CB0C28D13 for ; Mon, 22 Aug 2022 15:20:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1EA568D001A; Mon, 22 Aug 2022 11:20:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 198E88D0003; Mon, 22 Aug 2022 11:20:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 061BB8D001A; Mon, 22 Aug 2022 11:20:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id EC4008D0003 for ; Mon, 22 Aug 2022 11:20:03 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id D12D31A0A02 for ; Mon, 22 Aug 2022 15:20:03 +0000 (UTC) X-FDA: 79827589086.12.DA3A6BC Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by imf31.hostedemail.com (Postfix) with ESMTP id 580652005D for ; Mon, 22 Aug 2022 15:20:03 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 5067C2030D; Mon, 22 Aug 2022 15:20:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1661181602; 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=u+unRvwl5RKeXktuSK+JeocNb4ORKTqv8ZYCMY8yQM8=; b=Z5JRAObP6PGpMdquxeMFMidbeOHtWJLVEE9H5TfjY4M/WMd9ZT0jW0WV9Yp7wIT2uHYujK vrtWNCDHB5cxvadCzxPjVH948tgFEUdcfX8l61/jqmxdXQSDRyrYuCK77OhGk7sq3xkBGI VPjgQZhJpqdOtppnpDO8v0bcGYh8wQQ= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 23E3D1332D; Mon, 22 Aug 2022 15:20:02 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id T4jUBaKeA2O6FQAAMHmgww (envelope-from ); Mon, 22 Aug 2022 15:20:02 +0000 Date: Mon, 22 Aug 2022 17:20:01 +0200 From: Michal Hocko To: Shakeel Butt Cc: Johannes Weiner , Roman Gushchin , Muchun Song , Michal =?iso-8859-1?Q?Koutn=FD?= , Eric Dumazet , Soheil Hassas Yeganeh , Feng Tang , Oliver Sang , Andrew Morton , lkp@lists.01.org, Cgroups , Linux MM , netdev , LKML Subject: Re: [PATCH 1/3] mm: page_counter: remove unneeded atomic ops for low/min Message-ID: References: <20220822001737.4120417-1-shakeelb@google.com> <20220822001737.4120417-2-shakeelb@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661181603; a=rsa-sha256; cv=none; b=Y+NMED/mLrvsxi8niF+Oi7I7oxbGOlbdDkuyYfCecmxmRfCh+ecKuDziAGMJb6yzKLnb3d 3P1fh2cH8G6P6N9MTdOr+DvRLM+j1fkscIdo62Xw4k43YthLfMgsOokcbxAXizhn02iU8d 3uikKfQvNf27+yzDW7vFTcnfH626tKM= ARC-Authentication-Results: i=1; imf31.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=Z5JRAObP; spf=pass (imf31.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661181603; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=u+unRvwl5RKeXktuSK+JeocNb4ORKTqv8ZYCMY8yQM8=; b=cr4+fEXWUadFVBXW0fCTy468LI8st5nbJIYAJdpANe5NjdOtH5GkPoczn3OaZgXrVaKig8 qXc3NABFTkCM+Wih6p8x8IN39avL4JjQs66YvfGEsW4iXvdgXg4duvjG1SIPbrwoyA5err ED4EGSkVlTWjyqauzj29m/u8ufQeJng= Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=suse.com header.s=susede1 header.b=Z5JRAObP; spf=pass (imf31.hostedemail.com: domain of mhocko@suse.com designates 195.135.220.29 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 580652005D X-Stat-Signature: 9qc65gz43cebdfy96mnqa8sbdxygfzo9 X-Rspam-User: X-HE-Tag: 1661181603-362647 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 Mon 22-08-22 07:55:58, Shakeel Butt wrote: > On Mon, Aug 22, 2022 at 3:18 AM Michal Hocko wrote: > > > > On Mon 22-08-22 11:55:33, Michal Hocko wrote: > > > On Mon 22-08-22 00:17:35, Shakeel Butt wrote: > > [...] > > > > diff --git a/mm/page_counter.c b/mm/page_counter.c > > > > index eb156ff5d603..47711aa28161 100644 > > > > --- a/mm/page_counter.c > > > > +++ b/mm/page_counter.c > > > > @@ -17,24 +17,23 @@ static void propagate_protected_usage(struct page_counter *c, > > > > unsigned long usage) > > > > { > > > > unsigned long protected, old_protected; > > > > - unsigned long low, min; > > > > long delta; > > > > > > > > if (!c->parent) > > > > return; > > > > > > > > - min = READ_ONCE(c->min); > > > > - if (min || atomic_long_read(&c->min_usage)) { > > > > - protected = min(usage, min); > > > > + protected = min(usage, READ_ONCE(c->min)); > > > > + old_protected = atomic_long_read(&c->min_usage); > > > > + if (protected != old_protected) { > > > > > > I have to cache that code back into brain. It is really subtle thing and > > > it is not really obvious why this is still correct. I will think about > > > that some more but the changelog could help with that a lot. > > > > OK, so the this patch will be most useful when the min > 0 && min < > > usage because then the protection doesn't really change since the last > > call. In other words when the usage grows above the protection and your > > workload benefits from this change because that happens a lot as only a > > part of the workload is protected. Correct? > > Yes, that is correct. I hope the experiment setup is clear now. Maybe it is just me that it took a bit to grasp but maybe we want to save our future selfs from going through that mental process again. So please just be explicit about that in the changelog. It is really the part that workloads excessing the protection will benefit the most that would help to understand this patch. > > Unless I have missed anything this shouldn't break the correctness but I > > still have to think about the proportional distribution of the > > protection because that adds to the complexity here. > > The patch is not changing any semantics. It is just removing an > unnecessary atomic xchg() for a specific scenario (min > 0 && min < > usage). I don't think there will be any change related to proportional > distribution of the protection. Yes, I suspect you are right. I just remembered previous fixes like 503970e42325 ("mm: memcontrol: fix memory.low proportional distribution") which just made me nervous that this is a tricky area. I will have another look tomorrow with a fresh brain and send an ack. -- Michal Hocko SUSE Labs