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 0FC99D711C7 for ; Wed, 20 Nov 2024 16:42:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9713C6B009E; Wed, 20 Nov 2024 11:42:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 920E16B00A0; Wed, 20 Nov 2024 11:42:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7C29A6B00A1; Wed, 20 Nov 2024 11:42:33 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 60E1C6B009E for ; Wed, 20 Nov 2024 11:42:33 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 11BB0A07AB for ; Wed, 20 Nov 2024 16:42:33 +0000 (UTC) X-FDA: 82807039392.08.3DB5D69 Received: from mail-ej1-f54.google.com (mail-ej1-f54.google.com [209.85.218.54]) by imf21.hostedemail.com (Postfix) with ESMTP id 2CDC11C000A for ; Wed, 20 Nov 2024 16:40:52 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=R07+nElY; spf=pass (imf21.hostedemail.com: domain of amir73il@gmail.com designates 209.85.218.54 as permitted sender) smtp.mailfrom=amir73il@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1732120859; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=FcZ/qVRFrTrPsl9wheVLwLdCEZFphqCBC9fFa8ajopI=; b=KC+5HhT5TKzv7OssZ0ZYZSDE2SdpNhVmT/aGpd47uWo1p3Nu/FuX93X5/ly1iVCMqCWESV m2ETxmXQF6nRV461uifmpFe/3kg3ovd8OPpw2G4zTeLDe/8SGzsjL4ejcUi1ergp7oBz78 0mifmwVjvwH8aThYUnp9r7vPz9HlIaM= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732120859; a=rsa-sha256; cv=none; b=UegTCODNshb7tvQkC1v0rYF8VniXfiNjrw2QpntNcmWk5/NpO9RZeV8EDAS9tY6hXP94ds OQ/0XGmsz8jMLlF+PP/PYNqt9HeEanNnte3r/Cw2fYZDXGd4dxWCULXWJQS3ByZiCVZ7Mo DXGGAWroO+Aw4RIrL4HazuWHkR25sdQ= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=R07+nElY; spf=pass (imf21.hostedemail.com: domain of amir73il@gmail.com designates 209.85.218.54 as permitted sender) smtp.mailfrom=amir73il@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-ej1-f54.google.com with SMTP id a640c23a62f3a-a9ec267b879so959292966b.2 for ; Wed, 20 Nov 2024 08:42:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732120950; x=1732725750; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=FcZ/qVRFrTrPsl9wheVLwLdCEZFphqCBC9fFa8ajopI=; b=R07+nElYKmTtcCkzXOV7uQGCLqlpOj8O17U4MKTUQaKBOomabEk3E7KImDZvAiMOth iHgMAz2huHx84vG1naQD6vdJhrQnBYCbyRC7DFNav4EEvEhhwR6mcMCcQ+Cdf8eh53VC CN4FbotipqfvSMwA9ZyP2bHRZ1JdnY6mTEBV0JvnIjEN/fYS2GwKunTlDba33GSQELMj njsAm/GmN8wudS6EqoFpZ9b5afFTnEKZCocWtVFpH8IHTtSE5F98IuAbuJFFNZMPVwlR 2PIBVJ1y53zVWdfw0fwB3mpylxFY/OOx0Q1UUrD/JH/HCDQeE+zKSCo8c4g28dFkKO2g Xipg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732120950; x=1732725750; h=content-transfer-encoding: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=FcZ/qVRFrTrPsl9wheVLwLdCEZFphqCBC9fFa8ajopI=; b=lwFOpo19bbt+TVRB4RSrXEBC5yVGIDSxB/VbC9TMX3XU4XPesrVUrN6ZdOmiJC86sS 1mhaJ1WCGt7lDs5u1HYmOsbh/ZAIcB6cUPCv/G1b8YEY8EXmNM+1PVH4CeGkZAHgOqDv DNyLe+C3T8JsyyiN4jz9GdOqrtVTVURKQofTFiYxBPp1swT5fy9fRbNz5eX2RiPk+bMl FQ8XZ4dzy6uLDiGhUWElkYjEEkuZyptscRcB9hrOLaygoSl8gNOqYUfsZ0z69mf8HgLf ryoCt+8NigbNQbfD/t0q3qWdoXs2iS31yARtBzi/aoao3BVRMyYWBWICz6ryNP6tx8V8 A9sQ== X-Forwarded-Encrypted: i=1; AJvYcCXcKWIAoLVWw3FXRgPWEZKF/Iz41ACIz1MMZRThWVfzlZl4PJO1rPs3b2GHomjgKSZyrDTsf0RI9w==@kvack.org X-Gm-Message-State: AOJu0Yy+KxxQ8lZcTYxU6UIRFLPeaucS4KlM0bxw0ZTtOTU+3a8UbDL3 0FeOgsDKZ3mDoQQsc+YZs99qhHJT1zeYNhQv9f69+qsmCg2JL1ke1I+cbuqRwK/A4kvAbzZT4mD RY7l26EyS+zH512L0Q/MR9LgHfoY= X-Google-Smtp-Source: AGHT+IESXkPbNLJC+3RMgOX6j+HutEH0kyIIO5eXd31+9oHtDOGVK9zXOmSMvlelO3h+JPf71j6COjp4Ahjs7rPmPGw= X-Received: by 2002:a17:906:4793:b0:a99:fb56:39cc with SMTP id a640c23a62f3a-aa4dd71e7famr354632666b.38.1732120949412; Wed, 20 Nov 2024 08:42:29 -0800 (PST) MIME-Version: 1.0 References: <2ddcc9f8d1fde48d085318a6b5a889289d8871d8.1731684329.git.josef@toxicpanda.com> <20241120160247.sdvonyxkpmf4wnt2@quack3> In-Reply-To: <20241120160247.sdvonyxkpmf4wnt2@quack3> From: Amir Goldstein Date: Wed, 20 Nov 2024 17:42:18 +0100 Message-ID: Subject: Re: [PATCH v8 03/19] fsnotify: add helper to check if file is actually being watched To: Jan Kara Cc: Josef Bacik , kernel-team@fb.com, linux-fsdevel@vger.kernel.org, brauner@kernel.org, torvalds@linux-foundation.org, viro@zeniv.linux.org.uk, 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" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam10 X-Stat-Signature: wqj9w5feguniggti9n8tmyw37gb7h4o3 X-Rspamd-Queue-Id: 2CDC11C000A X-Rspam-User: X-HE-Tag: 1732120852-271740 X-HE-Meta: U2FsdGVkX18aTM7q7gnVjHCiF9E5YVU04M6jeucx5KqmaNkf8lTO7WfET+h98032Cc8oSmNHa+Bxe5xeoCbipDogtW4gHXBMSNa310HYOWgWz4fP5b/GJ6ItkkUm5qW+/tryaML2pqcTy7WhitpGQi/LYVm9Lf1cwyJkqFyAPWb8UPnjabKIz9vmsifadnjNXaOlByHM4ZaKKl6WUdElOM8lGEbsM6cRkdv0QCQImb3S6uGkJ84KukPbHP7UleY31sd/ujVQ2f3oQQLnq4YK89gH7Zp3a3ujESyVdyhIfCEzMDdghO1G/O0/d66WhI5/PmXHjL7yW+BouaqtwbdWeN64GEJ7oy6wX+0w5hiTobS6AY1nXdjpYRFM+IBAM2wX+sbLnVGFZ/zPz73sZTMnTZFDBcLCi/nLfDwB+csWGD34qsx5MELOg3PWAEkfi/047qZ0R9Xm2PJuu127C0qns98aKCfXvI21qmMTtNlfqpWjjHY0cALc/Dj0rEXrGgt/Hb9pnC0auFoSsQRW4SHyUQtw+Rlc4mE7AtG1U2vjaG2qe8mIQtWlasF0ttnbyrckuLc2bPlUlo1GyhYJI2TC3F4qwjsq/fsqGRBYzJQjq59v4u9ziWNZQ1neA+frvODtObVdGvEPainluNrA0+GkevGiwxjuM86E81B+pmSPwsGdVHlOXkd2V1btmLW57IoI0Ruf4CZUBMSEyu82BShCbgdCJTQj8vdBr+XRtLZT224NC9tGqNveNvsPcfp8MzIfSiUpacerNSqOEZbO5d8snsO7zzSuCR8WmnoeEn0TeFSUxtmCvzGx7LVNXaZS95Hh9ORyt6Emz/6EL2pEpPRSnr4zLj//rlsPYoecAdEeXF07X8naDdMnqXOPC1+c6EruQCFT3b2YWN31c8FhvjU5wfheYvRXpm7ZOdd8F5c6MxJhxyAyHhkpvkPeOmhrCxL/AvPOw7RMe1xFnp3gXsV WeO1WyMA eNESNOFCxO/f64gfNkJj1d7axzRLmUm+GCcKhIU0cR1z3u25tHrA0bWdCGn5cvx2f7XdY+9zE778AtoLl374PzJiLF985rL5BC3h7Q/U6FYc24KyJnQY7zxdCRv9sXKulMfLNKCoZOSc6eHoqOnJ1u4cFZSm0V0dgVls0QWnSniOmgW7yh0zdNWHc+1Lhsy3XXQR17el5QGCwn3bOAVA3/EDi5AYhvlC/dpKQCCgy1F68fE5z69oOnGaDrypcPcmkV1D9FyGgjRWyJk+/WOfk2U7+VKMXzwOM36A5dqbertze5QUksAOd6jIfSYhlNUmEQdUtiU67Ad8R2VJOtw8liwiq7A== 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, Nov 20, 2024 at 5:02=E2=80=AFPM Jan Kara wrote: > > On Fri 15-11-24 10:30:16, Josef Bacik wrote: > > From: Amir Goldstein > > > > So far, we set FMODE_NONOTIFY_ flags at open time if we know that there > > are no permission event watchers at all on the filesystem, but lack of > > FMODE_NONOTIFY_ flags does not mean that the file is actually watched. > > > > To make the flags more accurate we add a helper that checks if the > > file's inode, mount, sb or parent are being watched for a set of events= . > > > > This is going to be used for setting FMODE_NONOTIFY_HSM only when the > > specific file is actually watched for pre-content events. > > > > Signed-off-by: Amir Goldstein > > I did some changes here as well. See below: > > > -/* Are there any inode/mount/sb objects that are interested in this ev= ent? */ > > -static inline bool fsnotify_object_watched(struct inode *inode, __u32 = mnt_mask, > > - __u32 mask) > > +/* Are there any inode/mount/sb objects that watch for these events? *= / > > +static inline __u32 fsnotify_object_watched(struct inode *inode, __u32= mnt_mask, > > + __u32 events_mask) > > { > > __u32 marks_mask =3D READ_ONCE(inode->i_fsnotify_mask) | mnt_mask= | > > READ_ONCE(inode->i_sb->s_fsnotify_mask); > > > > - return mask & marks_mask & ALL_FSNOTIFY_EVENTS; > > + return events_mask & marks_mask; > > } > > > > +/* Are there any inode/mount/sb/parent objects that watch for these ev= ents? */ > > +__u32 fsnotify_file_object_watched(struct file *file, __u32 events_mas= k) > > +{ > > + struct dentry *dentry =3D file->f_path.dentry; > > + struct dentry *parent; > > + __u32 marks_mask, mnt_mask =3D > > + READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify_mask= ); > > + > > + marks_mask =3D fsnotify_object_watched(d_inode(dentry), mnt_mask, > > + events_mask); > > + > > + if (likely(!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED))) > > + return marks_mask; > > + > > + parent =3D dget_parent(dentry); > > + marks_mask |=3D fsnotify_inode_watches_children(d_inode(parent)); > > + dput(parent); > > + > > + return marks_mask & events_mask; > > +} > > +EXPORT_SYMBOL_GPL(fsnotify_file_object_watched); > > I find it confusing that fsnotify_object_watched() does not take parent > into account while fsnotify_file_object_watched() does. Furthermore the > naming doesn't very well reflect the fact we are actually returning a mas= k > of events. I've ended up dropping this helper (it's used in a single plac= e > anyway) and instead doing the same directly in file_set_fsnotify_mode(). > > @@ -658,6 +660,27 @@ void file_set_fsnotify_mode(struct file *file) > file->f_mode |=3D FMODE_NONOTIFY | FMODE_NONOTIFY_PERM; > return; > } > + > + /* > + * OK, there are some pre-content watchers. Check if anybody can = be > + * watching for pre-content events on *this* file. > + */ > + mnt_mask =3D READ_ONCE(real_mount(file->f_path.mnt)->mnt_fsnotify= _mask); > + if (likely(!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED) && > + !fsnotify_object_watched(d_inode(dentry), mnt_mask, > + FSNOTIFY_PRE_CONTENT_EVENTS))) { > + file->f_mode |=3D FMODE_NONOTIFY | FMODE_NONOTIFY_PERM; > + return; > + } > + > + /* Even parent is not watching for pre-content events on this fil= e? */ > + parent =3D dget_parent(dentry); > + p_mask =3D fsnotify_inode_watches_children(d_inode(parent)); > + dput(parent); > + if (!(p_mask & FSNOTIFY_PRE_CONTENT_EVENTS)) { > + file->f_mode |=3D FMODE_NONOTIFY | FMODE_NONOTIFY_PERM; > + return; > + } > } > Nice! Note that I had a "hidden motive" for future optimization when I changed return value of fsnotify_object_watched() to a mask - I figured that while we are doing the checks above, we can check for the same price the mask ALL_FSNOTIFY_PERM_EVENTS then we get several answers for the same price: 1. Is the specific file watched by HSM? 2. Is the specific file watched by open permission events? 3. Is the specific file watched by post-open FAN_ACCESS_PERM? If the answers are No, No, No, we get some extra optimization in the (uncommon) use case that there are permission event watchers on some random inodes in the filesystem. If the answers are Yes, Yes, No, or No, Yes, No we can return a special value from file_set_fsnotify_mode() to indicate that permission events are needed ONLY for fsnotify_open_perm() hook, but not thereafter. This would implement the semantic change of "respect FAN_ACCESS_PERM only if it existed at open time" that can save a lot of unneeded cycles in the very hot read/write path, for example, when watcher only cares about FAN_OPEN_EXEC_PERM. I wasn't sure that any of this was worth the effort at this time, but just in case this gives you ideas of other useful optimizations we can do with the object combined marks_mask if we get it for free. Thanks, Amir.