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 EFE94C54E58 for ; Sun, 24 Mar 2024 02:27:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E0FD46B0082; Sat, 23 Mar 2024 22:27:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DC02E6B0083; Sat, 23 Mar 2024 22:27:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C87A96B0085; Sat, 23 Mar 2024 22:27:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id B715C6B0082 for ; Sat, 23 Mar 2024 22:27:58 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 1C3831C0A4E for ; Sun, 24 Mar 2024 02:27:58 +0000 (UTC) X-FDA: 81930347436.28.3C792FB Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [62.89.141.173]) by imf19.hostedemail.com (Postfix) with ESMTP id 3895A1A0008 for ; Sun, 24 Mar 2024 02:27:56 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=linux.org.uk header.s=zeniv-20220401 header.b=WTdg4nnJ; spf=none (imf19.hostedemail.com: domain of viro@ftp.linux.org.uk has no SPF policy when checking 62.89.141.173) smtp.mailfrom=viro@ftp.linux.org.uk; dmarc=pass (policy=none) header.from=zeniv.linux.org.uk ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711247276; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=5anrztUeF8JG/Q33v10wfAqJ9O4OuzO3WxPcGzCz/TY=; b=2IAXILE1PMHrv5qd4eDKVW1kUoq/8qzZ0SurVgwofG1sJtYxawU608sAEP1NtHrc5J40hN jAWcrPT2j4Jxm5AdoiQYkB7pxp0xgeUygKjj/At4trmdC/rSQjL27eTrxbYrsC7rBXpqeY Nz5wZYXHTtULMMPj9JnJvYoWk/0iI5A= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=linux.org.uk header.s=zeniv-20220401 header.b=WTdg4nnJ; spf=none (imf19.hostedemail.com: domain of viro@ftp.linux.org.uk has no SPF policy when checking 62.89.141.173) smtp.mailfrom=viro@ftp.linux.org.uk; dmarc=pass (policy=none) header.from=zeniv.linux.org.uk ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711247276; a=rsa-sha256; cv=none; b=3qBuk9S9lkgTBP9lk2zhViVyeZ+wLGk4Gs0s+vaPY0FONTYV+R+XZaMT5+9uan+InDUmdf xwCOrxMpuS1ZyHOcX/H8pkGwHsWWL7n4SaLy66crpkgcCJR0NH0qJ6dkcBkqd2JogtOOs0 iYQEBU3zQrwSR+ko8vTPy0CgGOJ9Uak= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=5anrztUeF8JG/Q33v10wfAqJ9O4OuzO3WxPcGzCz/TY=; b=WTdg4nnJUuQpkBIFuZFHSNxfo/ p20+XA4M1RXQ7tTZAMDLOw7HTZW4HJBbv/utts4jrNzOkQHc2MBpBnrE5KgNf+ODD747qMSw0o5Pi oSDzWCvaLWU+8wykiMlVhhYQlACyKobyeEMzxyVFBL+KZU8Vkdz0FvXwptw1NTlGviMxwYwznjhp1 EnkViMnS2CCk9icn5ZnT7bSz58vEETs/gVjsZKMzV6wbfeOH3PBVzIQj7oRGdlIHHslmKR1NIOSOR 1lJfUll3dyJE5j42V5946Tz7SZfaYUJfWQLCQ5o8mzainCz7fDkPWJr/mH/ncNAdbsev9EAMaY+Dp qF6Lurnw==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1roDa7-00FXRD-2L; Sun, 24 Mar 2024 02:27:31 +0000 Date: Sun, 24 Mar 2024 02:27:31 +0000 From: Al Viro To: Linus Torvalds Cc: Vlastimil Babka , Josh Poimboeuf , Jeff Layton , Chuck Lever , Kees Cook , Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Johannes Weiner , Michal Hocko , Shakeel Butt , Muchun Song , Christian Brauner , Jan Kara , linux-mm@kvack.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH RFC 4/4] UNFINISHED mm, fs: use kmem_cache_charge() in path_openat() Message-ID: <20240324022731.GR538574@ZenIV> References: <20240301-slab-memcg-v1-0-359328a46596@suse.cz> <20240301-slab-memcg-v1-4-359328a46596@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 3895A1A0008 X-Rspam-User: X-Stat-Signature: 8xgzfdmzaug5ee1p9mbrrhayn9o5r3zt X-Rspamd-Server: rspam01 X-HE-Tag: 1711247276-360502 X-HE-Meta: U2FsdGVkX1/qw9lezfpn/kDREV/fddhRGBq2DHbJj4KR89gc1gE+tF/5IFbxyHi5xX+snAbdXGmXnQdhToSNAlRn7g4kt8YlCfi/yYJCKDTngQhWelZzV7irIWkAh4V315GYAp2+B6ifSyM6JyGSuFFmfNRiLCYXE9y77KxMfkoIu1RKyyujo0OKTwL5NhE9BZc2zLzLU8VBvRpFraNfFCvacD3wBH3r3CHzAXDpfOV2ixLHBftp3zIHflhGowomkjVSQJHn7ZC6AwuSVhKVWUSnAvMG/yISTTljbNilTVwnbnVrGbSvDsOgtO944b4Z/rzMyoGUEd9Trj4wp2PATylSAacM1tXCpp8+Xpb8CqnSfsDI0Q9PBbkPFnpBX0YeENE0Wl83jjEcllYjqC7npbDIyEnv2cxkolRFeVcOuoWyFCJ7mhKChSmqmI+UxMTYI1Nm74PU0feY6pns6p7n8wcrqZn857UYwcCJTjyzXTaYAKX/YgaZirxYxD+wtAUBGm5aW+x7cnxQXXDCCjB25fpKMoxXintg/A9kk4Jv59dVS2x5CKPnhb8BMpwc4I4tVBYHBBozwN+irO9SSxpLe/+GmlKoVPFHLMwi8nfwA3kldBcqvF4POIn9b/9nNaytmS44obADB8gmW6bjlMKvYjChgP0XZsbA5/GKHBQrfBEC3wcRgzUw5U6wGEqyuLLObOc0KRDyShM+X9Im5UNNw8k8exfzliNIyH/A8KCXaeeFD3oZamO4mQtH0Wvtx0UnscFSpC4OvR+o7m4qC4lkbVd95tWGipaUjKWHt/bim3W3lN2ALdYAz11BBSWPSDL/KJQOQl560eNT0l+0T7BMk9b34mjSJAIUm86ed34LfC1JvTJZYMHoDz3eEIbDE1Art0ZSUkoYPiZIyXMjZX7sCJpa9mfO4CkWWZUw1wgGkRgyM/ajZc+uAK5p7TrTP3cOsDhlz0hjCBc2oSm0Rnh b8pTK0fO wRSSZLhrOQ9C9D3+bi/yPt5blgFUnnAIBxM1g/j9gdUPGBuKw1q00SdduSYFOyz9+6xdc+U+baslN09W1IJ2JX0dBE8K1B0wWXR48imUYA6xk1DyDqg3Fvuf02BrghprL2h+61q/4OHWJEflZFwYXcTPrXCJ0anjJBmzFeU2Rnkmj2Bw/j8fv+Fl/5ls8bBBHoRkmp9QBAZlLD1+mhVFZWSe8PMVlmyu6Pa++3KHT0ZNZcvcQr6PaEd+hnMxc/TfZabbyAtugvXGL55oayt+tglpDwEG8Yf5eyDXbe70s5O1h6eifMz+7XlUOrKbMB0HGgLkJTPTtlarEeGOEKSWM9xJA3grd77k8Mp9+upjd5idZqVSQ0fL4mPcfujszUzLrL+SvWs4VeEIafJiDFqhuJxkPTIXUKf5/pdDJMu1Ugh/VA3POv+/lFYNE4uXvZ4/QmhC4uwsLU/NaCXbUHFZ91UYLzg== 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 Fri, Mar 01, 2024 at 09:51:18AM -0800, Linus Torvalds wrote: > Right. I think the natural and logical way to deal with this is to > just say "we account when we add the file to the fdtable". > > IOW, just have fd_install() do it. That's the really natural point, > and also makes it very logical why alloc_empty_file_noaccount() > wouldn't need to do the GFP_KERNEL_ACCOUNT. We can have the same file occuring in many slots of many descriptor tables, obviously. So it would have to be a flag (in ->f_mode?) set by it, for "someone's already charged for it", or you'll end up with really insane crap on each fork(), dup(), etc. But there's also MAP_ANON with its setup_shmem_file(), with the resulting file not going into descriptor tables at all, and that's not a rare thing. > > - I don't know how to properly unwind the accounting failure case. It > > seems like a new case because when we succeed the open, there's no > > further error path at least in path_openat(). > > Yeah, let me think about this part. Becasue fd_install() is the right > point, but that too does not really allow for error handling. > > Yes, we could close things and fail it, but it really is much too late > at this point. That as well. For things like O_CREAT even do_dentry_open() would be too late for unrolls. > What I *think* I'd want for this case is > > (a) allow the accounting to go over by a bit > > (b) make sure there's a cheap way to ask (before) about "did we go > over the limit" > > IOW, the accounting never needed to be byte-accurate to begin with, > and making it fail (cheaply and early) on the next file allocation is > fine. > > Just make it really cheap. Can we do that? That might be reasonable, but TBH I would rather combine that with do_dentry_open()/alloc_file() (i.e. the places where we set FMODE_OPENED) as places to do that, rather than messing with fd_install(). How does the following sound? * those who allocate empty files mark them if they are intended to be kernel-internal (see below for how to get the information there) * memcg charge happens when we set FMODE_OPENED, provided that struct file instance is not marked kernel-internal. * exceeding the limit => pretend we'd succeeded and fail the next allocation. As for how to get the information down there... We have 6 functions where "allocate" and "mark it opened" callchains converge - alloc_file() (pipe(2) et.al., mostly), path_openat() (normal opens, but also filp_open() et.al.), dentry_open(), kernel_file_open(), kernel_tmpfile_open(), dentry_create(). The last 3 are all kernel-internal; dentry_open() might or might not be. For path_openat() we can add a bit somewhere in struct open_flags; the places where we set struct open_flags up would be the ones that might need to be annotated. That's file_open_name() file_open_root() do_sys_openat2() (definitely userland) io_openat2() (ditto) sys_uselib() (ditto) do_open_execat() (IMO can be considered userland in all cases) For alloc_file() it's almost always userland. IMO things like dma_buf_export() and setup_shmem_file() should be charged. So it's a matter of propagating the information to dentry_open(), file_open_name() and file_open_root(). That's about 70 callers to annotate, including filp_open() and file_open_root_mnt() into the mix. 61, actually, and from the quick look it seems that most of them are really obvious... Comments?