All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Mark Salyzyn <salyzyn@android.com>
Cc: stable <stable@vger.kernel.org>,
	kernel <linux-kernel@vger.kernel.org>,
	Amir Goldstein <amir73il@gmail.com>,
	Miklos Szeredi <mszeredi@redhat.com>
Subject: Re: ovl: hash non-dir by lower inode for fsnotify
Date: Fri, 7 Sep 2018 11:51:32 +0200	[thread overview]
Message-ID: <20180907095132.GC13327@kroah.com> (raw)
In-Reply-To: <8c38c2fc-36e6-9aa7-5924-51d7bc2c3d73@android.com>

On Thu, Aug 02, 2018 at 08:29:02AM -0700, Mark Salyzyn wrote:
> On 08/01/2018 11:05 PM, Greg KH wrote:
> > On Wed, Aug 01, 2018 at 02:29:01PM -0700, Mark Salyzyn wrote:
> > > 764baba80168ad3adafb521d2ab483ccbc49e344 ovl: hash non-dir by lower inode
> > > for fsnotify is not part of 4.14 stable and yet it was marked for 4.13
> > > stable merge when committed.
> > > 
> > > Please evaluate.
> > Why not try applying it yourself to 4.14.y and note that it does not
> > apply at all and then provide a working backport so that we can skip at
> > least one email cycle here?  :)
> > 
> > thanks,
> > 
> > greg k-h
> 
> Because I am embarrassed by the backport (!) perhaps? :-)
> 
> +linux-kernel list and authors/approvers for clearance.
> 
> I took some liberty with sb = dentry_d_sb and then sprinkled it in, upstream
> passes sb to the function and the conflicts assumed so.
> 
> --------------------------> snip <-------------------------
> 
> 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: ovl: hash non-dir by lower inode for fsnotify
> 
> (cherry pick from commit 764baba80168ad3adafb521d2ab483ccbc49e344)
> 
> 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.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Cc: <stable@vger.kernel.org> #v4.13
> Signed-off-by: Mark Salyzyn <salyzyn@android.com> #v4.14
> ---
>  fs/overlayfs/inode.c | 62 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 44 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 28a320464609a..7cfef4152e9a4 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -14,6 +14,7 @@
>  #include <linux/posix_acl.h>
>  #include <linux/ratelimit.h>
>  #include "overlayfs.h"
> +#include "ovl_entry.h"
> 
>  int ovl_setattr(struct dentry *dentry, struct iattr *attr)
>  {
> @@ -608,39 +609,63 @@ static bool ovl_verify_inode(struct inode *inode,
> struct dentry *lowerdentry,

As this patch is deemed "good", can you please resend it in a
non-corrupted way so that I can apply it to the 4.14.y tree?

thanks,

greg k-h

WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <gregkh@linuxfoundation.org>
To: Mark Salyzyn <salyzyn@android.com>
Cc: stable <stable@vger.kernel.org>,
	kernel <linux-kernel@vger.kernel.org>,
	Amir Goldstein <amir73il@gmail.com>,
	Miklos Szeredi <mszeredi@redhat.com>
Subject: Re: ovl: hash non-dir by lower inode for fsnotify
Date: Fri, 7 Sep 2018 11:51:32 +0200	[thread overview]
Message-ID: <20180907095132.GC13327@kroah.com> (raw)
In-Reply-To: <8c38c2fc-36e6-9aa7-5924-51d7bc2c3d73@android.com>

On Thu, Aug 02, 2018 at 08:29:02AM -0700, Mark Salyzyn wrote:
> On 08/01/2018 11:05 PM, Greg KH wrote:
> > On Wed, Aug 01, 2018 at 02:29:01PM -0700, Mark Salyzyn wrote:
> > > 764baba80168ad3adafb521d2ab483ccbc49e344 ovl: hash non-dir by lower inode
> > > for fsnotify is not part of 4.14 stable and yet it was marked for 4.13
> > > stable merge when committed.
> > > 
> > > Please evaluate.
> > Why not try applying it yourself to 4.14.y and note that it does not
> > apply at all and then provide a working backport so that we can skip at
> > least one email cycle here?  :)
> > 
> > thanks,
> > 
> > greg k-h
> 
> Because I am embarrassed by the backport (!) perhaps? :-)
> 
> +linux-kernel list and authors/approvers for clearance.
> 
> I took some liberty with sb = dentry_d_sb and then sprinkled it in, upstream
> passes sb to the function and the conflicts assumed so.
> 
> --------------------------> snip <-------------------------
> 
> 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: ovl: hash non-dir by lower inode for fsnotify
> 
> (cherry pick from commit 764baba80168ad3adafb521d2ab483ccbc49e344)
> 
> 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.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Cc: <stable@vger.kernel.org> #v4.13
> Signed-off-by: Mark Salyzyn <salyzyn@android.com> #v4.14
> ---
> �fs/overlayfs/inode.c | 62 +++++++++++++++++++++++++++++++-------------
> �1 file changed, 44 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 28a320464609a..7cfef4152e9a4 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -14,6 +14,7 @@
> �#include <linux/posix_acl.h>
> �#include <linux/ratelimit.h>
> �#include "overlayfs.h"
> +#include "ovl_entry.h"
> 
> �int ovl_setattr(struct dentry *dentry, struct iattr *attr)
> �{
> @@ -608,39 +609,63 @@ static bool ovl_verify_inode(struct inode *inode,
> struct dentry *lowerdentry,

As this patch is deemed "good", can you please resend it in a
non-corrupted way so that I can apply it to the 4.14.y tree?

thanks,

greg k-h

  reply	other threads:[~2018-09-07  9:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-01 21:29 ovl: hash non-dir by lower inode for fsnotify Mark Salyzyn
2018-08-02  6:05 ` Greg KH
2018-08-02 15:29   ` Mark Salyzyn
2018-09-07  9:51     ` Greg KH [this message]
2018-09-07  9:51       ` Greg KH
2018-09-28 17:24       ` Mark Salyzyn
2018-09-30 15:40         ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2018-09-28 17:22 Mark Salyzyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180907095132.GC13327@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=amir73il@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=salyzyn@android.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.