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 58FBCC433EF for ; Fri, 11 Feb 2022 20:36:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B838D6B0078; Fri, 11 Feb 2022 15:36:47 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B33406B007B; Fri, 11 Feb 2022 15:36:47 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A21BC6B007D; Fri, 11 Feb 2022 15:36:47 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0063.hostedemail.com [216.40.44.63]) by kanga.kvack.org (Postfix) with ESMTP id 92FC76B0078 for ; Fri, 11 Feb 2022 15:36:47 -0500 (EST) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 4BA8C8249980 for ; Fri, 11 Feb 2022 20:36:47 +0000 (UTC) X-FDA: 79131657654.30.5E549F1 Received: from mail-lj1-f180.google.com (mail-lj1-f180.google.com [209.85.208.180]) by imf21.hostedemail.com (Postfix) with ESMTP id E44C91C0011 for ; Fri, 11 Feb 2022 20:36:46 +0000 (UTC) Received: by mail-lj1-f180.google.com with SMTP id c10so1379426ljr.9 for ; Fri, 11 Feb 2022 12:36:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CCAsjj+/2EPlyUQtiVo8WDkzcxXnQjrZ0PEqMW7wI0o=; b=ppE7RooPi4WL00e0ZLJbFk100+IS02OV5QywrNLcKaCJbEtR4YxukPIudnLp8SeKMe 1NpTHSxW8chgPGf7JOY4Ea15KZrCmewdQuJqeHkqMC3F2oB8XRhcoJdjL6j9/F+7f8WJ uUAPTnA1DepzO0Qt+pe0IQN9d9vuPEWAghF7tyEwP6gcUibW6u7V60dsZ2Om7MgSwh3u hnqtLmgCUXP8KKVYUrxRFdwOov2m9joI3QdDjxk8pE/fUxhg1L3y/BZJCLku2CDajQza 5EbbCjrTyKK1IaSFwD9Kfjiz5aTiqTn7xQlm4sYnrDALmXTu3uE6ySHxWdrZ9XJkKkF2 1Hdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CCAsjj+/2EPlyUQtiVo8WDkzcxXnQjrZ0PEqMW7wI0o=; b=goIcWLahDvaMjzP8YTO9cjZUFdDWfSva/f2Nm5C2XaJgOmTK2dYrRyk/KUIn31+fmx VFD/INOtVBMukqw6yUOUT/Dy2vhYNCg1IVQL0fZP8uyokChRCUARolKR8INuEFYRKwud vMLd62JG+ZWCs0goGkqu38lIcQ/0/TWOWbBuBQrbgAscJKPANzSj35Cvl7hu8WWGAjly ZVMyuzjgqmVNcJKUEUY6PMAziOEhpchbczCiaNWWYmaz+OZZdOGoh2dOO3ZdF3N470aH O4ESbgAHqP3mQzXBtl+G7gGyh9cG4uBtDCzwBUbNhvQVT4Ap+qYeRt4Wjauf47SE0y+k I6SA== X-Gm-Message-State: AOAM5308aYFUa7WBxW5psJhdjCcYc4mlAeGQFo6v0poCHvEG2BzUS4TK XW1Znfm+n+znLEcx32hs45ls44W4nBvKPJJ0HZJI6A== X-Google-Smtp-Source: ABdhPJwVEa48rkJ6G8rmFGKX6ejD7MgqMsPEy2hj6bqYfREd84v21zv7mlJcxID3H7rLs2teXuRiEvkPefLlwn+Djcc= X-Received: by 2002:a2e:b16e:: with SMTP id a14mr1959958ljm.35.1644611804994; Fri, 11 Feb 2022 12:36:44 -0800 (PST) MIME-Version: 1.0 References: <20220211064917.2028469-1-shakeelb@google.com> <20220211064917.2028469-5-shakeelb@google.com> In-Reply-To: From: Shakeel Butt Date: Fri, 11 Feb 2022 12:36:33 -0800 Message-ID: Subject: Re: [PATCH v2 4/4] memcg: synchronously enforce memory.high for large overcharges To: Chris Down Cc: Johannes Weiner , Michal Hocko , Roman Gushchin , Andrew Morton , Cgroups , Linux MM , LKML Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: E44C91C0011 X-Stat-Signature: 8j5eemddcu6edoqj74n737ii76e8fpmt Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ppE7RooP; spf=pass (imf21.hostedemail.com: domain of shakeelb@google.com designates 209.85.208.180 as permitted sender) smtp.mailfrom=shakeelb@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspam-User: X-HE-Tag: 1644611806-621354 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 Fri, Feb 11, 2022 at 4:13 AM Chris Down wrote: > [...] > >To make high limit enforcement more robust, this patch makes the limit > >enforcement synchronous only if the accumulated overcharge becomes > >larger than MEMCG_CHARGE_BATCH. So, most of the allocations would still > >be throttled on the return-to-userspace path but only the extreme > >allocations which accumulates large amount of overcharge without > >returning to the userspace will be throttled synchronously. The value > >MEMCG_CHARGE_BATCH is a bit arbitrary but most of other places in the > >memcg codebase uses this constant therefore for now uses the same one. > > Note that mem_cgroup_handle_over_high() has its own allocator throttling grace > period, where it bails out if the penalty to apply is less than 10ms. The > reclaim will still happen, though. So throttling might not happen even for > roughly MEMCG_CHARGE_BATCH-sized allocations, depending on the overall size of > the cgroup and its protection. > Here by throttling, I meant both reclaim and schedule_timeout_killable(). I don't want to say low level details which might change in future. [...] > > Thanks, I was going to comment on v1 that I prefer to keep the implementation > of mem_cgroup_handle_over_high if possible since we know that the mechanism has > been safe in production over the past few years. > > One question I have is about throttling. It looks like this new > mem_cgroup_handle_over_high callsite may mean that throttling is invoked more > than once on a misbehaving workload that's failing to reclaim since the > throttling could be invoked both here and in return to userspace, right? That > might not be a problem, but we should think about the implications of that, > especially in relation to MEMCG_MAX_HIGH_DELAY_JIFFIES. > Please note that mem_cgroup_handle_over_high() clears memcg_nr_pages_over_high and if on the return-to-userspace path mem_cgroup_handle_over_high() finds that memcg_nr_pages_over_high is non-zero, then it means the task has further accumulated the charges over high limit after a possibly synchronous memcg_nr_pages_over_high() call. > Maybe we should record if throttling happened previously and avoid doing it > again for this entry into kernelspace? Not certain that's the right answer, but > we should think about what the new semantics should be. For now, I will keep this as is and will add a comment in the code and a mention in the commit message about it. I will wait for others to comment before sending the next version and thanks for taking a look.