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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6ECA7C83F16 for ; Mon, 28 Aug 2023 17:10:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231236AbjH1RJm (ORCPT ); Mon, 28 Aug 2023 13:09:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50034 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232858AbjH1RJd (ORCPT ); Mon, 28 Aug 2023 13:09:33 -0400 Received: from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com [IPv6:2a00:1450:4864:20::52e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 61474CE5 for ; Mon, 28 Aug 2023 10:08:59 -0700 (PDT) Received: by mail-ed1-x52e.google.com with SMTP id 4fb4d7f45d1cf-52683b68c2fso4673546a12.0 for ; Mon, 28 Aug 2023 10:08:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1693242505; x=1693847305; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=CgmSxEVlzi5fMkwnrM8LhIHtQEbm/jAUKGlrVKGkYss=; b=iFiaXdvNyP/PGupmYZQ4VDhihu4roD+VOTAYUOk648A8zr/NnetU0xeTBrkoG+AsPR 2wxI9+4xAQyl3Xxl94EeAyF+UIHsPMJF7wGK0pCn3gzC4W2F4540PibPlUl5Zn6MTaR2 o9vymucc9ynm4RNh82v145GN+bLsUwc0gP3U/tWXkGAjscoX/fm2tFx7pnKGjTA5r+bw 1SR9g8zkzT/uibLlIedBK0SvUKoCFVbB3i1kaLcB1K4BYwKWqBKKzLSRxsgdcTYodIoi FusqkfeL0/iUTIn/J+n7J3p1yNB/tRDgBAeMhHmPuu5uQZq3RL4xZ0uTD5pWqvRs5dNW PSjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693242505; x=1693847305; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=CgmSxEVlzi5fMkwnrM8LhIHtQEbm/jAUKGlrVKGkYss=; b=hqRLYZPjjcpcGT19hBBBfyk3qXjk9lYHcHGGvosMf/DqNzvqwPy+Mq5K+3HThx11x3 SZJEo8FUe86p5XZLZKDYm2S2JRpgwCDvVIPzopxOaV7QbIctiXygUO/eYRsTFy5eQB1a aTmAaQHy/mK4s5S3u4VWI2eR7cbBCBDDYC9KUjDOdPkesFmCbiritpOMysAGYn6qt36P Ci95j+4e9ZPYpsV7lhe3jG72yYXlwBN4IKHXewITRcSvpvi74bpIWgTTjAqnEdk2ZdzT 5spy1qblssVYW/bZdcLT1WAoSkn6ia7qn9cSKW9oOpUvjHEdsU12G7b/qTb3duceFkhb UwOw== X-Gm-Message-State: AOJu0Yz1qGRJobBxhDDAn9BNnBMUVBLZqPaQmFbb+9XiJIWVXQYN4AGK /2o7D71esxV7l0w030i6AGi2dRoenG3o6BuGu+Q0Mw== X-Google-Smtp-Source: AGHT+IEebaOPHNuzP7D5Utx3nzTPX+fvnhl+LnF5mzhFALZEervRMpT4j2HBe8/3OWEpuNkUYvYhIzytsgarWkOfJOw= X-Received: by 2002:a17:906:2092:b0:9a5:852f:10ac with SMTP id 18-20020a170906209200b009a5852f10acmr5947834ejq.53.1693242505112; Mon, 28 Aug 2023 10:08:25 -0700 (PDT) MIME-Version: 1.0 References: <20230821205458.1764662-4-yosryahmed@google.com> In-Reply-To: From: Yosry Ahmed Date: Mon, 28 Aug 2023 10:07:48 -0700 Message-ID: Subject: Re: [PATCH 3/3] mm: memcg: use non-unified stats flushing for userspace reads To: Shakeel Butt Cc: Michal Hocko , Andrew Morton , Johannes Weiner , Roman Gushchin , Muchun Song , Ivan Babrou , Tejun Heo , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Aug 28, 2023 at 10:00=E2=80=AFAM Shakeel Butt = wrote: > > On Mon, Aug 28, 2023 at 9:15=E2=80=AFAM Yosry Ahmed wrote: > > > [...] > > > > > > Well, I really have to say that I do not like the notion that reading > > > stats is unpredictable. This just makes it really hard to use. If > > > the precision is to be sarificed then this should be preferable over > > > potentially high global lock contention. We already have that model i= n > > > place of /proc/vmstat (configurable timeout for flusher and a way to > > > flush explicitly). I appreciate you would like to have a better > > > precision but as you have explored the locking is really hard to get = rid > > > of here. > > > > Reading the stats *is* unpredictable today. In terms of > > accuracy/staleness and cost. Avoiding the flush entirely on the read > > path will surely make the cost very stable and cheap, but will make > > accuracy even less predictable. > > > > On average you would get the stats at most 2 second old, so I would > not say it is less predictable. > > > > > > > So from my POV I would prefer to avoid flushing from the stats readin= g > > > path and implement force flushing by writing to stat file. If the 2s > > > flushing interval is considered to coarse I would be OK to allow sett= ing > > > it from userspace. This way this would be more in line with /proc/vms= tat > > > which seems to be working quite well. > > > > > > If this is not accaptable or deemed a wrong approach long term then i= t > > > would be good to reonsider the current cgroup_rstat_lock at least. > > > Either by turning it into mutex or by dropping the yielding code whic= h > > > can severly affect the worst case latency AFAIU. > > > > Honestly I think it's better if we do it the other way around. We make > > flushing on the stats reading path non-unified and deterministic. That > > model also exists and is used for cpu.stat. If we find a problem with > > the locking being held from userspace, we can then remove flushing > > from the read path and add interface(s) to configure the periodic > > flusher and do a force flush. > > > > Here I agree with you. Let's go with the approach which is easy to > undo for now. Though I prefer the new explicit interface for flushing, > that step would be very hard to undo. Let's reevaluate if the proposed > approach shows negative impact on production traffic and I think > Cloudflare folks can give us the results soon. Do you prefer we also switch to using a mutex (with preemption disabled) to avoid the scenario Michal described where flushers give up the lock and sleep resulting in an unbounded wait time in the worst case?