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 03CF7E77179 for ; Fri, 6 Dec 2024 10:02:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 879E46B0207; Fri, 6 Dec 2024 05:02:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 827BE6B0208; Fri, 6 Dec 2024 05:02:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 716306B0209; Fri, 6 Dec 2024 05:02:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 545A66B0207 for ; Fri, 6 Dec 2024 05:02:50 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 0FFDB161BAD for ; Fri, 6 Dec 2024 10:02:50 +0000 (UTC) X-FDA: 82864094922.05.BA8909F Received: from dggsgout11.his.huawei.com (dggsgout11.his.huawei.com [45.249.212.51]) by imf15.hostedemail.com (Postfix) with ESMTP id 3D963A000F for ; Fri, 6 Dec 2024 10:02:27 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf15.hostedemail.com: domain of chenridong@huaweicloud.com designates 45.249.212.51 as permitted sender) smtp.mailfrom=chenridong@huaweicloud.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733479360; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oMcGr09rzcjt1HsnObDsuhUfKW1lCQhfyc+wmIPPOcI=; b=IqLnuKsbq3Gu58BvKIa65Mg5CoI1ZW3IQKl2iH7dep75SzlP2cK47HTr1Od6uEm9n9NU5w /PwN1z7km/WjbFg8ThGOwPts4rf8Erjd8+hNjW9IFuedWKiqDRVsyq8eJ2ist5eXhtczqp Kn1JrymnR9tVKkPJEEnT37PE0jJl81c= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733479360; a=rsa-sha256; cv=none; b=yzInSMV8tTkg2F8YR/Ij2v5RsCcjuRcZ/kXKb/8fkxFUOrdBGCF6kx9tUrFZZi18OQOByY VJOYZVo36DPjnMIjIHVnG5VXy8uTOM9q004g+1pg8L5qBTng8QdukYkDV1Zk/yyT+L3AE/ ko8hytbdRWX/xtu5+Y4UranMkGHKRnU= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf15.hostedemail.com: domain of chenridong@huaweicloud.com designates 45.249.212.51 as permitted sender) smtp.mailfrom=chenridong@huaweicloud.com Received: from mail.maildlp.com (unknown [172.19.163.235]) by dggsgout11.his.huawei.com (SkyGuard) with ESMTP id 4Y4RdF1WYHz4f3jkk for ; Fri, 6 Dec 2024 18:02:25 +0800 (CST) Received: from mail02.huawei.com (unknown [10.116.40.252]) by mail.maildlp.com (Postfix) with ESMTP id 453971A058E for ; Fri, 6 Dec 2024 18:02:39 +0800 (CST) Received: from [10.67.109.79] (unknown [10.67.109.79]) by APP3 (Coremail) with SMTP id _Ch0CgAHmMS9y1JnrSHUDg--.2760S2; Fri, 06 Dec 2024 18:02:39 +0800 (CST) Message-ID: Date: Fri, 6 Dec 2024 18:02:37 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [next -v1 3/5] memcg: simplify the mem_cgroup_update_lru_size function To: Hugh Dickins Cc: Yu Zhao , akpm@linux-foundation.org, mhocko@kernel.org, hannes@cmpxchg.org, yosryahmed@google.com, roman.gushchin@linux.dev, shakeel.butt@linux.dev, muchun.song@linux.dev, davidf@vimeo.com, vbabka@suse.cz, linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, chenridong@huawei.com, wangweiyang2@huawei.com References: <20241206013512.2883617-1-chenridong@huaweicloud.com> <20241206013512.2883617-4-chenridong@huaweicloud.com> <897b04c9-dba3-44ae-8113-145ca3457cb3@huaweicloud.com> Content-Language: en-US From: Chen Ridong In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-CM-TRANSID:_Ch0CgAHmMS9y1JnrSHUDg--.2760S2 X-Coremail-Antispam: 1UD129KBjvJXoWxWw18uw17KF48WF1xKrWfKrg_yoWrAw4kpF WUKFyYya1kZrW7A3s2ywsaq3y0kr4fAFWUXr13G34rAw1qvFyxtF4UKrWY9FWDArs3Cw43 tFZ3Wrn7AFs8ZaDanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUvYb4IE77IF4wAFF20E14v26r4j6ryUM7CY07I20VC2zVCF04k2 6cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rwA2F7IY1VAKz4 vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_Xr0_Ar1l84ACjcxK6xIIjxv20xvEc7Cj xVAFwI0_Gr1j6F4UJwA2z4x0Y4vEx4A2jsIE14v26rxl6s0DM28EF7xvwVC2z280aVCY1x 0267AKxVW0oVCq3wAS0I0E0xvYzxvE52x082IY62kv0487Mc02F40EFcxC0VAKzVAqx4xG 6I80ewAv7VC0I7IYx2IY67AKxVWUJVWUGwAv7VC2z280aVAFwI0_Jr0_Gr1lOx8S6xCaFV Cjc4AY6r1j6r4UM4x0Y48IcVAKI48JM4IIrI8v6xkF7I0E8cxan2IY04v7MxkF7I0En4kS 14v26r1q6r43MxAIw28IcxkI7VAKI48JMxC20s026xCaFVCjc4AY6r1j6r4UMI8I3I0E5I 8CrVAFwI0_Jr0_Jr4lx2IqxVCjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AKxVW8ZVWr XwCIc40Y0x0EwIxGrwCI42IY6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI8IcVCY1x 0267AKxVW8JVWxJwCI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAFwI0_ Jr0_Gr1lIxAIcVC2z280aVCY1x0267AKxVW8Jr0_Cr1UYxBIdaVFxhVjvjDU0xZFpf9x07 UAwIDUUUUU= X-CM-SenderInfo: hfkh02xlgr0w46kxt4xhlfz01xgou0bp/ X-Stat-Signature: sr9k4z1ndnbfxscuk9wcu6imi4r67g3g X-Rspamd-Queue-Id: 3D963A000F X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1733479347-543349 X-HE-Meta: U2FsdGVkX1+I5EvK2BfEq7pgLpu4jCkGagXzmCdgd+6Sj/YL9FXwyer478Y6g/q1o5SzGdsaxkW3fvTlhCv/SPm+K76YOwg5Is7A/UYKn+49ABjQVOyOn5/hlVtDwC8EBBOGea8y4E4mJXPzCtCY+vcFXjPSyWAgIj1LZu0Sug4coTRqh2pymp7YihbTnshz4/KCQGeSRaDDnkVfIeT9fMmtPdCzex8bKYLrOvnyFej8pallbsTdl9cdpPjApPncHNXC5bmLKxu9yRuup4+bl/xz2p755f80j3ongQzLhJvxkylaqgQAQ0gAnlO6VCbN7U/3O+qeIzhoY79uadCBYqeuC6ki692EAVHwO58Vw7IewrdH8s9MAfU8dlRaCevEiRc1pKSaqwyTo6wSKNzbsPxkoiOV2qWbkGDIpMtSE2FWCbrdNDSK5d/NayZSH8daiWKSv5o8zIt2uUmjaNyKp0ZeEv3EaAwg3ZXIqQ/ZhRQv1ANLXum/1TRiFpk2saGt1rRniUtBkdy/oxSTwxWfwwu0HFuR+naqaMwroWpD+8GqGEU6kPhARvbmOUAdsYYYZftjtNGAFl/v8eqi1LxAccXHk6kP1G6/9R8tBsAG3EuYkYoMeHqSpMKykJoTexuFnJXan4rh0BXv4RbBcCCe1Qo3wQMAlJw/tjUTqBehHC56rmvaY4c5Qmz7P3bhbuyYM0TQDY3ck+ZQinbiLsdN942djUDkhaDxQvGoN80lcrpf0uZmtMmOgaQwiphruebqe49DOrVO9nEbc35bh5iKc+FWq1Vms4P1FLohyBlyNVQAbQlEoL/+dO1zOusJ98ZIKhi5onAI/7wE50+9f9Rwq1suFR6N+JIcletK3C2oi8IvZ1Z38EK4K9tvv8yGvtk24z0za/2kj6efUhmM/ExqDHki5wakU/viV2Icdz3jUyWiY4EJ5QubjCyi1zQNkhgWRptjp371YFtFruEyGT3 9m+ein6n +6Jen8vwI7cGZJ7rPIKMe7IgvrD56L08i1BmWrDm3fL2pqM2EnkiYQ9gTOR1nMdwl/+FrFYVCBJ0riQ3JeO9RaEBwC/1fDrZ0lTTM7Nrv/llGKnGFK1QvgvMq2oxQPsu2DufNDk7Ea3/dzDa1wVKK0MwmjEOwQ0SfYhCrLU+cyFNU/eV8MvBUQyUjdnbZh3PNyvj9jaKAVbtDdHzYqhOpcIEoL0NZQ5DmlNqYRoAUI4F3x2H1L1TvtN48VWes+7VyPu0sKPYJbO5naagXyEQ4ipjE+g== 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: List-Subscribe: List-Unsubscribe: On 2024/12/6 16:24, Hugh Dickins wrote: > On Fri, 6 Dec 2024, Chen Ridong wrote: >> On 2024/12/6 13:33, Yu Zhao wrote: >>> On Thu, Dec 5, 2024 at 6:45 PM Chen Ridong wrote: >>>> From: Chen Ridong >>>> >>>> In the `mem_cgroup_update_lru_size` function, the `lru_size` should be >>>> updated by adding `nr_pages` regardless of whether `nr_pages` is greater >>>> than 0 or less than 0. To simplify this function, add a check for >>>> `nr_pages` == 0. When `nr_pages` is not equal to 0, perform the same >>>> actions. >>>> >>>> Signed-off-by: Chen Ridong >>> >>> NAK. >>> >>> The commit that added that clearly explains why it was done that way. > > Thanks Yu: I did spot this going by, but was indeed hoping that someone > else would NAK it, with more politeness than I could muster at the time! > >> >> Thank you for your reply. >> >> I have read the commit message for ca707239e8a7 ("mm: update_lru_size >> warn and reset bad lru_size") before sending my patch. However, I did >> not quite understand why we need to deal with the difference between >> 'nr_pages > 0' and 'nr_pages < 0'. >> >> >> The 'lru_zone_size' can only be modified in the >> 'mem_cgroup_update_lru_size' function. Only subtracting 'nr_pages' or >> adding 'nr_pages' in a way that causes an overflow can make the size < 0. >> >> For case 1, subtracting 'nr_pages', we should issue a warning if the >> size goes below 0. For case 2, when adding 'nr_pages' results in an >> overflow, there will be no warning and the size won't be reset to 0 the >> first time it occurs . It might be that a warning will be issued the >> next time 'mem_cgroup_update_lru_size' is called to modify the >> 'lru_zone_size'. However, as the commit message said, "the first >> occurrence is the most informative," and it seems we have missed that >> first occurrence. >> >> As the commit message said: "and then the vast unsigned long size draws >> page reclaim into a loop of repeatedly", I think that a warning should >> be issued and 'lru_zone_size' should be reset whenever 'size < 0' occurs >> for the first time, whether from subtracting or adding 'nr_pages'. > > One thing, not obvious, but important to understand, is that WARN_ONCE() > only issues a warning the first time its condition is found true, but > it returns the true or false of the condition every time. So lru_size > is forced to 0 each time an inconsistency is detected. > Thank you for your explanation. My patch does not change this logic. > (But IIRC VM_WARN_ON_ONCE() does not behave in that useful way; or maybe > I've got that macro name wrong - we have so many slightly differing.) > > Perhaps understanding that will help you to make better sense of the > order of events in this function. > > Another thing to understand: it's called before adding folio to list, > but after removing folio from list: when it can usefully compare whether > the emptiness of the list correctly matches lru_size 0. It cannot do so > when adding if you "simplify" it in the way that you did. > The commit b4536f0c829c ("mm, memcg: fix the active list aging for lowmem requests when memcg is enabled") has removed the emptiness check. What is meaningful is that we can determine whether the size is smaller than 0 before adding `nr_pages`. However, as I mentioned, the `lru_zone_size` can only be modified in the `mem_cgroup_update_lru_size` function, which means that a warning must have been issued if the size was smaller than 0 before adding `nr_pages` when `nr_pages` is greater than 0. In this case, it will not issue a warning again. Perhaps "when it can usefully compare whether the emptiness of the list correctly matches lru_size 0" is not useful now. > You might be focusing too much on the "size < 0" aspect of it, or you > might be worrying more than I did about size growing larger and larger > until it wraps to negative (not likely on 64-bit, unless corrupted).> I hope these remarks help, but you need to think through it again > for yourself. > > Hugh Thank you very much for your patience. Best regards, Ridong > >> >> I am be grateful if you can explain more details, it has confused me for >> a while. Thank you very much. >> >> Best regards, >> Ridong