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;
* 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 at 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;
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
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 ?
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
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
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
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.
On Thu, Jul 5, 2018 at 7:32 PM, Greg KH gregkh@linuxfoundation.org wrote:
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?
So this is the story with overlayfs - besides NFS export in v4.16, I don't think overlayfs got any new "features". Since the day it was merged upstream (v3.18) it was all about fixing bugs and "non-standard-behaviors": https://github.com/amir73il/overlayfs/wiki/Overlayfs-non-standard-behavior
Are those non-standard behaviors reported and testable bugs? yes but they have been around for so long that applications may have become dependent on the non-standard-behavior, so the "bug fixes" are often introduced as "features" that are off by default and need to be explicitly enabled by config/module/mount option (e.g. redirect_dir added in v4.10).
Now if you want to apply all the fixes to non-standard-behavior, I am maintaining a 4.9.y backport branch with *everything*: https://github.com/amir73il/linux/commits/overlayfs-4.9.y
So this branch also includes the NFS export feature, but I suspect it going to be hairy to apply $SUBJECT fix in question without applying the NFS export patches.
What does the new benevolent Greg have to say about this? Would you consider taking all non-standard-behavior fixes to stable? If you do, I can help with maintaining the stable overlayfs branches.
Thanks, Amir.
On Thu, Jul 05, 2018 at 09:28:56PM +0300, Amir Goldstein wrote:
On Thu, Jul 5, 2018 at 7:32 PM, Greg KH gregkh@linuxfoundation.org wrote:
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?
So this is the story with overlayfs - besides NFS export in v4.16, I don't think overlayfs got any new "features". Since the day it was merged upstream (v3.18) it was all about fixing bugs and "non-standard-behaviors": https://github.com/amir73il/overlayfs/wiki/Overlayfs-non-standard-behavior
Are those non-standard behaviors reported and testable bugs? yes but they have been around for so long that applications may have become dependent on the non-standard-behavior, so the "bug fixes" are often introduced as "features" that are off by default and need to be explicitly enabled by config/module/mount option (e.g. redirect_dir added in v4.10).
Now if you want to apply all the fixes to non-standard-behavior, I am maintaining a 4.9.y backport branch with *everything*: https://github.com/amir73il/linux/commits/overlayfs-4.9.y
So this branch also includes the NFS export feature, but I suspect it going to be hairy to apply $SUBJECT fix in question without applying the NFS export patches.
What does the new benevolent Greg have to say about this? Would you consider taking all non-standard-behavior fixes to stable? If you do, I can help with maintaining the stable overlayfs branches.
I'd prefer to stick with as close-to-possible as what Linus's tree has. But for stuff like this, maybe it just makes sense to leave it all alone and if people want the newer "features" they need to move to a newer kernel, like they should be doing anyway.
So for now, let's just leave it as-is, if anyone complains we can revisit and look at the patches that are needed for backporting.
sound reasonable?
thanks,
greg k-h
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?
So this is the story with overlayfs - besides NFS export in v4.16, I don't think overlayfs got any new "features". Since the day it was merged upstream (v3.18) it was all about fixing bugs and "non-standard-behaviors": https://github.com/amir73il/overlayfs/wiki/Overlayfs-non-standard-behavior
Are those non-standard behaviors reported and testable bugs? yes but they have been around for so long that applications may have become dependent on the non-standard-behavior, so the "bug fixes" are often introduced as "features" that are off by default and need to be explicitly enabled by config/module/mount option (e.g. redirect_dir added in v4.10).
Now if you want to apply all the fixes to non-standard-behavior, I am maintaining a 4.9.y backport branch with *everything*: https://github.com/amir73il/linux/commits/overlayfs-4.9.y
So this branch also includes the NFS export feature, but I suspect it going to be hairy to apply $SUBJECT fix in question without applying the NFS export patches.
What does the new benevolent Greg have to say about this? Would you consider taking all non-standard-behavior fixes to stable? If you do, I can help with maintaining the stable overlayfs branches.
I'd prefer to stick with as close-to-possible as what Linus's tree has. But for stuff like this, maybe it just makes sense to leave it all alone and if people want the newer "features" they need to move to a newer kernel, like they should be doing anyway.
So for now, let's just leave it as-is, if anyone complains we can revisit and look at the patches that are needed for backporting.
sound reasonable?
Greg,
As spoke in this thread last week, I have prepared a patchset for v4.9 tree for one of the bugs I mentioned (https://bugs.linaro.org/show_bug.cgi?id=3303). This bug is related to a dead-lock in kernel waiting for userland events (fanotify).
Short Summary of BUG: https://bugs.linaro.org/show_bug.cgi?id=3303#c16
Full conclusion after kdump analysis: https://bugs.linaro.org/show_bug.cgi?id=3303#c14
The patch list for resolution is:
** [35/35] 054c636e5c80 fsnotify: Move ->free_mark callback to fsnotify_ops ok [34/35] 7b1293234084 fsnotify: Add group pointer in fsnotify_init_mark() ok [33/35] ebb3b47e37a4 fsnotify: Drop inode_mark.c ok [32/35] b1362edfe15b fsnotify: Remove fsnotify_find_{inode|vfsmount}_mark() ok [31/35] 2e37c6ca8d76 fsnotify: Remove fsnotify_detach_group_marks() ok [30/35] 18f2e0d3a436 fsnotify: Rename fsnotify_clear_marks_by_group_flags() ok [29/35] 416bcdbcbbb4 fsnotify: Inline fsnotify_clear_{inode|vfsmount}_mark_group() ok [28/35] 8920d2734d9a fsnotify: Remove fsnotify_recalc_{inode|vfsmount}_mask() ok [27/35] 66d2b81bcb92 fsnotify: Remove fsnotify_set_mark_{,ignored_}mask_locked() ok [26/35] 05f0e38724e8 fanotify: Release SRCU lock when waiting for userspace response ok [25/35] 9385a84d7e1f fsnotify: Pass fsnotify_iter_info into handle_event handler ** [NEED ] 3cd5eca8d7a2 fsnotify: constify 'data' passed to ->handle_event() ok [24/35] abc77577a669 fsnotify: Provide framework for dropping SRCU lock in ->handle_event ok [23/35] f09b04a03e02 fsnotify: Remove special handling of mark destruction on group shutdown ok [22/35] 6b3f05d24d35 fsnotify: Detach mark from object list when last reference is dropped ok [21/35] 11375145a70d fsnotify: Move queueing of mark for destruction into fsnotify_put_mark() ok [20/35] e7253760587e inotify: Do not drop mark reference under idr_lock ok [19/35] 08991e83b728 fsnotify: Free fsnotify_mark_connector when there is no mark attached ok [18/35] 04662cab59fc fsnotify: Lock object list with connector lock ok [17/35] 2629718dd26f fsnotify: Remove useless list deletion and comment ok [16/35] 73cd3c33ab79 fsnotify: Avoid double locking in fsnotify_detach_from_object() ok [15/35] 8212a6097a72 fsnotify: Remove indirection from fsnotify_detach_mark() ok [14/35] a03e2e4f0783 fsnotify: Determine lock in fsnotify_destroy_marks() ok [13/35] f06fd9875945 fsnotify: Move locking into fsnotify_find_mark() ok [12/35] a242677bb1e6 fsnotify: Move locking into fsnotify_recalc_mask() ok [11/35] 0810b4f9f207 fsnotify: Move fsnotify_destroy_marks() ok [10/35] 755b5bc681eb fsnotify: Remove indirection from mark list addition ok [09/35] e911d8af87db fsnotify: Make fsnotify_mark_connector hold inode reference ok [08/35] 86ffe245c430 fsnotify: Move object pointer to fsnotify_mark_connector ok [NEED ] be29d20f3f5d audit: Fix sleep in atomic ok [NEED ] e3ba730702af fsnotify: Remove fsnotify_duplicate_mark() ** [07/35] 9dd813c15b2c fsnotify: Move mark list head from object into dedicated structure -> THIS ONE ok [06/35] c1f33073ac1b fsnotify: Update comments ok [05/35] 43471d15df0e audit_tree: Use mark flags to check whether mark is alive ok [04/35] f410ff65548c audit: Abstract hash key handling ok [03/35] c97476400d3b fanotify: Move recalculation of inode / vfsmount mask under mark_mutex ok [02/35] 25c829afbd74 inotify: Remove inode pointers from debug messages ok [01/35] 5198adf649a0 fsnotify: Remove unnecessary tests when showing fdinfo
ok = cherry-pick (no changes needed) ** = backport [NEED] = needed for original patchset to be cherry-picked
(Original patchset came from https://www.spinics.net/lists/linux-fsdevel/msg109131.html and there was 3 backports for positional changes and 3 patches to satisfy the cherry-picks).
And it merges with no issues in stable v4.9 tree (as you can see in https://bugs.linaro.org/show_bug.cgi?id=3303#c21). I can submit in a thread in stable list if you are willing to move further.
As you can see, this patchset solves the issue:
BUG [unpatched] https://bugs.linaro.org/show_bug.cgi?id=3303#c18 SOLVED [patched] https://bugs.linaro.org/show_bug.cgi?id=3303#c19
And introduces NO regressions in LTP or KSELFTEST:
KSELFTEST: https://bugs.linaro.org/show_bug.cgi?id=3303#c23 LTP: https://bugs.linaro.org/show_bug.cgi?id=3303#c27
I think now we've reached the "It depends" phase =). Let me know if you think this is good to be acceptable for v4.9. We can run full round of tests (on all boards and x86/amd64) if you choose to pull this, during stable review.
I can try same thing for v4.4 if it is worth.
Cheers o/
-Rafael
[remove: linux-unionfs] [add: Jan Kara]
On Tue, Jul 10, 2018 at 5:16 PM, Rafael David Tinoco rafael.tinoco@linaro.org wrote: [...]
Greg,
As spoke in this thread last week, I have prepared a patchset for v4.9 tree for one of the bugs I mentioned (https://bugs.linaro.org/show_bug.cgi?id=3303). This bug is related to a dead-lock in kernel waiting for userland events (fanotify).
Jan may already have backports to this bug fix? I don't suppose customers were complaining about the bug in mainline kernel.
Thanks, Amir.
Short Summary of BUG: https://bugs.linaro.org/show_bug.cgi?id=3303#c16
Full conclusion after kdump analysis: https://bugs.linaro.org/show_bug.cgi?id=3303#c14
The patch list for resolution is:
** [35/35] 054c636e5c80 fsnotify: Move ->free_mark callback to fsnotify_ops ok [34/35] 7b1293234084 fsnotify: Add group pointer in fsnotify_init_mark() ok [33/35] ebb3b47e37a4 fsnotify: Drop inode_mark.c ok [32/35] b1362edfe15b fsnotify: Remove fsnotify_find_{inode|vfsmount}_mark() ok [31/35] 2e37c6ca8d76 fsnotify: Remove fsnotify_detach_group_marks() ok [30/35] 18f2e0d3a436 fsnotify: Rename fsnotify_clear_marks_by_group_flags() ok [29/35] 416bcdbcbbb4 fsnotify: Inline fsnotify_clear_{inode|vfsmount}_mark_group() ok [28/35] 8920d2734d9a fsnotify: Remove fsnotify_recalc_{inode|vfsmount}_mask() ok [27/35] 66d2b81bcb92 fsnotify: Remove fsnotify_set_mark_{,ignored_}mask_locked() ok [26/35] 05f0e38724e8 fanotify: Release SRCU lock when waiting for userspace response ok [25/35] 9385a84d7e1f fsnotify: Pass fsnotify_iter_info into handle_event handler ** [NEED ] 3cd5eca8d7a2 fsnotify: constify 'data' passed to ->handle_event() ok [24/35] abc77577a669 fsnotify: Provide framework for dropping SRCU lock in ->handle_event ok [23/35] f09b04a03e02 fsnotify: Remove special handling of mark destruction on group shutdown ok [22/35] 6b3f05d24d35 fsnotify: Detach mark from object list when last reference is dropped ok [21/35] 11375145a70d fsnotify: Move queueing of mark for destruction into fsnotify_put_mark() ok [20/35] e7253760587e inotify: Do not drop mark reference under idr_lock ok [19/35] 08991e83b728 fsnotify: Free fsnotify_mark_connector when there is no mark attached ok [18/35] 04662cab59fc fsnotify: Lock object list with connector lock ok [17/35] 2629718dd26f fsnotify: Remove useless list deletion and comment ok [16/35] 73cd3c33ab79 fsnotify: Avoid double locking in fsnotify_detach_from_object() ok [15/35] 8212a6097a72 fsnotify: Remove indirection from fsnotify_detach_mark() ok [14/35] a03e2e4f0783 fsnotify: Determine lock in fsnotify_destroy_marks() ok [13/35] f06fd9875945 fsnotify: Move locking into fsnotify_find_mark() ok [12/35] a242677bb1e6 fsnotify: Move locking into fsnotify_recalc_mask() ok [11/35] 0810b4f9f207 fsnotify: Move fsnotify_destroy_marks() ok [10/35] 755b5bc681eb fsnotify: Remove indirection from mark list addition ok [09/35] e911d8af87db fsnotify: Make fsnotify_mark_connector hold inode reference ok [08/35] 86ffe245c430 fsnotify: Move object pointer to fsnotify_mark_connector ok [NEED ] be29d20f3f5d audit: Fix sleep in atomic ok [NEED ] e3ba730702af fsnotify: Remove fsnotify_duplicate_mark() ** [07/35] 9dd813c15b2c fsnotify: Move mark list head from object into dedicated structure -> THIS ONE ok [06/35] c1f33073ac1b fsnotify: Update comments ok [05/35] 43471d15df0e audit_tree: Use mark flags to check whether mark is alive ok [04/35] f410ff65548c audit: Abstract hash key handling ok [03/35] c97476400d3b fanotify: Move recalculation of inode / vfsmount mask under mark_mutex ok [02/35] 25c829afbd74 inotify: Remove inode pointers from debug messages ok [01/35] 5198adf649a0 fsnotify: Remove unnecessary tests when showing fdinfo
ok = cherry-pick (no changes needed) ** = backport [NEED] = needed for original patchset to be cherry-picked
(Original patchset came from https://www.spinics.net/lists/linux-fsdevel/msg109131.html and there was 3 backports for positional changes and 3 patches to satisfy the cherry-picks).
And it merges with no issues in stable v4.9 tree (as you can see in https://bugs.linaro.org/show_bug.cgi?id=3303#c21). I can submit in a thread in stable list if you are willing to move further.
As you can see, this patchset solves the issue:
BUG [unpatched] https://bugs.linaro.org/show_bug.cgi?id=3303#c18 SOLVED [patched] https://bugs.linaro.org/show_bug.cgi?id=3303#c19
And introduces NO regressions in LTP or KSELFTEST:
KSELFTEST: https://bugs.linaro.org/show_bug.cgi?id=3303#c23 LTP: https://bugs.linaro.org/show_bug.cgi?id=3303#c27
I think now we've reached the "It depends" phase =). Let me know if you think this is good to be acceptable for v4.9. We can run full round of tests (on all boards and x86/amd64) if you choose to pull this, during stable review.
I can try same thing for v4.4 if it is worth.
Cheers o/
-Rafael
Greg,
As spoke in this thread last week, I have prepared a patchset for v4.9 tree for one of the bugs I mentioned (https://bugs.linaro.org/show_bug.cgi?id=3303). This bug is related to a dead-lock in kernel waiting for userland events (fanotify).
Jan may already have backports to this bug fix? I don't suppose customers were complaining about the bug in mainline kernel.
If not, and he wants to review, all patches are here:
http://people.linaro.org/~rafael.tinoco/bugs/3303/
And tests output are bellow.
Thanks!
Thanks, Amir.
Short Summary of BUG: https://bugs.linaro.org/show_bug.cgi?id=3303#c16
Full conclusion after kdump analysis: https://bugs.linaro.org/show_bug.cgi?id=3303#c14
The patch list for resolution is:
** [35/35] 054c636e5c80 fsnotify: Move ->free_mark callback to fsnotify_ops ok [34/35] 7b1293234084 fsnotify: Add group pointer in fsnotify_init_mark() ok [33/35] ebb3b47e37a4 fsnotify: Drop inode_mark.c ok [32/35] b1362edfe15b fsnotify: Remove fsnotify_find_{inode|vfsmount}_mark() ok [31/35] 2e37c6ca8d76 fsnotify: Remove fsnotify_detach_group_marks() ok [30/35] 18f2e0d3a436 fsnotify: Rename fsnotify_clear_marks_by_group_flags() ok [29/35] 416bcdbcbbb4 fsnotify: Inline fsnotify_clear_{inode|vfsmount}_mark_group() ok [28/35] 8920d2734d9a fsnotify: Remove fsnotify_recalc_{inode|vfsmount}_mask() ok [27/35] 66d2b81bcb92 fsnotify: Remove fsnotify_set_mark_{,ignored_}mask_locked() ok [26/35] 05f0e38724e8 fanotify: Release SRCU lock when waiting for userspace response ok [25/35] 9385a84d7e1f fsnotify: Pass fsnotify_iter_info into handle_event handler ** [NEED ] 3cd5eca8d7a2 fsnotify: constify 'data' passed to ->handle_event() ok [24/35] abc77577a669 fsnotify: Provide framework for dropping SRCU lock in ->handle_event ok [23/35] f09b04a03e02 fsnotify: Remove special handling of mark destruction on group shutdown ok [22/35] 6b3f05d24d35 fsnotify: Detach mark from object list when last reference is dropped ok [21/35] 11375145a70d fsnotify: Move queueing of mark for destruction into fsnotify_put_mark() ok [20/35] e7253760587e inotify: Do not drop mark reference under idr_lock ok [19/35] 08991e83b728 fsnotify: Free fsnotify_mark_connector when there is no mark attached ok [18/35] 04662cab59fc fsnotify: Lock object list with connector lock ok [17/35] 2629718dd26f fsnotify: Remove useless list deletion and comment ok [16/35] 73cd3c33ab79 fsnotify: Avoid double locking in fsnotify_detach_from_object() ok [15/35] 8212a6097a72 fsnotify: Remove indirection from fsnotify_detach_mark() ok [14/35] a03e2e4f0783 fsnotify: Determine lock in fsnotify_destroy_marks() ok [13/35] f06fd9875945 fsnotify: Move locking into fsnotify_find_mark() ok [12/35] a242677bb1e6 fsnotify: Move locking into fsnotify_recalc_mask() ok [11/35] 0810b4f9f207 fsnotify: Move fsnotify_destroy_marks() ok [10/35] 755b5bc681eb fsnotify: Remove indirection from mark list addition ok [09/35] e911d8af87db fsnotify: Make fsnotify_mark_connector hold inode reference ok [08/35] 86ffe245c430 fsnotify: Move object pointer to fsnotify_mark_connector ok [NEED ] be29d20f3f5d audit: Fix sleep in atomic ok [NEED ] e3ba730702af fsnotify: Remove fsnotify_duplicate_mark() ** [07/35] 9dd813c15b2c fsnotify: Move mark list head from object into dedicated structure -> THIS ONE ok [06/35] c1f33073ac1b fsnotify: Update comments ok [05/35] 43471d15df0e audit_tree: Use mark flags to check whether mark is alive ok [04/35] f410ff65548c audit: Abstract hash key handling ok [03/35] c97476400d3b fanotify: Move recalculation of inode / vfsmount mask under mark_mutex ok [02/35] 25c829afbd74 inotify: Remove inode pointers from debug messages ok [01/35] 5198adf649a0 fsnotify: Remove unnecessary tests when showing fdinfo
ok = cherry-pick (no changes needed) ** = backport [NEED] = needed for original patchset to be cherry-picked
(Original patchset came from https://www.spinics.net/lists/linux-fsdevel/msg109131.html and there was 3 backports for positional changes and 3 patches to satisfy the cherry-picks).
And it merges with no issues in stable v4.9 tree (as you can see in https://bugs.linaro.org/show_bug.cgi?id=3303#c21). I can submit in a thread in stable list if you are willing to move further.
As you can see, this patchset solves the issue:
BUG [unpatched] https://bugs.linaro.org/show_bug.cgi?id=3303#c18 SOLVED [patched] https://bugs.linaro.org/show_bug.cgi?id=3303#c19
And introduces NO regressions in LTP or KSELFTEST:
KSELFTEST: https://bugs.linaro.org/show_bug.cgi?id=3303#c23 LTP: https://bugs.linaro.org/show_bug.cgi?id=3303#c27
I think now we've reached the "It depends" phase =). Let me know if you think this is good to be acceptable for v4.9. We can run full round of tests (on all boards and x86/amd64) if you choose to pull this, during stable review.
I can try same thing for v4.4 if it is worth.
Cheers o/
-Rafael
On Tue 10-07-18 17:39:58, Amir Goldstein wrote:
[remove: linux-unionfs] [add: Jan Kara]
On Tue, Jul 10, 2018 at 5:16 PM, Rafael David Tinoco rafael.tinoco@linaro.org wrote: [...]
Greg,
As spoke in this thread last week, I have prepared a patchset for v4.9 tree for one of the bugs I mentioned (https://bugs.linaro.org/show_bug.cgi?id=3303). This bug is related to a dead-lock in kernel waiting for userland events (fanotify).
Jan may already have backports to this bug fix?
Definitely not to v4.9 as we don't have any kernel based on that version. We do have v4.4-based kernels but we didn't backport these fixes there as they are too intrusive and in principle the problem these patches are fixing is in a careless priviledged userspace application.
Honza
Short Summary of BUG: https://bugs.linaro.org/show_bug.cgi?id=3303#c16
Full conclusion after kdump analysis: https://bugs.linaro.org/show_bug.cgi?id=3303#c14
The patch list for resolution is:
** [35/35] 054c636e5c80 fsnotify: Move ->free_mark callback to fsnotify_ops ok [34/35] 7b1293234084 fsnotify: Add group pointer in fsnotify_init_mark() ok [33/35] ebb3b47e37a4 fsnotify: Drop inode_mark.c ok [32/35] b1362edfe15b fsnotify: Remove fsnotify_find_{inode|vfsmount}_mark() ok [31/35] 2e37c6ca8d76 fsnotify: Remove fsnotify_detach_group_marks() ok [30/35] 18f2e0d3a436 fsnotify: Rename fsnotify_clear_marks_by_group_flags() ok [29/35] 416bcdbcbbb4 fsnotify: Inline fsnotify_clear_{inode|vfsmount}_mark_group() ok [28/35] 8920d2734d9a fsnotify: Remove fsnotify_recalc_{inode|vfsmount}_mask() ok [27/35] 66d2b81bcb92 fsnotify: Remove fsnotify_set_mark_{,ignored_}mask_locked() ok [26/35] 05f0e38724e8 fanotify: Release SRCU lock when waiting for userspace response ok [25/35] 9385a84d7e1f fsnotify: Pass fsnotify_iter_info into handle_event handler ** [NEED ] 3cd5eca8d7a2 fsnotify: constify 'data' passed to ->handle_event() ok [24/35] abc77577a669 fsnotify: Provide framework for dropping SRCU lock in ->handle_event ok [23/35] f09b04a03e02 fsnotify: Remove special handling of mark destruction on group shutdown ok [22/35] 6b3f05d24d35 fsnotify: Detach mark from object list when last reference is dropped ok [21/35] 11375145a70d fsnotify: Move queueing of mark for destruction into fsnotify_put_mark() ok [20/35] e7253760587e inotify: Do not drop mark reference under idr_lock ok [19/35] 08991e83b728 fsnotify: Free fsnotify_mark_connector when there is no mark attached ok [18/35] 04662cab59fc fsnotify: Lock object list with connector lock ok [17/35] 2629718dd26f fsnotify: Remove useless list deletion and comment ok [16/35] 73cd3c33ab79 fsnotify: Avoid double locking in fsnotify_detach_from_object() ok [15/35] 8212a6097a72 fsnotify: Remove indirection from fsnotify_detach_mark() ok [14/35] a03e2e4f0783 fsnotify: Determine lock in fsnotify_destroy_marks() ok [13/35] f06fd9875945 fsnotify: Move locking into fsnotify_find_mark() ok [12/35] a242677bb1e6 fsnotify: Move locking into fsnotify_recalc_mask() ok [11/35] 0810b4f9f207 fsnotify: Move fsnotify_destroy_marks() ok [10/35] 755b5bc681eb fsnotify: Remove indirection from mark list addition ok [09/35] e911d8af87db fsnotify: Make fsnotify_mark_connector hold inode reference ok [08/35] 86ffe245c430 fsnotify: Move object pointer to fsnotify_mark_connector ok [NEED ] be29d20f3f5d audit: Fix sleep in atomic ok [NEED ] e3ba730702af fsnotify: Remove fsnotify_duplicate_mark() ** [07/35] 9dd813c15b2c fsnotify: Move mark list head from object into dedicated structure -> THIS ONE ok [06/35] c1f33073ac1b fsnotify: Update comments ok [05/35] 43471d15df0e audit_tree: Use mark flags to check whether mark is alive ok [04/35] f410ff65548c audit: Abstract hash key handling ok [03/35] c97476400d3b fanotify: Move recalculation of inode / vfsmount mask under mark_mutex ok [02/35] 25c829afbd74 inotify: Remove inode pointers from debug messages ok [01/35] 5198adf649a0 fsnotify: Remove unnecessary tests when showing fdinfo
ok = cherry-pick (no changes needed) ** = backport [NEED] = needed for original patchset to be cherry-picked
(Original patchset came from https://www.spinics.net/lists/linux-fsdevel/msg109131.html and there was 3 backports for positional changes and 3 patches to satisfy the cherry-picks).
And it merges with no issues in stable v4.9 tree (as you can see in https://bugs.linaro.org/show_bug.cgi?id=3303#c21). I can submit in a thread in stable list if you are willing to move further.
As you can see, this patchset solves the issue:
BUG [unpatched] https://bugs.linaro.org/show_bug.cgi?id=3303#c18 SOLVED [patched] https://bugs.linaro.org/show_bug.cgi?id=3303#c19
And introduces NO regressions in LTP or KSELFTEST:
KSELFTEST: https://bugs.linaro.org/show_bug.cgi?id=3303#c23 LTP: https://bugs.linaro.org/show_bug.cgi?id=3303#c27
I think now we've reached the "It depends" phase =). Let me know if you think this is good to be acceptable for v4.9. We can run full round of tests (on all boards and x86/amd64) if you choose to pull this, during stable review.
I can try same thing for v4.4 if it is worth.
Cheers o/
-Rafael
On Tue, Jul 10, 2018 at 5:16 PM, Rafael David Tinoco rafael.tinoco@linaro.org wrote: [...]
Greg,
As spoke in this thread last week, I have prepared a patchset for v4.9 tree for one of the bugs I mentioned (https://bugs.linaro.org/show_bug.cgi?id=3303). This bug is related to a dead-lock in kernel waiting for userland events (fanotify).
Jan may already have backports to this bug fix?
Definitely not to v4.9 as we don't have any kernel based on that version. We do have v4.4-based kernels but we didn't backport these fixes there as they are too intrusive and in principle the problem these patches are fixing is in a careless priviledged userspace application.
Honza
That was my initial thought: to discover if this effort was worth or not for v4.4 and v4.9 (and v4.14 for overlayfs) for both issues: LTP test inotify08 (overlayfs lower file not getting events after cached dentries are dropped) and LTP test fanotify07 (this one you reviewed).
I'm dropping the attempt then, marking both as won't fix on our side and moving on. Thanks for reviewing.
linux-stable-mirror@lists.linaro.org