* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree [not found] <152086697777252@kroah.com> @ 2018-07-05 15:11 ` Rafael David Tinoco 2018-07-05 15:18 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Rafael David Tinoco @ 2018-07-05 15:11 UTC (permalink / raw) To: ltp * INFORMATIONAL ONLY - NO ACTION/READING NEEDED BUG: https://bugs.linaro.org/show_bug.cgi?id=3881 Because the lack of the following commit: commit 764baba80168ad3adafb521d2ab483ccbc49e344 Author: Amir Goldstein <amir73il@gmail.com> Date: Sun Feb 4 15:35:09 2018 +0200 ovl: hash non-dir by lower inode for fsnotify INFO: inotify issue with non-dir non-upper files in overlayfs exists in LTS <= v4.14. INFO: LTP inotify08 test fails on * v4.14 and bellow * and should be skipped. Trying to cherry-pick to v4.14 has proven not to be feasible without bringing many new features/code into LTS tree (up to any Linux distribution to decide to backport or not). For clean cherry picks, commits: ovl: hash non-dir by lower inode for fsnotify ovl: hash non-indexed dir by upper inode for NFS export ovl: do not pass overlay dentry to ovl_get_inode() ovl: hash directory inodes for fsnotify ovl: no direct iteration for dir with origin xattr Revert "ovl: hash directory inodes for fsnotify" are needed AND all the logic for setting up "origin" variable in ovl_lookup, passed to ovl_lookup_index() after it got its prototype changed, would still be missing. Unfortunately the "origin" also depends on commit 051224438 ("ovl: generalize ovl_verify_origin() and helpers") and some others ones that refactor many functions. -Rafael On Mon, Mar 12, 2018@12:02 PM <gregkh@linuxfoundation.org> wrote: > > > The patch below does not apply to the 4.14-stable tree. > If someone wants it applied there, or to any other stable or longterm > tree, then please email the backport, including the original git commit > id to <stable@vger.kernel.org>. > > thanks, > > greg k-h > > ------------------ original commit in Linus's tree ------------------ > > From 764baba80168ad3adafb521d2ab483ccbc49e344 Mon Sep 17 00:00:00 2001 > From: Amir Goldstein <amir73il@gmail.com> > Date: Sun, 4 Feb 2018 15:35:09 +0200 > Subject: [PATCH] ovl: hash non-dir by lower inode for fsnotify > > Commit 31747eda41ef ("ovl: hash directory inodes for fsnotify") > fixed an issue of inotify watch on directory that stops getting > events after dropping dentry caches. > > A similar issue exists for non-dir non-upper files, for example: > > $ mkdir -p lower upper work merged > $ touch lower/foo > $ mount -t overlay -o > lowerdir=lower,workdir=work,upperdir=upper none merged > $ inotifywait merged/foo & > $ echo 2 > /proc/sys/vm/drop_caches > $ cat merged/foo > > inotifywait doesn't get the OPEN event, because ovl_lookup() called > from 'cat' allocates a new overlay inode and does not reuse the > watched inode. > > Fix this by hashing non-dir overlay inodes by lower real inode in > the following cases that were not hashed before this change: > - A non-upper overlay mount > - A lower non-hardlink when index=off > > A helper ovl_hash_bylower() was added to put all the logic and > documentation about which real inode an overlay inode is hashed by > into one place. > > The issue dates back to initial version of overlayfs, but this > patch depends on ovl_inode code that was introduced in kernel v4.13. > > Cc: <stable@vger.kernel.org> #v4.13 > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> > > diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c > index fcd97b783fa1..3b1bd469accd 100644 > --- a/fs/overlayfs/inode.c > +++ b/fs/overlayfs/inode.c > @@ -669,38 +669,59 @@ struct inode *ovl_lookup_inode(struct super_block *sb, struct dentry *real, > return inode; > } > > +/* > + * Does overlay inode need to be hashed by lower inode? > + */ > +static bool ovl_hash_bylower(struct super_block *sb, struct dentry *upper, > + struct dentry *lower, struct dentry *index) > +{ > + struct ovl_fs *ofs = sb->s_fs_info; > + > + /* No, if pure upper */ > + if (!lower) > + return false; > + > + /* Yes, if already indexed */ > + if (index) > + return true; > + > + /* Yes, if won't be copied up */ > + if (!ofs->upper_mnt) > + return true; > + > + /* No, if lower hardlink is or will be broken on copy up */ > + if ((upper || !ovl_indexdir(sb)) && > + !d_is_dir(lower) && d_inode(lower)->i_nlink > 1) > + return false; > + > + /* No, if non-indexed upper with NFS export */ > + if (sb->s_export_op && upper) > + return false; > + > + /* Otherwise, hash by lower inode for fsnotify */ > + return true; > +} > + > struct inode *ovl_get_inode(struct super_block *sb, struct dentry *upperdentry, > struct dentry *lowerdentry, struct dentry *index, > unsigned int numlower) > { > - struct ovl_fs *ofs = sb->s_fs_info; > struct inode *realinode = upperdentry ? d_inode(upperdentry) : NULL; > struct inode *inode; > - /* Already indexed or could be indexed on copy up? */ > - bool indexed = (index || (ovl_indexdir(sb) && !upperdentry)); > - struct dentry *origin = indexed ? lowerdentry : NULL; > + bool bylower = ovl_hash_bylower(sb, upperdentry, lowerdentry, index); > bool is_dir; > > - if (WARN_ON(upperdentry && indexed && !lowerdentry)) > - return ERR_PTR(-EIO); > - > if (!realinode) > realinode = d_inode(lowerdentry); > > /* > - * Copy up origin (lower) may exist for non-indexed non-dir upper, but > - * we must not use lower as hash key in that case. > - * Hash non-dir that is or could be indexed by origin inode. > - * Hash dir that is or could be merged by origin inode. > - * Hash pure upper and non-indexed non-dir by upper inode. > - * Hash non-indexed dir by upper inode for NFS export. > + * Copy up origin (lower) may exist for non-indexed upper, but we must > + * not use lower as hash key if this is a broken hardlink. > */ > is_dir = S_ISDIR(realinode->i_mode); > - if (is_dir && (indexed || !sb->s_export_op || !ofs->upper_mnt)) > - origin = lowerdentry; > - > - if (upperdentry || origin) { > - struct inode *key = d_inode(origin ?: upperdentry); > + if (upperdentry || bylower) { > + struct inode *key = d_inode(bylower ? lowerdentry : > + upperdentry); > unsigned int nlink = is_dir ? 1 : realinode->i_nlink; > > inode = iget5_locked(sb, (unsigned long) key, > @@ -728,6 +749,7 @@ struct inode *ovl_get_inode(struct super_block *sb, struct dentry *upperdentry, > nlink = ovl_get_nlink(lowerdentry, upperdentry, nlink); > set_nlink(inode, nlink); > } else { > + /* Lower hardlink that will be broken on copy up */ > inode = new_inode(sb); > if (!inode) > goto out_nomem; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree 2018-07-05 15:11 ` [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree Rafael David Tinoco @ 2018-07-05 15:18 ` Greg KH 2018-07-05 15:27 ` Rafael David Tinoco 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2018-07-05 15:18 UTC (permalink / raw) To: ltp On Thu, Jul 05, 2018 at 12:11:53PM -0300, Rafael David Tinoco wrote: > * INFORMATIONAL ONLY - NO ACTION/READING NEEDED So what am I supposed to do with this? Why post something that no one needs to even read? confused, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree 2018-07-05 15:18 ` Greg KH @ 2018-07-05 15:27 ` Rafael David Tinoco 2018-07-05 15:30 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Rafael David Tinoco @ 2018-07-05 15:27 UTC (permalink / raw) To: ltp v4.14 and below have the bug. I thought about, at least, documenting it here. Disclaimer tried to reduce the noise for you (and I removed you from To:), but it clearly did not work :\. Should I document here known bugs, discovered by testing, and aren't fixed in LTS ? If not, I won't for the next ones. On Thu, Jul 5, 2018 at 12:18 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Thu, Jul 05, 2018 at 12:11:53PM -0300, Rafael David Tinoco wrote: > > * INFORMATIONAL ONLY - NO ACTION/READING NEEDED > > So what am I supposed to do with this? Why post something that no one > needs to even read? > > confused, > > greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree 2018-07-05 15:27 ` Rafael David Tinoco @ 2018-07-05 15:30 ` Greg KH 2018-07-05 15:53 ` Rafael David Tinoco 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2018-07-05 15:30 UTC (permalink / raw) To: ltp On Thu, Jul 05, 2018 at 12:27:05PM -0300, Rafael David Tinoco wrote: > v4.14 and below have the bug. What bug? Please never top-post, you loose all context :( greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree 2018-07-05 15:30 ` Greg KH @ 2018-07-05 15:53 ` Rafael David Tinoco 2018-07-05 15:57 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Rafael David Tinoco @ 2018-07-05 15:53 UTC (permalink / raw) To: ltp > What bug? Please never top-post, you loose all context :( Won't happen again. Summary is this: commit 764baba80168ad3adafb521d2ab483ccbc49e344 Author: Amir Goldstein <amir73il@gmail.com> Date: Sun Feb 4 15:35:09 2018 +0200 ovl: hash non-dir by lower inode for fsnotify INFO: inotify issue with non-dir non-upper files in overlayfs exists in LTS <= v4.14. INFO: LTP inotify08 test fails on * v4.14 and bellow * and should be skipped. And message was informative only (clearly didn't work). Either way, do you think it's worth informing existing LTS bugs, found by test tooling, here ? ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree 2018-07-05 15:53 ` Rafael David Tinoco @ 2018-07-05 15:57 ` Greg KH 2018-07-05 16:15 ` Rafael David Tinoco 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2018-07-05 15:57 UTC (permalink / raw) To: ltp On Thu, Jul 05, 2018 at 12:53:24PM -0300, Rafael David Tinoco wrote: > > What bug? Please never top-post, you loose all context :( > > Won't happen again. > > Summary is this: > > commit 764baba80168ad3adafb521d2ab483ccbc49e344 > Author: Amir Goldstein <amir73il@gmail.com> > Date: Sun Feb 4 15:35:09 2018 +0200 > > ovl: hash non-dir by lower inode for fsnotify > > INFO: inotify issue with non-dir non-upper files in overlayfs exists > in LTS <= v4.14. > INFO: LTP inotify08 test fails on * v4.14 and bellow * and should be skipped. > > And message was informative only (clearly didn't work). Either way, do > you think it's worth informing existing LTS bugs, found by test > tooling, here ? Why can't we fix those bugs in the stable kernel releases? Is it too difficult to do so? thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree 2018-07-05 15:57 ` Greg KH @ 2018-07-05 16:15 ` Rafael David Tinoco 2018-07-05 16:32 ` Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Rafael David Tinoco @ 2018-07-05 16:15 UTC (permalink / raw) To: ltp > > commit 764baba80168ad3adafb521d2ab483ccbc49e344 > > Author: Amir Goldstein <amir73il@gmail.com> > > Date: Sun Feb 4 15:35:09 2018 +0200 > > > > ovl: hash non-dir by lower inode for fsnotify > > > > INFO: inotify issue with non-dir non-upper files in overlayfs exists > > in LTS <= v4.14. > > INFO: LTP inotify08 test fails on * v4.14 and bellow * and should be skipped. > > > > And message was informative only (clearly didn't work). Either way, do > > you think it's worth informing existing LTS bugs, found by test > > tooling, here ? > > Why can't we fix those bugs in the stable kernel releases? Is it too > difficult to do so? For this inotify bug: Commits ovl: hash non-dir by lower inode for fsnotify ovl: hash non-indexed dir by upper inode for NFS export ovl: do not pass overlay dentry to ovl_get_inode() ovl: hash directory inodes for fsnotify ovl: no direct iteration for dir with origin xattr Revert "ovl: hash directory inodes for fsnotify" are needed AND all the logic for setting up "origin" variable in ovl_lookup, passed to ovl_lookup_index() after it got its prototype changed, would still be missing (and other refactoring changes, commits splitting functions and so on). So I assumed it was a no-go. There is also another bug: https://bugs.linaro.org/show_bug.cgi?id=3303. Fanotify faces a srcu dead-lock when userland stops responding to events for this other case. Fix for that bug is a 35 patches patchset (including the fix, commit 9dd813c15b2c101, for the particular issue). Question is, should I document things of this nature on this list also ? Even if it is likely a no-go for the backports ? Just as information ? Should I just bring the attention to the backport need (all patches) and you decide ? Tks -Rafael ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree 2018-07-05 16:15 ` Rafael David Tinoco @ 2018-07-05 16:32 ` Greg KH 2018-07-05 16:41 ` Rafael David Tinoco 0 siblings, 1 reply; 9+ messages in thread From: Greg KH @ 2018-07-05 16:32 UTC (permalink / raw) To: ltp On Thu, Jul 05, 2018 at 01:15:43PM -0300, Rafael David Tinoco wrote: > > > commit 764baba80168ad3adafb521d2ab483ccbc49e344 > > > Author: Amir Goldstein <amir73il@gmail.com> > > > Date: Sun Feb 4 15:35:09 2018 +0200 > > > > > > ovl: hash non-dir by lower inode for fsnotify > > > > > > INFO: inotify issue with non-dir non-upper files in overlayfs exists > > > in LTS <= v4.14. > > > INFO: LTP inotify08 test fails on * v4.14 and bellow * and should be skipped. > > > > > > And message was informative only (clearly didn't work). Either way, do > > > you think it's worth informing existing LTS bugs, found by test > > > tooling, here ? > > > > Why can't we fix those bugs in the stable kernel releases? Is it too > > difficult to do so? > > For this inotify bug: > > Commits > > ovl: hash non-dir by lower inode for fsnotify > ovl: hash non-indexed dir by upper inode for NFS export > ovl: do not pass overlay dentry to ovl_get_inode() > ovl: hash directory inodes for fsnotify > ovl: no direct iteration for dir with origin xattr > Revert "ovl: hash directory inodes for fsnotify" > > are needed AND all the logic for setting up "origin" variable in > ovl_lookup, passed to ovl_lookup_index() after it got its prototype > changed, would still be missing (and other refactoring changes, > commits splitting functions and so on). > > So I assumed it was a no-go. It all depends, let's get the git commit ids for these please. And have you successfully applied and tested that those patches fix the issue? If so, great, let's apply them! > There is also another bug: > > https://bugs.linaro.org/show_bug.cgi?id=3303. > > Fanotify faces a srcu dead-lock when userland stops responding to > events for this other case. Fix for that bug is a 35 patches patchset > (including the fix, commit 9dd813c15b2c101, for the particular > issue). > > Question is, should I document things of this nature on this list also > ? Even if it is likely a no-go for the backports ? Just as information > ? Should I just bring the attention to the backport need (all patches) > and you decide ? Same as above, if you test them and they work, and they resolve a reported and testable bug, why wouldn't we apply them? thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree 2018-07-05 16:32 ` Greg KH @ 2018-07-05 16:41 ` Rafael David Tinoco 0 siblings, 0 replies; 9+ messages in thread From: Rafael David Tinoco @ 2018-07-05 16:41 UTC (permalink / raw) To: ltp > > So I assumed it was a no-go. > > It all depends, let's get the git commit ids for these please. And have > you successfully applied and tested that those patches fix the issue? > If so, great, let's apply them! > > > There is also another bug: > > > > https://bugs.linaro.org/show_bug.cgi?id=3303. > > > > Fanotify faces a srcu dead-lock when userland stops responding to > > events for this other case. Fix for that bug is a 35 patches patchset > > (including the fix, commit 9dd813c15b2c101, for the particular > > issue). > > > > Question is, should I document things of this nature on this list also > > ? Even if it is likely a no-go for the backports ? Just as information > > ? Should I just bring the attention to the backport need (all patches) > > and you decide ? > > Same as above, if you test them and they work, and they resolve a > reported and testable bug, why wouldn't we apply them? Great to know! Will work on both then. Tks a lot. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-07-05 16:41 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <152086697777252@kroah.com>
2018-07-05 15:11 ` [LTP] FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree Rafael David Tinoco
2018-07-05 15:18 ` Greg KH
2018-07-05 15:27 ` Rafael David Tinoco
2018-07-05 15:30 ` Greg KH
2018-07-05 15:53 ` Rafael David Tinoco
2018-07-05 15:57 ` Greg KH
2018-07-05 16:15 ` Rafael David Tinoco
2018-07-05 16:32 ` Greg KH
2018-07-05 16:41 ` Rafael David Tinoco
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox