| Message ID | 1474028371-21288-5-git-send-email-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri 16-09-16 14:19:23, Miklos Szeredi wrote: > From: Aihua Zhang <zhangaihua1@huawei.com> > > When an event occurs direct it to the overlay inode instead of the real > underlying inode. > > This will work even if the file was first on the lower layer and then > copied up, while the watch is there. This is because the watch is on the > overlay inode, which stays the same through the copy-up. > > For filesystems other than overlayfs this is a no-op, except for the > performance impact of an extra pointer dereferece. > > Verified to work correctly with the inotify/fanotify tests in LTP. > > Signed-off-by: Aihua Zhang <zhangaihua1@huawei.com> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > Cc: Jan Kara <jack@suse.cz> > Cc: Eric Paris <eparis@redhat.com> > --- > include/linux/fsnotify.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h > index eed9e853a06f..b8bcc058e031 100644 > --- a/include/linux/fsnotify.h > +++ b/include/linux/fsnotify.h > @@ -29,7 +29,11 @@ static inline int fsnotify_parent(struct path *path, struct dentry *dentry, __u3 > static inline int fsnotify_perm(struct file *file, int mask) > { > struct path *path = &file->f_path; > - struct inode *inode = file_inode(file); > + /* > + * Do not use file_inode() here or anywhere in this file to get the > + * inode. That would break *notity on overlayfs. > + */ > + struct inode *inode = path->dentry->d_inode; So shouldn't we rather have d_backing_inode(path->dentry) here and everywhere else? Honza
On Fri, Sep 16, 2016 at 7:38 PM, Jan Kara <jack@suse.cz> wrote: > On Fri 16-09-16 14:19:23, Miklos Szeredi wrote: >> From: Aihua Zhang <zhangaihua1@huawei.com> >> >> When an event occurs direct it to the overlay inode instead of the real >> underlying inode. >> >> This will work even if the file was first on the lower layer and then >> copied up, while the watch is there. This is because the watch is on the >> overlay inode, which stays the same through the copy-up. >> >> For filesystems other than overlayfs this is a no-op, except for the >> performance impact of an extra pointer dereferece. >> >> Verified to work correctly with the inotify/fanotify tests in LTP. >> >> Signed-off-by: Aihua Zhang <zhangaihua1@huawei.com> >> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> >> Cc: Jan Kara <jack@suse.cz> >> Cc: Eric Paris <eparis@redhat.com> >> --- >> include/linux/fsnotify.h | 14 +++++++++----- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h >> index eed9e853a06f..b8bcc058e031 100644 >> --- a/include/linux/fsnotify.h >> +++ b/include/linux/fsnotify.h >> @@ -29,7 +29,11 @@ static inline int fsnotify_parent(struct path *path, struct dentry *dentry, __u3 >> static inline int fsnotify_perm(struct file *file, int mask) >> { >> struct path *path = &file->f_path; >> - struct inode *inode = file_inode(file); >> + /* >> + * Do not use file_inode() here or anywhere in this file to get the >> + * inode. That would break *notity on overlayfs. >> + */ >> + struct inode *inode = path->dentry->d_inode; > > So shouldn't we rather have d_backing_inode(path->dentry) here and > everywhere else? d_backing_inode()'s intention is quite the opposite it's documented as "Get upper or lower inode we should be using" So there is no helper to return this "unreal" inode. In fact, it seems that was the intention of d_inode() helper, but it won't work for overlayfs. Miklos just submitted a new helper locks_inode() for 4.9, which does exactly that. I suppose it's either another similar helper fsnotify_inode() or find a better and generic name for this creature. IMO, the helper file_dentry(), that Miklos introduced in commit d101a1259 should be renamed to file_backing_dentry() and a new helper file_dentry() should be used here. i.e. inode = d_inode(file_dentry(file)); Also in this case, I think that an explicit name for the helper to get the "unreal" dentry would serve better then a generic name, but I can't think of a better name... Cheers, Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h index eed9e853a06f..b8bcc058e031 100644 --- a/include/linux/fsnotify.h +++ b/include/linux/fsnotify.h @@ -29,7 +29,11 @@ static inline int fsnotify_parent(struct path *path, struct dentry *dentry, __u3 static inline int fsnotify_perm(struct file *file, int mask) { struct path *path = &file->f_path; - struct inode *inode = file_inode(file); + /* + * Do not use file_inode() here or anywhere in this file to get the + * inode. That would break *notity on overlayfs. + */ + struct inode *inode = path->dentry->d_inode; __u32 fsnotify_mask = 0; int ret; @@ -173,7 +177,7 @@ static inline void fsnotify_mkdir(struct inode *inode, struct dentry *dentry) static inline void fsnotify_access(struct file *file) { struct path *path = &file->f_path; - struct inode *inode = file_inode(file); + struct inode *inode = path->dentry->d_inode; __u32 mask = FS_ACCESS; if (S_ISDIR(inode->i_mode)) @@ -191,7 +195,7 @@ static inline void fsnotify_access(struct file *file) static inline void fsnotify_modify(struct file *file) { struct path *path = &file->f_path; - struct inode *inode = file_inode(file); + struct inode *inode = path->dentry->d_inode; __u32 mask = FS_MODIFY; if (S_ISDIR(inode->i_mode)) @@ -209,7 +213,7 @@ static inline void fsnotify_modify(struct file *file) static inline void fsnotify_open(struct file *file) { struct path *path = &file->f_path; - struct inode *inode = file_inode(file); + struct inode *inode = path->dentry->d_inode; __u32 mask = FS_OPEN; if (S_ISDIR(inode->i_mode)) @@ -225,7 +229,7 @@ static inline void fsnotify_open(struct file *file) static inline void fsnotify_close(struct file *file) { struct path *path = &file->f_path; - struct inode *inode = file_inode(file); + struct inode *inode = path->dentry->d_inode; fmode_t mode = file->f_mode; __u32 mask = (mode & FMODE_WRITE) ? FS_CLOSE_WRITE : FS_CLOSE_NOWRITE;