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

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

[gregkh]

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;

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

[Rafael David Tinoco]

* 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;
>

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

[Greg KH]

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

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

[Rafael David Tinoco]

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

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

[Greg KH]

On Thu, Jul 05, 2018 at 12:27:05PM -0300, Rafael David Tinoco wrote:
> v4.14 and below have the bug.

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

greg k-h

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

[Rafael David Tinoco]

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

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

[Greg KH]

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

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

[Rafael David Tinoco]

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

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

[Greg KH]

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

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

[Rafael David Tinoco]

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

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

[Amir Goldstein]

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.

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

[Greg KH]

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

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

[Rafael David Tinoco]

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

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

[Amir Goldstein]

[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

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

[Rafael David Tinoco]

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

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

[Jan Kara]

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
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Rafael David Tinoco]

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