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 A308CD41D41 for ; Tue, 12 Nov 2024 00:37:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 382E96B009A; Mon, 11 Nov 2024 19:37:54 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 3322B6B00B4; Mon, 11 Nov 2024 19:37:54 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1AB9B6B00BC; Mon, 11 Nov 2024 19:37:54 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id EF8086B009A for ; Mon, 11 Nov 2024 19:37:53 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 9C802A023A for ; Tue, 12 Nov 2024 00:37:53 +0000 (UTC) X-FDA: 82775580048.28.5A0798F Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by imf13.hostedemail.com (Postfix) with ESMTP id 30A7E20006 for ; Tue, 12 Nov 2024 00:37:08 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=g+9SLYOw; spf=pass (imf13.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.218.45 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=1731371817; 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=8s+Ye3Sb5A7DOLizr5823ibyYsiIFEnIKQUrUtOpKwk=; b=BwWQvCc6XFtNRZZAh/MQ00VPxhL4JaCAtYvsEQ1wW4stjNGH/X01jbOxqoPluKBQFJj6YR 4e0ohyJm4BDHxB4rx1G5tn3lN9TzknFeLaS8eaVEmUOmzk9EjezjchWNELIfEAi8P2xRtc YLFfmzJt+8OZk9hVxlr+e4sYimVdTXI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1731371817; a=rsa-sha256; cv=none; b=QMkheBw4cjrzidI4svKOrZ/Hct+fr18CHFJj+BS05hv1XWFNUv1OrvzNWzQdUulxG4nl3F vQqTea+88jpUbl13GaFXK3nZzFdNSUSepgac5VyyMAK/eHolrGKJrd9vTDXxcBFs7Vnq82 OA+O6TJEHNo5mr9H00u1qX1AggRy9OY= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=google header.b=g+9SLYOw; spf=pass (imf13.hostedemail.com: domain of torvalds@linuxfoundation.org designates 209.85.218.45 as permitted sender) smtp.mailfrom=torvalds@linuxfoundation.org; dmarc=none Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-a9a0c7abaa6so663331666b.2 for ; Mon, 11 Nov 2024 16:37:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; t=1731371870; x=1731976670; 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=8s+Ye3Sb5A7DOLizr5823ibyYsiIFEnIKQUrUtOpKwk=; b=g+9SLYOwGx/3EgzEc2m4yfjm7CYyrZ68/iA0OPaSzbpYCgSkMTlBgx0O+BNSgvX8Wq ZjJ7vdt+wfznnGGQF9z3y2f7iLlbdAz7LYw/2TYgHW0lyL7rb3Iesuex7ME1XWCq7Td+ NUxD+3HFk8j4BJcjGLZQUjEzdC9yFkiqYAp58= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731371870; x=1731976670; 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=8s+Ye3Sb5A7DOLizr5823ibyYsiIFEnIKQUrUtOpKwk=; b=bISRqRkJP893aiV/aGZxVT4nQCD374cOllzkp1mhc6u2UI6qLyZ8g9f1fo1rYDXIg1 neA0tSc+kF3nCMkxwupMnQz2lG4TpHtQMmtBWhw2fOOpPWIFvXGS6/W3/sTSTC9kegoZ TdLzdw+X2SelT+YlIafz+m+vnyPqK0amini5IHaD7/NDuZQKnX4lDO1AamsMKhZJwDC4 9RZtEgW6gjDK5PFPaxR1m0dIFE9es7a9eL3cRgHQMY+rDm+cKj63l6o7fR6k+cROjnwN Vi7UDye3R9SLcoGyBcz1dSmn4DJ/6YDTcrM7YOgq/eVyW642EB+Y9g75hZ9mWwavx/HI FK7Q== X-Forwarded-Encrypted: i=1; AJvYcCUASqoXtvVGzjMINEqtioZhmfeyJavtt8YgxtHyEBjFc/bD5cpvre5ECe8ZAD+yhgz9JgZOLzdfWA==@kvack.org X-Gm-Message-State: AOJu0Yy1tKN6A2rq7dJj/WpWGCiQ5HaTLbKhQemMnnbD5lbFwXHKBfc4 SdZ2rM2XUQ5UV3iYtYgzT1f4B6CL4PeXi4UWlobDhsE71QkGSwIL/clH/eAu/riOGxHaQj5Ck5+ HRN0= X-Google-Smtp-Source: AGHT+IFrlUFV+ODZNypCdfbk1WPyE7OXLgxMZHETzI/D1Ww1JJ6QG//bp33T3hNATVlY+1EAx+8GJg== X-Received: by 2002:a17:907:804:b0:a9a:4e7d:b0a1 with SMTP id a640c23a62f3a-a9ef001322fmr1427703766b.49.1731371869901; Mon, 11 Nov 2024 16:37:49 -0800 (PST) Received: from mail-ej1-f42.google.com (mail-ej1-f42.google.com. [209.85.218.42]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9ee0a4cb66sm644465466b.80.2024.11.11.16.37.47 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 11 Nov 2024 16:37:48 -0800 (PST) Received: by mail-ej1-f42.google.com with SMTP id a640c23a62f3a-a9a850270e2so933710266b.0 for ; Mon, 11 Nov 2024 16:37:47 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCXWxbVh3SNVQkk5LZud2+iQe/Y7+CIim0Rx+xHZHZuZmG2nQr8tVdtWaB6Nq4kVdgWqV7jcZ9AA+g==@kvack.org X-Received: by 2002:a17:906:dc8a:b0:a9e:b090:e65d with SMTP id a640c23a62f3a-a9eeff383eemr1195211366b.32.1731371866917; Mon, 11 Nov 2024 16:37:46 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Linus Torvalds Date: Mon, 11 Nov 2024 16:37:30 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v6 06/17] fsnotify: generate pre-content permission event on open To: Amir Goldstein Cc: Josef Bacik , kernel-team@fb.com, linux-fsdevel@vger.kernel.org, jack@suse.cz, brauner@kernel.org, linux-xfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-mm@kvack.org, linux-ext4@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: u7t6detqdhfgu4cpz99oxy9bg99h7ehc X-Rspam-User: X-Rspamd-Queue-Id: 30A7E20006 X-Rspamd-Server: rspam02 X-HE-Tag: 1731371828-351580 X-HE-Meta: U2FsdGVkX18X3dmhOfswviRLras2pffd1aXG0XCGNZhNAFGR4zodfPOOnsF/AS5ENH1J8ENcTVZEuqhw5ycG31bVyDt/7/a5EvPedeV7efyiFlM/Qjyn48hS62gB2hOvX8I/8C8A2FYQHcD0z3tLzu9hJhrwQj6aJ6z0Lrgz+dxsouAQk4i824KxEDr1y3gPiOl7GSnyNbJg0Faegxaw+csl5mvMIrDQGXuHu64WZCHmtkmZ4c0mFKn2sXbYXIT4W5u/AoiwcFUz8ZbEsXGKZ5vaIQjgqCa0Pc39xWCzowr07eOMM5IglXR8z1nbTr+C2V8G4obGs9HgFG7dzEibuwBfKgu4Ct4GvDFdfaUHn/Gs7sfeKe5Nwc+3F5ZHDl2i5kpNrxceFBFVaMaSluIwGkj82arPtTjtn5YzEh+HhEyjE4ivg5kqcTLjMgue9fh4n6EsN0Bto0QsCtJeicyUheeVLUo2nZFkl8kbW2/2sUcMoDX8K9jJHGsdfzc7qFwf2ZJQz/hheaRnXjmm90XhY5a44jnjxsyxOC9QCrZzWV367NUKPebOupvRq/oaMc5sP+/A4K9dqy5AL9JeOp3tVIoIVSHmeqZ7ZE0oR6Bi6bfpWs4b3Nvoy07EHfHNsQl56YMERPyVyFWPD8R0sKTsf+Tymboj8em+vthnYfEUAiv9/W6PP2oZalNE58nyGBh+M4abvWcono1EJJoynoTVhxkRnlNwsBOMDjlaMG682uoKyYcn0JVMeruabfo9jFoRQcKUCDLVoOG+nlNl+XzsXFWcP4G67re9S3N5UDgzSllTp1MlpJxpC/JRoaEbcCxQY/yrxpl0bCGbS621pEv2ILARiopK27aH+BzctHeKaU6/e+MCCl1Pfp/RukcKlyNzOYNQZyUneCAnJVzT0yPHbpHTFjIe/3XcEexLwj+eeE4+abOMgfRMvbgXFLBcAKqU0BzD3BEpvnQsiBHBqOO CRUkn1Pw 6LEZy5DaxbbQGwLwgpZc6zuQBU4wXXT8SPudNfjWem3QZRTX7nPDYW3ywyAREt9k86ve4pUBWVwC3aZF8roReSVGUSvQCxxMtK7iXZ24S8nrP92Tjf3tdfvfuHC/30mnBkDNuu2RwieJGSANn7CZWPgIzemB1FEHJ/3iYfFRM/nRXQvuG+Pg6cfJ8KiBkBbElb0aP8KhGO2zOswelCPVUP8Vsc6Wn/MPAdyAFgHvCntu+4xBiTbiZ/TJumgjnFGcawJKXz5VUY6ekamo++SilEHnVhvIEKOY7Vp8G8o7eOb4/lxrXO4UPepMm6cqMf8nBAp+warhdc1UPZUYBuD+xVC4eR8TjtK0nrCpwNzZ1BhwRWLas46E145Lw7wgERScVKZE+O7RqHf9AC4BlvvRDycxUatbB31Z3Ipkyajs87h2FrZFMeDgpuK2OWYtif31Am0CPZTvKWUrbYhxZCrI3t5tEBxPkIfouvyyktTHXkO5DlQ6NKyU3gN/Xrw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000045, 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 Mon, 11 Nov 2024 at 16:00, Amir Goldstein wrote: > > I think that's a good idea for pre-content events, because it's fine > to say that if the sb/mount was not watched by a pre-content event listener > at the time of file open, then we do not care. Right. > The problem is that legacy inotify/fanotify watches can be added after > file is open, so that is allegedly why this optimization was not done for > fsnotify hooks in the past. So honestly, even if the legacy fsnotify hooks can't look at the file flag, they could damn well look at an inode flag. And I'm not even convinced that we couldn't fix them to just look at a file flag, and say "tough luck, somebody opened that file before you started watching, you don't get to see what they did". So even if we don't look at a file->f_mode flag, the lergacy cases should look at i_fsnotify_mask, and do that *first*. IOW, not do it like fsnotify_object_watched() does now, which is just broken. Again, it looks at inode->i_sb->s_fsnotify_mask completely pointlessly, but it also does it much too late - it gets called after we've already called into the fsnotify() code and have messed up the I$ etc. The "linode->i_sb->s_fsnotify_mask" is not only an extra indirection, it should be very *literally* pointless. If some bit isn't set in i_sb->s_fsnotify_mask, then there should be no way to set that bit in inode->i_fsnotify_mask. So the only time we should access i_sb->s_fsnotify_mask is when i_notify_mask gets *modified*, not when it gets tested. But even if that silly and pointless i_sb->s_fsnotify_mask thing is removed, fsnotify_object_watched() is *still* wrong, because it requires that mnt_mask as an argument, which means that the caller now has to look it up - all this entirely pointless work that should never be done if the bit wasn't set in inode->i_fsnotify_mask. So I really think fsnotify is doing *everything* wrong. And I most certainly don't want to add more runtime hooks to *critical* code like open/read/write. Right now, many of the fsnotify things are for "metadata", ie for bigger file creation / removal / move etc. And yes, the "don't do this if there are no fsnotify watchers AT ALL" does actually end up meaning that most of the time I never see any of it in profiles, because the fsnotify_sb_has_watchers() culls out that case. And while the fsnotify_sb_has_watchers() thing is broken garbage and does too many indirections and is not testing the right thing, at least it's inlined and you don't get the function calls. That doesn't make fsnotify "right", but at least it's not in my face. I see the sb accesses, and I hate them, but it's usually at least hidden. Admittedly not as well hidden as it *should* be, since it does the access tests in the wrong order, but the old fsnotify_open() doesn't strike me as "terminally broken". It doesn't have a permission test after the open has already done things, and it's inlined enough that it isn't actively offensive. And most of the other fsnotify things have the same pattern - not great, but not actively offensive. These new patches make it in my face. So I do require that the *new* cases at least get it right. The fact that we have old code that is misdesigned and gets it wrong and should also be improved isn't an excuse to add *more* badly coded stuff. And yes, if somebody fixes the old fsnotify stuff to check just the i_fsnotify_mask in the inline function, and moves all the other silly checks out-of-line, that would be an improvement. I'd very much applaud that. But it's a separate thing from adding new hooks. Linus