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 AFEFDC83F12 for ; Mon, 28 Aug 2023 15:48:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232401AbjH1Pru (ORCPT ); Mon, 28 Aug 2023 11:47:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40738 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230491AbjH1PrS (ORCPT ); Mon, 28 Aug 2023 11:47:18 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F05379D; Mon, 28 Aug 2023 08:47:15 -0700 (PDT) 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-out1.suse.de (Postfix) with ESMTPS id A4FCE21A89; Mon, 28 Aug 2023 15:47:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1693237634; 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=wgffJfTbN2hrM2LfawRv94KbFXBSWdoYhh+f+YXVzXA=; b=j9QKq8c9xmscDVZf6LzoNgCsUfGNvGMoqZlKWzlvV1liR+juWKDkG/lLU4xNc5qBs/MW+x uyeKOP+bAwmg6pleKvqb1/El8aRPFEFsnNGdwa/rM1xNFzsehyP9P939blDXfum1VRnruS +NjieNHQc211STH1b2HQt6KFCWRUK/Q= 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 827D713A11; Mon, 28 Aug 2023 15:47:14 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id bwb/HILB7GQ0DgAAMHmgww (envelope-from ); Mon, 28 Aug 2023 15:47:14 +0000 Date: Mon, 28 Aug 2023 17:47:13 +0200 From: Michal Hocko To: Yosry Ahmed Cc: Andrew Morton , Johannes Weiner , Roman Gushchin , Shakeel Butt , Muchun Song , Ivan Babrou , Tejun Heo , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] mm: memcg: use non-unified stats flushing for userspace reads Message-ID: References: <20230821205458.1764662-4-yosryahmed@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Done my homework and studied the rstat code more (sorry should have done that earlier). On Fri 25-08-23 08:14:54, Yosry Ahmed wrote: [...] > I guess what I am trying to say is, breaking down that lock is a major > surgery that might require re-designing or re-implementing some parts > of rstat. I would be extremely happy to be proven wrong. If we can > break down that lock then there is no need for unified flushing even > for in-kernel contexts, and we can all live happily ever after with > cheap(ish) and accurate stats flushing. Yes, this seems like a big change and also over complicating the whole thing. I am not sure this is worth it. > I really hope we can move forward with the problems at hand (sometimes > reads are expensive, sometimes reads are stale), and not block fixing > them until we can come up with an alternative to that global lock > (unless, of course, there is a simpler way of doing that). 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 in 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. So from my POV I would prefer to avoid flushing from the stats reading 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 setting it from userspace. This way this would be more in line with /proc/vmstat which seems to be working quite well. If this is not accaptable or deemed a wrong approach long term then it would be good to reonsider the current cgroup_rstat_lock at least. Either by turning it into mutex or by dropping the yielding code which can severly affect the worst case latency AFAIU. > Sorry for the very long reply :) Thanks for bearing with me and taking time to formulate all this! -- Michal Hocko SUSE Labs