[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-unionfs
Subject:    Re: [PATCH] fsnotify: do not generate duplicate events for "fake" path
From:       Amir Goldstein <amir73il () gmail ! com>
Date:       2019-04-24 16:17:23
Message-ID: CAOQ4uxi+i1_MBwgh0FEsi0Q_kLfxkJwzwKoAafEyxE4yQeAZkg () mail ! gmail ! com
[Download RAW message or body]

On Wed, Apr 24, 2019 at 6:57 PM Jan Kara <jack@suse.cz> wrote:
>
> 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 <jencce.kernel@gmail.com>
> > Fixes: d1d04ef8572b ("ovl: stack file ops")
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> 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.
>

That is correct.

> 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.

Not sure. I think the use case is FAN_MARK_FILESYSTEM watch from
host by Anti-Virus software to protect host from containers.
Yes, host can watch all container overlayfs mounts, but that is less scalable.

> 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...
>

So it seems that before 573e1784817c Revert "fsnotify: support overlayfs"
no fanotify events were generated on underlying regular inodes.
Overlayfs uses private cloned mounts to access real inodes, so marking
underlying mounts was never going to produce any events.
Overlayfs does open underlying directory files, so events on underlying
directories would be generated (I think) even before v4.19.

inotify and new fanotify directory modification events would also be
generated from overlayfs, but we do not have a "fake" path issue with
those.

The bottom line is that it is probably best not to report any events
originating from overlayfs file with "fake" path and the simplest way to
achieve that would be to open the overlayfs realfile with
FMODE_NONOTIFY. At first I though doing that may cause regressions
from pre v4.19, but I was wrong. It would be the same behavior as pre v4.19.

I'll go write the patch...

Thanks,
Amir.
[prev in list] [next in list] [prev in thread] [next in thread]