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 9E73DD711BF for ; Wed, 20 Nov 2024 16:12:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 31B586B008A; Wed, 20 Nov 2024 11:12:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2CA5B6B0092; Wed, 20 Nov 2024 11:12:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 16BC76B0093; Wed, 20 Nov 2024 11:12:38 -0500 (EST) 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 E985D6B008A for ; Wed, 20 Nov 2024 11:12:37 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id A1BAF120AA3 for ; Wed, 20 Nov 2024 16:12:37 +0000 (UTC) X-FDA: 82806964758.10.E85CD03 Received: from mail-lj1-f180.google.com (mail-lj1-f180.google.com [209.85.208.180]) by imf30.hostedemail.com (Postfix) with ESMTP id 221EA80003 for ; Wed, 20 Nov 2024 16:10:57 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Lu4c5Ic9; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf30.hostedemail.com: domain of amir73il@gmail.com designates 209.85.208.180 as permitted sender) smtp.mailfrom=amir73il@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1732118971; 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=E0cgeSNXkJH2+E5FXlc5NSe4Q9H23sTP1cPn8NwPK4A=; b=MYEyvH25yPhcFYIFon6OmYlBU3AezGbJrcAB+rThHSAnC1UAkYqrnXoC0kNeK4YJJA+Trq rne5UTOBS/22qfnpdrymplvSsuwebkqvLCzZD3EqLJOfFTUhGe/fv3099b+MiLVLjy28vM fPmJ0DtPO1sADZ3uQUAx2yGA+mqjsPI= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Lu4c5Ic9; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf30.hostedemail.com: domain of amir73il@gmail.com designates 209.85.208.180 as permitted sender) smtp.mailfrom=amir73il@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732118971; a=rsa-sha256; cv=none; b=NvK8Cx8qHGgY5h4IJzCVRB/YTRjFoxKGli0rBS9RePG79Cwh9fRVko8LY2slYYwQzDEALg XMaDCgVe4+fdpqou059wzTgQPuNbhzPWSdVa2cquW3bIAUzyCg+9pWI5gaJGug+1obcja1 QmVKMhYjsIJevMUQbEZIKkL6AlelW6I= Received: by mail-lj1-f180.google.com with SMTP id 38308e7fff4ca-2feeb1e8edfso65069431fa.1 for ; Wed, 20 Nov 2024 08:12:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1732119154; x=1732723954; 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=E0cgeSNXkJH2+E5FXlc5NSe4Q9H23sTP1cPn8NwPK4A=; b=Lu4c5Ic9y6i4tHQgDf/ghvwelEMRRQzZaw6bcSr3TxhdpKsQypYQ7AMeRQ500bP4V/ tPQd4B8gGMi1bytuuMCsc2eolLd65hFbnI3iD6lxySbLw3yADDtuNuEaJE9k1DdGwmUY d2hjIJw/u/RsqDr2jgXyzNj2V4DcmFxnYZhqUCS4aezvapb9dARZLFwBra6pKpiVRkz1 BPN+jBkxlx32LhUv9cK+IR4OwucVLxjnwTuwS4+kQXvyfPTQZuZaeiUXGAyYawRL+eDk L2vHfy4NUce8v67NS16mUt9xTnUKxLrnxNRnHGQzHhMSHDNlAoVlep6hvdQivCL1gbth kjbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732119154; x=1732723954; 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=E0cgeSNXkJH2+E5FXlc5NSe4Q9H23sTP1cPn8NwPK4A=; b=tKV8A/dz2fVbDvQl0Gkrsbsao4fU2YVPvw9jlj7VRfyiuLlbOW4hZbTouqYQfMJ9W6 3sR12bb0dWES7JDYWqFUNMUcnhCksHYyIOIqCJ4QFyQaCejHnum4PThXkuar/Yl8Bhna 3Yq2lDH1WTGSARPDYbG44RoL4t5Z3yglECLGRmTGZ/tktvS62n5tgJz+XZQafEd+kuqk Tl0nkUyHjbCcyz4PT43eKrsvno/dNi4tWk9/wvIS4dQDQtpUiNM0/X6nbVKLBk3eQGoj Vj38LKAZMle7I5bIrVb4PLXZjEQa8GQP9jDgUitfdh/BP5x6kDoI1c8w+XvjIPEeFkA7 z/gA== X-Forwarded-Encrypted: i=1; AJvYcCUwkRD5DUgTu8+3wOo8cy4gc3yNmtbwOPvfN9RzTL297K+QhfF3+GjEHIFpWDtvmWLsGOp1TvOYVQ==@kvack.org X-Gm-Message-State: AOJu0YxEFyx6zwjt4Tfj09gib4l7A2TezqLHsvilvhpnnHKkpka3mMS3 /VL6o6WeetLgeACOHWisLEJaJkLQECv73JQ3rYD5zHCN+eiUBIHXJyThUvZwg7ioH19j+ANKuSj 4kwkmhN0V1g/Seek5pr+Sk5a0FGE= X-Google-Smtp-Source: AGHT+IF8f7v4a4sptq5ia4dSWBkD2+QG1/RGNLsS7h/kWKRgH7sx+NYjTqd9GraWx1v1b2aBXhIThMPt13OjiV/rAVI= X-Received: by 2002:a05:651c:221b:b0:2fb:6362:284e with SMTP id 38308e7fff4ca-2ff8db165bdmr29289991fa.8.1732119153173; Wed, 20 Nov 2024 08:12:33 -0800 (PST) MIME-Version: 1.0 References: <5ea5f8e283d1edb55aa79c35187bfe344056af14.1731684329.git.josef@toxicpanda.com> <20241120155309.lecjqqhohgcgyrkf@quack3> In-Reply-To: <20241120155309.lecjqqhohgcgyrkf@quack3> From: Amir Goldstein Date: Wed, 20 Nov 2024 17:12:21 +0100 Message-ID: Subject: Re: [PATCH v8 02/19] fsnotify: opt-in for permission events at file open time 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: rspam04 X-Rspamd-Queue-Id: 221EA80003 X-Stat-Signature: f8pw5uk7tsdi3sndapr655oqw7jmqtmu X-Rspam-User: X-HE-Tag: 1732119057-307580 X-HE-Meta: U2FsdGVkX1862dCILhmhDm5EbB98N9iw60W1XTq6AG0gKdz9zcYvoH/wJGye4PV/2+4XrTKEqmW8GxgrrtnJeOJB4B7hzTgLf8n57hP4AHJq3/JS7d6gnvKMapafAikQ7UjHu3h+y8jiEEFaPcFDb+T5SYPTqATekniBpRndA0+4GX4UR+EPLQ3mTPcRCuoIq/6ptJnIFVOSII+4Ws/l8lsDxef84H2IKmBcMfZQr9ah1wExa4AVCy1vWhpa0KR1SHa+oecUfBlFoA5v9qFumqDuW/366xQ8f0LBwHSJkegT2L0ncx968NLocwTZJDOCMry6w5Wr4H6ps4wscER+AylqqzRlPwU7E8MYkqaZa83WrWsKruDpkuRa6QjbgDM5Awraizdip+AlWNifW0Gc/FpsGJ06rwlJo46afonMfPaXyzI18ZFwjf53a3Sb7FZGtR9n99DV0A6G8bUhcULOWuA4mpzOrj8U7FwpnO2XU0JEACH7Vb1M+s+QI6IjBxK1h04EMiPw7EBGIBJ/h/+oP1VZsuvRlT05ubJ71daRX0bplDQgghuP/R7jBrl5cgdeKnOxp23FTF5WdRfIu55RYZudsyqGHCEf7PBvNRtSHjGAhE5v2vtqKFQ/9SD+A1wgaz9SxC2COROh2BtBUZ56bkvLEB09Fc7upfbdEwxwwzvSdjMq/AsMpHa4FvO1hbTkW1SHo2VlmlvmnjLK6w5MaVkvsMTBD5IpoUyHRPlG3ig4bQRg1WAiG6FnvxhJ0RHMEbTrG3ECtNNz8fSsSfsNOEn4TTfwioN/D4X77btUxkrLKR4xVyR3ZoiHA9runaydNF7jL+e7pwFydqF/GG9c2cqSwZ0z+paBTau7O+WN7OF8FrL71dSGP/BlM//9iENAtYfb8NxX5RR/vKQTWx3WmxYcBsMQab7uzTNj0Q0Cy/P+akbVrRKCzR4wxbBp0DbM4S4cfLdaSxru35wxRut JD9+j6LF RUMVTPBAtwBqQZwI6eXMofRvICsFZ+wgX9WYXL2aMMNtvB23AK/qEi20965x3tZ8hH2VjWhw8hDco16N7fn7WYnnfCB4CG+sWKDJUdvb1GOHEZgHmtdOdj0vKBqKL5CzphAYKcsS+1WN8rEW3NUFCnLg1Ts5DPscpfw/QI+i7c0fVhnybvmhqjClA4V5PWJYx5rRlSwU7o/78CIfhpWMMo0RxFYSHG+/T54PC5yrd+ugCMwlq2Rx0w/dp1vdqXkuBSv73V+qYppHxSa9a5Se5OpF+ICapLgf0WhhsvL01tpjX1Y4jg1sEAr3G+u7u/niDyeNHq/feLn1EVUVhTYGh284s41Zoz3l8ek6DOcvpOZpPOlTdUBZ8DKu+VOZ4GUcu4TUEqHcJAO2I6ZDZ68d8gYtWbRWV+g6f4PHBETY083c4ef8= 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 4:53=E2=80=AFPM Jan Kara wrote: > > On Fri 15-11-24 10:30:15, Josef Bacik wrote: > > From: Amir Goldstein > > > > Legacy inotify/fanotify listeners can add watches for events on inode, > > parent or mount and expect to get events (e.g. FS_MODIFY) on files that > > were already open at the time of setting up the watches. > > > > fanotify permission events are typically used by Anti-malware sofware, > > that is watching the entire mount and it is not common to have more tha= t > > one Anti-malware engine installed on a system. > > > > To reduce the overhead of the fsnotify_file_perm() hooks on every file > > access, relax the semantics of the legacy FAN_ACCESS_PERM event to gene= rate > > events only if there were *any* permission event listeners on the > > filesystem at the time that the file was opened. > > > > The new semantic is implemented by extending the FMODE_NONOTIFY bit int= o > > two FMODE_NONOTIFY_* bits, that are used to store a mode for which of t= he > > events types to report. > > > > This is going to apply to the new fanotify pre-content events in order > > to reduce the cost of the new pre-content event vfs hooks. > > > > Suggested-by: Linus Torvalds > > Link: https://lore.kernel.org/linux-fsdevel/CAHk-=3Dwj8L=3DmtcRTi=3DNEC= HMGfZQgXOp_uix1YVh04fEmrKaMnXA@mail.gmail.com/ > > Signed-off-by: Amir Goldstein > > FWIW I've ended up somewhat massaging this patch (see below). > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > index 23bd058576b1..8e5c783013d2 100644 > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -173,13 +173,14 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, lo= ff_t offset, > > > > #define FMODE_NOREUSE ((__force fmode_t)(1 << 23)) > > > > -/* FMODE_* bit 24 */ > > - > > /* File is embedded in backing_file object */ > > -#define FMODE_BACKING ((__force fmode_t)(1 << 25)) > > +#define FMODE_BACKING ((__force fmode_t)(1 << 24)) > > > > -/* File was opened by fanotify and shouldn't generate fanotify events = */ > > -#define FMODE_NONOTIFY ((__force fmode_t)(1 << 26)) > > +/* File shouldn't generate fanotify pre-content events */ > > +#define FMODE_NONOTIFY_HSM ((__force fmode_t)(1 << 25)) > > + > > +/* File shouldn't generate fanotify permission events */ > > +#define FMODE_NONOTIFY_PERM ((__force fmode_t)(1 << 26)) > > Firstly, I've kept FMODE_NONOTIFY to stay a single bit instead of two bit > constant. I've seen too many bugs caused by people expecting the constant > has a single bit set when it actually had more in my life. So I've ended = up > with: > > +/* > + * Together with FMODE_NONOTIFY_PERM defines which fsnotify events shoul= dn't be > + * generated (see below) > + */ > +#define FMODE_NONOTIFY ((__force fmode_t)(1 << 25)) > + > +/* > + * Together with FMODE_NONOTIFY defines which fsnotify events shouldn't = be > + * generated (see below) > + */ > +#define FMODE_NONOTIFY_PERM ((__force fmode_t)(1 << 26)) > > and > > +/* > + * The two FMODE_NONOTIFY* define which fsnotify events should not be ge= nerated > + * for a file. These are the possible values of (f->f_mode & > + * FMODE_FSNOTIFY_MASK) and their meaning: > + * > + * FMODE_NONOTIFY - suppress all (incl. non-permission) events. > + * FMODE_NONOTIFY_PERM - suppress permission (incl. pre-content) events. > + * FMODE_NONOTIFY | FMODE_NONOTIFY_PERM - suppress only pre-content even= ts. > + */ > +#define FMODE_FSNOTIFY_MASK \ > + (FMODE_NONOTIFY | FMODE_NONOTIFY_PERM) > + > +#define FMODE_FSNOTIFY_NONE(mode) \ > + ((mode & FMODE_FSNOTIFY_MASK) =3D=3D FMODE_NONOTIFY) > +#define FMODE_FSNOTIFY_PERM(mode) \ > + (!(mode & FMODE_NONOTIFY_PERM)) That looks incorrect - It gives the wrong value for FMODE_NONOTIFY | FMODE_NONOTIFY_PERM should be: !=3D FMODE_NONOTIFY_PERM && !=3D FMODE_NONOTIFY The simplicity of the single bit test is for permission events is why I chose my model, but I understand your reasoning. > +#define FMODE_FSNOTIFY_HSM(mode) \ > + ((mode & FMODE_FSNOTIFY_MASK) =3D=3D 0) > > Also I've moved file_set_fsnotify_mode() out of line into fsnotify.c. The > function gets quite big and the call is not IMO so expensive to warrant > inlining. Furthermore it saves exporting some fsnotify internals to modul= es > (in later patches). Sounds good. Since you wanted to refrain from defining a two bit constant, I wonder how you annotated for NONOTIFY_HSM case return FMODE_NONOTIFY | FMODE_NONOTIFY_PERM; Thanks, Amir.