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 40680C4725D for ; Mon, 22 Jan 2024 05:10:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 87AD76B0074; Mon, 22 Jan 2024 00:10:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 828AC6B0078; Mon, 22 Jan 2024 00:10:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 717C46B007B; Mon, 22 Jan 2024 00:10:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 63CDE6B0074 for ; Mon, 22 Jan 2024 00:10:30 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 0F54C1C0D20 for ; Mon, 22 Jan 2024 05:10:30 +0000 (UTC) X-FDA: 81705771420.19.89879C9 Received: from mail-ej1-f67.google.com (mail-ej1-f67.google.com [209.85.218.67]) by imf01.hostedemail.com (Postfix) with ESMTP id 1008F40010 for ; Mon, 22 Jan 2024 05:10:27 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=PjqpzZQx; spf=pass (imf01.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.218.67 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705900228; h=from:from: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=bX4dObL3c95165XLM7yzt7rEIgdTlOtpBnPF9EhNwGQ=; b=TQ19BKrRrKCziiNloktgzbOA/4Xmn5MMzhPJwbw22TWl9vVjAWPz3G85nf5DWoYrxpJUI9 Ae/Zh+z4Vy9mraHGHWBjl9dm+14jMVxTr6LhALWbEIxtL27HCy6YtM5a89dGZJrOFXuA4k fcTMoj8/7kIbW3WC9OkyAwbhCjgUFkk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705900228; a=rsa-sha256; cv=none; b=mMAdVJ/9vK8KqUmquSrLbxfIYAisbut9SG1bnihTwSp62E3kZkjW7EsJee/iooTANEEPcw IvqVXu7xdmklJQsxXVvaesziArQU6t0uzYPYy3ay8Ye6Sh0ssdHg7ooCC4nmcItU6wl+mC 0vCyD0cW4qOFm6CGql/HcWwNJ3lK2uE= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=PjqpzZQx; spf=pass (imf01.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.218.67 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none Received: by mail-ej1-f67.google.com with SMTP id a640c23a62f3a-a29058bb2ceso237769066b.0 for ; Sun, 21 Jan 2024 21:10:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; t=1705900226; x=1706505026; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=bX4dObL3c95165XLM7yzt7rEIgdTlOtpBnPF9EhNwGQ=; b=PjqpzZQx94ubN9wqBiU1hDxq4vmcx17UNHJLcIYHNV1VIiTlWOV5bWIAix9LENd0L1 +/mQyV77wbi2KCMfiTbOrS7bcqAVW9E2c68qBunN5gsUKXRHa2xwsgl0GRWR8yMvSN0L FvcRmY6ErzMW+9I78vl2MHMAIzATQLkutnyQg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705900226; x=1706505026; h=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=bX4dObL3c95165XLM7yzt7rEIgdTlOtpBnPF9EhNwGQ=; b=acV2K/BUOAadbDiSxl/qJuiUyhXBKRX3W++jGx46u0YqxlvkykpCb1WZBeoqwvfmoL glY60o415Nmn3hGWe4MTRISaqAy9O7mVbv41OTZmZwC6p01c7YsCZbikz0GEXhsC/02x Z/g7q42Q6ukViqNNBuG8O3YD7p6GWAxdJ+wFE8wBdWhxE6LWajFKL8WhhDtoZHntX1pc ZlG6wJH7GauFqm/EySYVlP32+EsLC40/usrxRLlFILpjCBFLbzBHoJDj6kMEt6iG6O3Y mt+Rquo/UvTIB02n33JRLDc4qTjDAIYlff83uyxcqR8w0aQVbmdonjtw2U+1pBFl9uPM rMKw== X-Gm-Message-State: AOJu0YzEtdp+d165Mvw/dSld+X9lUKVjT9OwVOgzMWXtK9+M78XPkksZ mr7weN54p5PHQoKt5oZgwwkLXutt83q07Qs44Enkbh0GoZYQ05NJT8DaC8YmWF0IL8Y7KsA8Zpz OJTX3jXrA X-Google-Smtp-Source: AGHT+IF6wEtleHNsByrO1k/fmSidL9uY79kRjd/RtSfZFvn5Bl/0HBVEwRcIaRxOy5pGVzAHMo+HdQ== X-Received: by 2002:a17:906:ca44:b0:a29:bf51:6a9f with SMTP id jx4-20020a170906ca4400b00a29bf516a9fmr1190050ejb.127.1705900226449; Sun, 21 Jan 2024 21:10:26 -0800 (PST) Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com. [209.85.128.53]) by smtp.gmail.com with ESMTPSA id u26-20020a17090626da00b00a2ed3049392sm5888573ejc.82.2024.01.21.21.10.26 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 21 Jan 2024 21:10:26 -0800 (PST) Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-40e9d288f45so33473855e9.2 for ; Sun, 21 Jan 2024 21:10:26 -0800 (PST) X-Received: by 2002:a7b:ce95:0:b0:40e:49ac:e4a6 with SMTP id q21-20020a7bce95000000b0040e49ace4a6mr1440118wmj.171.1705900225838; Sun, 21 Jan 2024 21:10:25 -0800 (PST) MIME-Version: 1.0 References: <6667b799702e1815bd4e4f7744eddbc0bd042bb7.camel@kernel.org> <20240117193915.urwueineol7p4hg7@treble> In-Reply-To: From: Linus Torvalds Date: Sun, 21 Jan 2024 21:10:09 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH RFC 1/4] fs/locks: Fix file lock cache accounting, again To: Shakeel Butt Cc: Roman Gushchin , Josh Poimboeuf , Vlastimil Babka , Jeff Layton , Chuck Lever , Johannes Weiner , Michal Hocko , linux-kernel@vger.kernel.org, Jens Axboe , Tejun Heo , Vasily Averin , Michal Koutny , Waiman Long , Muchun Song , Jiri Kosina , cgroups@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 1008F40010 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: zj8gpn5e7n75nwh7wtpoiqfn3gemqr9t X-HE-Tag: 1705900227-665953 X-HE-Meta: U2FsdGVkX1/hZpdCruwG1K6RDp6ig6ua1VwD0vsKigfDuuKJpg3vZF9JFMWUmY8K3WoTx113WBxDq8pDQU7AL8nNMHDyQv8nwagRROb1d4wmgdcggORHp4Kh3Qd+eobCbJEYRVvlv7IiQT0w94mFlXSkM9VsipPwjKSBo0zJnJtTBlOY/8hDIJA/dfJ+S9RVCmx077qK9ap8Wm/oW4beEMSv7vQC91i46L5sy9agqoVcC4ckipvAXbmRwvK6HAWjN4tLiBaEODWvdAOFgvptg/2o1solMSmeLr3aPa54+DevmFg/iLRFRE55sYzGPoeAf6iYIrzlKoBUScEhStNz4fSZkQUrK9PN1kyyQcogP+Aof/6zLKfmOf/vo7NOippqH1f/TRRyGLqGYVHJh7+Vs7JB7KhDUtWlcvpx/IH6/NuZa0Aoqahm3shRec0bPjWL3NQXc56/Vy771pMgNhh3uG34kUVci6eXwgSlF2URBaQbjBMT/dT39r97YycJ6x+NWO/yHdCLy3jZMG95a2T6oSuVs5UCUlZUmO59jxD58tiBYLz6J5pwzR0AirbaIOCi6OqRZXm57jEGv5ztRmiP2awO6z+mVyyNfRjd0VZcRXRzHi32/saLyR/MbsaziZm90JZUvsMB3wTO90XEkYQDv1Z4vcu3ctQOmyaDQPRJJ1xB5lOZghc0/8VBKaicFZ+z+0UGuYHTELySJzST3+kAS1qnN8tyCKm5Q9Gqa0a94dpZbCAbJHhx7nYoFbI42YVfODZqbMI6JZpEIuLLAU25ffGpfpNfVTAzoQikH57fNNzWyXPQnBq6w7yn8nWLczuqWYGFn9JYJy8pXLNDm9JhnSDR8NjT4LmVNQRQ0IubLvS5QWdka52lAuF25LRHi3/n1GVcn3FIksWQARaendtwXWhL7FGVFsP4DBomaUaLc5bhds23V3ILzaDeIZMgFeDt2s191xslkZA/LYLcwb1 4W+vAqKv UxfsPTgiin/UnCeJ0vKLlXtvRlt4QHFqQRNu7dFIsNj+Iq1O67WAZg8ITX0SqaC7v7SmdIP5i5MFWD5wnWjm/n0Co2islHlg8U3jcy1wFI7NskwUx+EIGG/SfsBeBLrYOSY2fbzB71Xg+wRKPhqqYduxWJ5+S8KZ8la3OI3hRKHrszwqhsnOdk7okYYLjDebST73E3aj/VAZTqeFWzioxwiFmORRPQlngDkndy5kZ+pXa6LWWHgtoaXRBGePn/M0KHrwIQkUwVbFYCufmfSdZhxX5Nlq3W/S0MFrOW31gahZgqHCxt0MlDhEXv74PPc5CGwatnhDGs4D7HkzZA/NpJk0/GPG9fldaRV8sakuPxmsS0z8ZfvVOo+KFcrqxtGTKEX8ZtLUWLljcqwHnXoXiwU7W1qe9BtkuTFA4BfHhNy5FmJJVmPtmOEV+0VWpIDuBqe6OMXF9UeXDyp60+pxid7zEaVRbarHSzG6EpCYusZyMKIaxzFqCusTeKP3QD31/d3sa5xw8hwF9TZZWb3D55rVpdbrHAQmqX/C8f/GvA8xJnRpXN4AunxEu5txREkV+58W8vzDbJMObSSSiD/eQ8r/2uDc1MPxwwbk4 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 Wed, 17 Jan 2024 at 14:56, Shakeel Butt wrote: > > > > So I don't see how we can make it really cheap (say, less than 5% overhead) > > without caching pre-accounted objects. > > Maybe this is what we want. Now we are down to just SLUB, maybe such > caching of pre-accounted objects can be in SLUB layer and we can > decide to keep this caching per-kmem-cache opt-in or always on. So it turns out that we have another case of SLAB_ACCOUNT being quite a big expense, and it's actually the normal - but failed - open() or execve() case. See the thread at https://lore.kernel.org/all/CAHk-=whw936qzDLBQdUz-He5WK_0fRSWwKAjtbVsMGfX70Nf_Q@mail.gmail.com/ and to see the effect in profiles, you can use this EXTREMELY stupid test program: #include int main(int argc, char **argv) { for (int i = 0; i < 10000000; i++) open("nonexistent", O_RDONLY); } where the point of course is that the "nonexistent" pathname doesn't actually exist (so don't create a file called that for the test). What happens is that open() allocates a 'struct file *' early from the filp kmem_cache, which has SLAB_ACCOUNT set. So we'll do accounting for it, failt the pathname open, and free it again, which uncharges the accounting. Now, in this case, I actually have a suggestion: could we please just make SLAB_ACCOUNT be something that we do *after* the allocation, kind of the same way the zeroing works? IOW, I'd love to get rid of slab_pre_alloc_hook() entirely, and make slab_post_alloc_hook() do all the "charge the memcg if required". Obviously that means that now a failure to charge the memcg would have to then de-allocate things, but that's an uncommon path and would be marked unlikely and not be in the hot path at all. Now, the reason I would prefer that is that the *second* step would be to (a) expose a "kmem_cache_charge()" function that takes a *non*-accounted slab allocation, and turns it into an accounted one (and obviously this is why you want to do everything in the post-alloc hook: just try to share this code) (b) remote the SLAB_ACCOUNT from the filp_cachep, making all file allocations start out unaccounted. (c) when we have *actually* looked up the pathname and open the file successfully, at *that* point we'd do a error = kmem_cache_charge(filp_cachep, file); in do_dentry_open() to turn the unaccounted file pointer into an accounted one (and if that fails, we do the cleanup and free it, of course, exactly like we do when file_get_write_access() fails) which means that now the failure case doesn't unnecessarily charge the allocation that never ends up being finalized. NOTE! I think this would clean up mm/slub.c too, simply because it would get rid of that memcg_slab_pre_alloc_hook() entirely, and get rid of the need to carry the "struct obj_cgroup **objcgp" pointer along until the post-alloc hook: everything would be done post-alloc. The actual kmem_cache_free() code already deals with "this slab hasn't been accounted" because it obviously has to deal with allocations that were done without __GFP_ACCOUNT anyway. So there's no change needed on the freeing path, it already has to handle this all gracefully. I may be missing something, but it would seem to have very little downside, and fix a case that actually is visible right now. Linus