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 36BF0C282E1 for ; Wed, 24 Apr 2019 15:57:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 09D14208E4 for ; Wed, 24 Apr 2019 15:57:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732050AbfDXP54 (ORCPT ); Wed, 24 Apr 2019 11:57:56 -0400 Received: from mx2.suse.de ([195.135.220.15]:51470 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1730625AbfDXP5z (ORCPT ); Wed, 24 Apr 2019 11:57:55 -0400 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 ACF96AE33; Wed, 24 Apr 2019 15:57:53 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 210481E2F36; Wed, 24 Apr 2019 17:57:53 +0200 (CEST) Date: Wed, 24 Apr 2019 17:57:53 +0200 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Murphy Zhou , Miklos Szeredi , linux-fsdevel@vger.kernel.org, linux-unionfs@vger.kernel.org Subject: Re: [PATCH] fsnotify: do not generate duplicate events for "fake" path Message-ID: <20190424155753.GB29968@quack2.suse.cz> References: <20190424100951.16678-1-amir73il@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190424100951.16678-1-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 Hi Amir! On Wed 24-04-19 13:09:51, Amir Goldstein wrote: > Overlayfs "fake" path is used for stacked file operations on > underlying files. Operations on files with "fake" path must not > generate events on mount marks and on parent watches, because > those events have already been generated at overlayfs layer. > > The reported event->fd for inode/sb marks will have the wrong path > (overlayfs path), but we have no choice but to report them anyway. > > Link: https://lore.kernel.org/linux-fsdevel/20190423065024.12695-1-jencce.kernel@gmail.com/ > Reported-by: Murphy Zhou > Fixes: d1d04ef8572b ("ovl: stack file ops") > Signed-off-by: Amir Goldstein So honestly I don't quite like that fsnotify core has to care about peculiarities of overlayfs. And I'm not sure I fully understand what the problem actually is so let me try to summarize here: With write to overlayfs, we generate event both for inode on underlying filesystem (upper inode; this gets generated by vfs_write()->ovl_write_iter()->do_iter_write()) and also for "virtual" overlayfs inode (generated directly by vfs_write()). Now for overlayfs inode the (mnt, dentry) pair is a valid one - the one used for opening the file. For upper inode you say we don't use proper path (mnt, dentry) pair but some made-up one - looks like the original overlayfs path if I read the code right. So fsnotify will get two fsnotify_modify() calls, both with the same file->f_path but with different file->f_inode. So no surprise mntpoint / path based stuff is getting confused by this. I guess the first question that comes to my mind is: Is fsnotify on 'upper' inodes actually useful? On overlayfs as a whole it makes sense but on individual filesystems I'm not so sure. And here we can see that mountpoint watches are going to have hard times, some events (e.g. open / close) do not seem to be generated at all, not sure if directory events are generated... Honza > --- > > Jan, > > This is a slightly simplified and cleaner version of the patch I posted > yesterday on the linked bug report thread. > > I have posted an additional overlayfs test case to fanotify06 on LTP > list. > > Thanks, > Amir. > > fs/notify/fsnotify.c | 3 ++- > include/linux/fsnotify.h | 23 ++++++++++++++++++++--- > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c > index df06f3da166c..6f752b13e3fd 100644 > --- a/fs/notify/fsnotify.c > +++ b/fs/notify/fsnotify.c > @@ -334,7 +334,8 @@ int fsnotify(struct inode *to_tell, __u32 mask, const void *data, int data_is, > int ret = 0; > __u32 test_mask = (mask & ALL_FSNOTIFY_EVENTS); > > - if (data_is == FSNOTIFY_EVENT_PATH) { > + if (data_is == FSNOTIFY_EVENT_PATH && > + !fsnotify_is_fake_path(to_tell, data)) { > mnt = real_mount(((const struct path *)data)->mnt); > mnt_or_sb_mask |= mnt->mnt_fsnotify_mask; > } > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h > index 09587e2860b5..c51f57d3a025 100644 > --- a/include/linux/fsnotify.h > +++ b/include/linux/fsnotify.h > @@ -40,6 +40,20 @@ static inline int fsnotify_parent(const struct path *path, > return __fsnotify_parent(path, dentry, mask); > } > > +/* > + * Overlayfs "fake" path is used for stacked file operations on underlying > + * files. file->f_path is from overlayfs layer and file->f_inode is from > + * underlying layer. We must not generate events on mount and on parent > + * based on fake path, because those events have already been generated at > + * overlayfs layer. The reported event->fd for inode/sb marks will have the > + * wrong path (overlayfs path), but we have no choice but to report them as is. > + */ > +static inline bool fsnotify_is_fake_path(struct inode *inode, > + const struct path *path) > +{ > + return unlikely(inode->i_sb != path->dentry->d_sb); > +} > + > /* > * Simple wrapper to consolidate calls fsnotify_parent()/fsnotify() when > * an event is on a path. > @@ -47,10 +61,13 @@ static inline int fsnotify_parent(const struct path *path, > static inline int fsnotify_path(struct inode *inode, const struct path *path, > __u32 mask) > { > - int ret = fsnotify_parent(path, NULL, mask); > + if (!fsnotify_is_fake_path(inode, path)) { > + int ret = fsnotify_parent(path, NULL, mask); > + > + if (ret) > + return ret; > + } > > - if (ret) > - return ret; > return fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0); > } > > -- > 2.17.1 > -- Jan Kara SUSE Labs, CR