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 X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF45CC169C4 for ; Wed, 6 Feb 2019 14:14:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A8D9820844 for ; Wed, 6 Feb 2019 14:14:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727132AbfBFOOT (ORCPT ); Wed, 6 Feb 2019 09:14:19 -0500 Received: from mx2.suse.de ([195.135.220.15]:47074 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726864AbfBFOOT (ORCPT ); Wed, 6 Feb 2019 09:14:19 -0500 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id D7274AFBE; Wed, 6 Feb 2019 14:14:17 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 7898F1E41EE; Wed, 6 Feb 2019 15:14:17 +0100 (CET) Date: Wed, 6 Feb 2019 15:14:17 +0100 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Matthew Bobrowski , linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v5 01/17] fsnotify: annotate directory entry modification events Message-ID: <20190206141417.GA28837@quack2.suse.cz> References: <20190110170444.30616-1-amir73il@gmail.com> <20190110170444.30616-2-amir73il@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190110170444.30616-2-amir73il@gmail.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Thu 10-01-19 19:04:28, Amir Goldstein wrote: > "dirent" events are referring to events that modify directory entries, > such as create,delete,rename. Those events should always be reported > on a watched directory, regardless if FS_EVENT_ON_CHILD is set > on the watch mask. > > fsnotify_nameremove() and fsnotify_move() were modified to no longer > set the FS_EVENT_ON_CHILD event bit. This is a semantic change to > align with the "dirent" event definition. It has no effect on any > existing backend, because dnotify, inotify and audit always requets the > child events and fanotify does not get the delete,rename events. > > The fsnotify_dirent() helper is used instead of fsnotify_parent() to > report a dirent event to dentry->d_parent without FS_EVENT_ON_CHILD > and regardless if parent has the FS_EVENT_ON_CHILD bit set. > > Unlike fsnotify_parent(), fsnotify_dirent() assumes that dentry->d_name > and dentry->d_parent are stable. For fsnotify_create()/fsnotify_mkdir(), > this assumption is abviously correct. For fsnotify_nameremove(), it is > less trvial, so we use dget_parent() and take_dentry_name_snapshot() to > grab stable references. > > Signed-off-by: Amir Goldstein I've taken the liberty of removing the case analysis from fsnotify_nameremove() comment. That is bound to become out of date pretty soon and not very useful for the reader. Otherwise the patch looks good and I've added it to my tree. Honza > --- > include/linux/fsnotify.h | 68 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 59 insertions(+), 9 deletions(-) > > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h > index 2ccb08cb5d6a..116907928c7f 100644 > --- a/include/linux/fsnotify.h > +++ b/include/linux/fsnotify.h > @@ -17,8 +17,22 @@ > #include > #include > > +/* > + * Notify this @dir inode about a change in the directory entry @dentry. > + * > + * Unlike fsnotify_parent(), the event will be reported regardless of the > + * FS_EVENT_ON_CHILD mask on the parent inode. > + */ > +static inline int fsnotify_dirent(struct inode *dir, struct dentry *dentry, > + __u32 mask) > +{ > + return fsnotify(dir, mask, d_inode(dentry), FSNOTIFY_EVENT_INODE, > + dentry->d_name.name, 0); > +} > + > /* Notify this dentry's parent about a child's events. */ > -static inline int fsnotify_parent(const struct path *path, struct dentry *dentry, __u32 mask) > +static inline int fsnotify_parent(const struct path *path, > + struct dentry *dentry, __u32 mask) > { > if (!dentry) > dentry = path->dentry; > @@ -85,8 +99,8 @@ static inline void fsnotify_move(struct inode *old_dir, struct inode *new_dir, > { > struct inode *source = moved->d_inode; > u32 fs_cookie = fsnotify_get_cookie(); > - __u32 old_dir_mask = (FS_EVENT_ON_CHILD | FS_MOVED_FROM); > - __u32 new_dir_mask = (FS_EVENT_ON_CHILD | FS_MOVED_TO); > + __u32 old_dir_mask = FS_MOVED_FROM; > + __u32 new_dir_mask = FS_MOVED_TO; > const unsigned char *new_name = moved->d_name.name; > > if (old_dir == new_dir) > @@ -128,15 +142,54 @@ static inline void fsnotify_vfsmount_delete(struct vfsmount *mnt) > > /* > * fsnotify_nameremove - a filename was removed from a directory > + * > + * Called from d_delete() and nfs_complete_sillyrename(). > + * The latter is called from nfs client ->unlink() ->rmdir() ->rename() > + * under parent vfs inode lock. > + * > + * Most filesystems call d_delete() from ->unlink() ->rmdir() ->rename() > + * ops under parent vfs inode lock. > + * > + * Some pseudo filesystems call d_delete() without parent inode lock. > + * Those filesystems have no ->rename() op and they do not call > + * d_move() directly, so d_parent and d_name are stable by definition. > + * Examples: devpts, efivarfs, rpc_pipefs, functionfs. > + * > + * Some clustered filesystems call d_delete() on remote nodes, not under > + * vfs parent inode lock, but they use cluster distributed locks on local > + * and remote nodes. Those filesystems call d_delete() under their cluster > + * lock. Examples: > + * - in ceph_fill_trace() under CEPH_MDS_R_PARENT_LOCKED > + * - in ocfs2_dentry_convert_worker() under ocfs2_dentry_lock > + * But those filesystems also call d_move() under the same cluster lock > + * (i.e. FS_RENAME_DOES_D_MOVE), so d_parent and d_name are also stable. > + * > + * However, to be on the safe side and be reselient to future callers > + * and out of tree users of d_delete(), we do not assume that d_parent and > + * d_name are stable and we use dget_parent() and take_dentry_name_snapshot() > + * to grab stable references. > */ > static inline void fsnotify_nameremove(struct dentry *dentry, int isdir) > { > + struct dentry *parent; > + struct name_snapshot name; > __u32 mask = FS_DELETE; > > + /* d_delete() of pseudo inode? (e.g. __ns_get_path() playing tricks) */ > + if (IS_ROOT(dentry)) > + return; > + > if (isdir) > mask |= FS_ISDIR; > > - fsnotify_parent(NULL, dentry, mask); > + parent = dget_parent(dentry); > + take_dentry_name_snapshot(&name, dentry); > + > + fsnotify(d_inode(parent), mask, d_inode(dentry), FSNOTIFY_EVENT_INODE, > + name.name, 0); > + > + release_dentry_name_snapshot(&name); > + dput(parent); > } > > /* > @@ -155,7 +208,7 @@ static inline void fsnotify_create(struct inode *inode, struct dentry *dentry) > { > audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE); > > - fsnotify(inode, FS_CREATE, dentry->d_inode, FSNOTIFY_EVENT_INODE, dentry->d_name.name, 0); > + fsnotify_dirent(inode, dentry, FS_CREATE); > } > > /* > @@ -176,12 +229,9 @@ static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct > */ > static inline void fsnotify_mkdir(struct inode *inode, struct dentry *dentry) > { > - __u32 mask = (FS_CREATE | FS_ISDIR); > - struct inode *d_inode = dentry->d_inode; > - > audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE); > > - fsnotify(inode, mask, d_inode, FSNOTIFY_EVENT_INODE, dentry->d_name.name, 0); > + fsnotify_dirent(inode, dentry, FS_CREATE | FS_ISDIR); > } > > /* > -- > 2.17.1 > -- Jan Kara SUSE Labs, CR