FAILED: patch "[PATCH] ovl: hash non-dir by lower inode for fsnotify" failed to apply to 4.14-stable tree

PATCH [email protected] 4 participants 17 messages Mar 12, 2018 – Jul 11, 2018

Subsystems (2)

Stable branch Overlay filesystem

lore

Share & tools

lore

[email protected]

author maintainer

2018-03-12 15:02 UTC #0 lore raw

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 <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From 764baba80168ad3adafb521d2ab483ccbc49e344 Mon Sep 17 00:00:00 2001

From: Amir Goldstein <[email protected]>
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 <[email protected]> #v4.13

Signed-off-by Amir Goldstein <[email protected]>

Signed-off-by Miklos Szeredi <[email protected]>

fs/overlayfs/inode.c +40 ~~-18~~

index fcd97b783fa1..3b1bd469accd 100644
@@ -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;

Rafael David Tinoco

2018-07-05 15:11 UTC #1 lore raw

* 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 <[email protected]>
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

Quoted 143 lines · On Mon, Mar 12, 2018@12:02 PM <[email protected]> wrote:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 764baba80168ad3adafb521d2ab483ccbc49e344 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <[email protected]>
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 <[email protected]> #v4.13

Signed-off-by Amir Goldstein <[email protected]>

Signed-off-by Miklos Szeredi <[email protected]>

fs/overlayfs/inode.c +40 ~~-18~~

index fcd97b783fa1..3b1bd469accd 100644
@@ -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;

Greg KH

author maintainer

2018-07-05 15:18 UTC #2 lore raw

Quoted 1 line · Rafael David Tinoco

* 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

Rafael David Tinoco

2018-07-05 15:27 UTC #3 lore raw

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.

Quoted 10 lines · Greg KH

Quoted 1 line · Rafael David Tinoco
* 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

Greg KH

author maintainer

2018-07-05 15:30 UTC #4 lore raw

Quoted 1 line · Rafael David Tinoco

v4.14 and below have the bug.

What bug? Please never top-post, you loose all context :(

greg k-h

Rafael David Tinoco

2018-07-05 15:53 UTC #5 lore raw

Quoted 1 line

What bug? Please never top-post, you loose all context :(

Won't happen again.

Summary is this:

commit 764baba80168ad3adafb521d2ab483ccbc49e344

Author: Amir Goldstein <[email protected]>
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 ?

Greg KH

author maintainer

2018-07-05 15:57 UTC #6 lore raw

Quoted 12 lines · Rafael David Tinoco

Quoted 1 line
What bug? Please never top-post, you loose all context :(
Won't happen again.
Summary is this:
commit 764baba80168ad3adafb521d2ab483ccbc49e344
Author: Amir Goldstein <[email protected]>
Date:   Sun Feb 4 15:35:09 2018 +0200
ovl: hash non-dir by lower inode for fsnotify

Quoted 7 lines

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

Rafael David Tinoco

2018-07-05 16:15 UTC #7 lore raw

Quoted 10 lines

Quoted 10 lines
commit 764baba80168ad3adafb521d2ab483ccbc49e344
Author: Amir Goldstein <[email protected]>
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.

Quoted 6 lines

Quoted 3 lines
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

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

Greg KH

author maintainer

2018-07-05 16:32 UTC #8 lore raw

Quoted 28 lines · Rafael David Tinoco

Quoted 10 lines
Quoted 10 lines
commit 764baba80168ad3adafb521d2ab483ccbc49e344
Author: Amir Goldstein <[email protected]>
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.
Quoted 6 lines
Quoted 3 lines
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"

Quoted 6 lines

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!

Quoted 9 lines

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

Quoted 4 lines

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

Rafael David Tinoco

2018-07-05 16:41 UTC #9 lore raw

Quoted 15 lines

Quoted 1 line
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!
Quoted 9 lines
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).

Quoted 7 lines

Quoted 4 lines
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.

Amir Goldstein

maintainer

2018-07-05 18:28 UTC #10 lore raw

In reply to Greg KH

Quoted 49 lines · Greg KH

Quoted 28 lines · Rafael David Tinoco
Quoted 10 lines
Quoted 10 lines
commit 764baba80168ad3adafb521d2ab483ccbc49e344
Author: Amir Goldstein <[email protected]>
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.
Quoted 6 lines
Quoted 3 lines
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"
Quoted 6 lines
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!
Quoted 9 lines
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).

Quoted 8 lines

Quoted 4 lines
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.

Greg KH

author maintainer

2018-07-10 13:55 UTC #11 lore raw

Quoted 76 lines · Amir Goldstein

Quoted 49 lines · Greg KH
Quoted 28 lines · Rafael David Tinoco
Quoted 10 lines
Quoted 10 lines
commit 764baba80168ad3adafb521d2ab483ccbc49e344
Author: Amir Goldstein <[email protected]>
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.
Quoted 6 lines
Quoted 3 lines
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"
Quoted 6 lines
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!
Quoted 9 lines
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).
Quoted 8 lines
Quoted 4 lines
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

Quoted 7 lines

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

Rafael David Tinoco

2018-07-10 14:16 UTC #12 lore raw

Quoted 48 lines

Quoted 35 lines
Quoted 9 lines
Quoted 9 lines
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).
Quoted 8 lines
Quoted 4 lines
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
Quoted 7 lines
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.

Quoted 5 lines

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

Amir Goldstein

maintainer

2018-07-10 14:39 UTC #13 lore raw

[remove: linux-unionfs]
[add: Jan Kara]

On Tue, Jul 10, 2018 at 5:16 PM, Rafael David Tinoco <[email protected]> wrote:
[...]

Quoted 7 lines

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.

Quoted 94 lines

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

Rafael David Tinoco

2018-07-10 15:01 UTC #14 lore raw

Quoted 10 lines

Quoted 7 lines
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!

Quoted 97 lines

Thanks,
Amir.
Quoted 94 lines
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

Jan Kara

2018-07-11 08:26 UTC #15 lore raw

In reply to Amir Goldstein

Quoted 9 lines · Amir Goldstein

[remove: linux-unionfs]
[add: Jan Kara]
On Tue, Jul 10, 2018 at 5:16 PM, Rafael David Tinoco <[email protected]> wrote:
[...]
Quoted 2 lines
Greg,

Quoted 7 lines

Quoted 5 lines
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

Quoted 94 lines

Quoted 94 lines
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

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Rafael David Tinoco

2018-07-11 11:41 UTC #16 lore raw

Quoted 12 lines

Quoted 12 lines
On Tue, Jul 10, 2018 at 5:16 PM, Rafael David Tinoco <[email protected]> wrote:
[...]
Quoted 7 lines
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).

Quoted 8 lines

Quoted 1 line
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.