From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from lindbergh.monkeyblade.net (lindbergh.monkeyblade.net [23.128.96.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 50E0B1D6A8; Thu, 9 Nov 2023 14:58:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=tycho.pizza header.i=@tycho.pizza header.b="tRxVX7Yc"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="EXVyeR8h" Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A678C325B; Thu, 9 Nov 2023 06:58:26 -0800 (PST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.48]) by mailout.west.internal (Postfix) with ESMTP id 9FB163200B77; Thu, 9 Nov 2023 09:58:23 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Thu, 09 Nov 2023 09:58:24 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tycho.pizza; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm2; t=1699541903; x=1699628303; bh=jW dS6GkJOycSu70WYEYSDlUZAEtt7yUOuuE+eluEIcE=; b=tRxVX7Ycq9mg0RkTk5 68Lnt1Ue7Ljxgl1DKoOBsOMC6Yq3BBuOYVqAG6idR7uwYlBscvlKFPgDc+N8tt/D Qa2f8D8juC+Pw8+NU7Wu4uNcRh697+bNLuKcmY0joqbzra/wDOwLWDVl/lXsWqak TbmD3DsIenmyUMJZe4nKo7bNFUKkuOGPWJmkZh10zhxQqUIKVslQONA7uSd8Hof6 5XWFJA/4k6ETnqVP6tn8NydNQX6TWMTBs9ejDTD8iV80WsHUTmhwW2z8l8C8q6uq bCZIJ/mAZt7v6eVxAttekDeW3/4VTL3S/PWydNUq3RnJ1ZtIYFxaBuufdG5f8wVk 4ibA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; t=1699541903; x=1699628303; bh=jWdS6GkJOycSu 70WYEYSDlUZAEtt7yUOuuE+eluEIcE=; b=EXVyeR8hvIv06aYAHOjdORgUpspXJ ZwaTo7Xo0uJcHAjOidX8/U5AHdxK9VCa6+6ugr+0hUfA1KXG6Q52q3QPYs6JJVgX iPOjARSs9a/NZiNGuHm/4A/bl1mT6RcGaUoe8jQH7iBNtBGs1VmMvv/D+R0hOLn1 NPc4r4qoC9xUzvm/g/CmMX/HzA9Gmbs4SLIrVOMlWd+ACR3VudinRttPmwYXL/WD eydMs4cvyR4txcC0AFIFBrIoFcP2QPqQHU+FCraJcCvturcjfGapds0IHL0cG2J1 /ahbx/S5CSKRUJfWxmPg78SHMbFLLI541CIVKJtTKg0zUhnuFUfh1kLCA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedruddvuddgieelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvdenucfhrhhomhepvfihtghh ohcutehnuggvrhhsvghnuceothihtghhohesthihtghhohdrphhiiiiirgeqnecuggftrf grthhtvghrnhepueettdetgfejfeffheffffekjeeuveeifeduleegjedutdefffetkeel hfelleetnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomh epthihtghhohesthihtghhohdrphhiiiiirg X-ME-Proxy: Feedback-ID: i21f147d5:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 9 Nov 2023 09:58:21 -0500 (EST) Date: Thu, 9 Nov 2023 07:58:19 -0700 From: Tycho Andersen To: Christian Brauner Cc: cgroups@vger.kernel.org, linux-fsdevel@vger.kernel.org, Tejun Heo , Zefan Li , Johannes Weiner , Haitao Huang , Kamalesh Babulal , Tycho Andersen Subject: Re: [RFC 4/6] misc cgroup: introduce an fd counter Message-ID: References: <20231108002647.73784-1-tycho@tycho.pizza> <20231108002647.73784-5-tycho@tycho.pizza> <20231108-ernst-produktiv-f0f5d2ceeade@brauner> Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231108-ernst-produktiv-f0f5d2ceeade@brauner> On Thu, Nov 09, 2023 at 10:53:17AM +0100, Christian Brauner wrote: > > @@ -411,9 +453,22 @@ struct files_struct *dup_fd(struct files_struct *oldf, unsigned int max_fds, int > > > > rcu_assign_pointer(newf->fdt, new_fdt); > > > > - return newf; > > + if (!charge_current_fds(newf, count_open_files(new_fdt))) > > + return newf; > > > > @@ -542,6 +600,10 @@ static int alloc_fd(unsigned start, unsigned end, unsigned flags) > > if (error) > > goto repeat; > > > > + error = -EMFILE; > > + if (charge_current_fds(files, 1) < 0) > > + goto out; > > Whoops, I had that message ready to fire but didn't send it. > > This may have a noticeable performance impact as charge_current_fds() > calls misc_cg_try_charge() which looks pretty expensive in this > codepath. > > We're constantly getting patches to tweak performance during file open > and closing and adding a function that does require multiple atomics and > spinlocks won't exactly improve this. I don't see any spin locks in misc_cg_try_charge(), but it does walk up the tree, resulting in multiple atomic writes. If we didn't walk up the tree it would change the semantics, but Netflix probably wouldn't delegate this, so at least for our purposes having only one atomic would be sufficient. Is that more tenable? > On top of that I really dislike that we're pulling cgroups into this > code here at all. > > Can you get a similar effect through a bpf program somehow that you > don't even tie this to cgroups? Possibly, I can look into it. Tycho