All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org, evvers@ya.ru, kernel-team@fb.com,
	John McCutchan <john@johnmccutchan.com>,
	Robert Love <rlove@rlove.org>, Eric Paris <eparis@parisplace.org>
Subject: Re: [PATCH kernfs/for-4.7-fixes] kernfs: don't depend on d_find_any_alias() when generating notifications
Date: Tue, 21 Jun 2016 22:49:09 -0700	[thread overview]
Message-ID: <20160622054909.GA28598@kroah.com> (raw)
In-Reply-To: <20160621152447.GE3262@mtj.duckdns.org>

On Tue, Jun 21, 2016 at 11:24:47AM -0400, Tejun Heo wrote:
> On Fri, Jun 17, 2016 at 05:51:17PM -0400, Tejun Heo wrote:
> > kernfs_notify_workfn() sends out file modified events for the
> > scheduled kernfs_nodes.  Because the modifications aren't from
> > userland, it doesn't have the matching file struct at hand and can't
> > use fsnotify_modify().  Instead, it looked up the inode and then used
> > d_find_any_alias() to find the dentry and used fsnotify_parent() and
> > fsnotify() directly to generate notifications.
> > 
> > The assumption was that the relevant dentries would have been pinned
> > if there are listeners, which isn't true as inotify doesn't pin
> > dentries at all and watching the parent doesn't pin the child dentries
> > even for dnotify.  This led to, for example, inotify watchers not
> > getting notifications if the system is under memory pressure and the
> > matching dentries got reclaimed.  It can also be triggered through
> > /proc/sys/vm/drop_caches or a remount attempt which involves shrinking
> > dcache.
> > 
> > fsnotify_parent() only uses the dentry to access the parent inode,
> > which kernfs can do easily.  Update kernfs_notify_workfn() so that it
> > uses fsnotify() directly for both the parent and target inodes without
> > going through d_find_any_alias().  While at it, supply the target file
> > name to fsnotify() from kernfs_node->name.
> > 
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > Reported-by: Evgeny Vereshchagin <evvers@ya.ru>
> > Fixes: d911d9874801 ("kernfs: make kernfs_notify() trigger inotify events too")
> > Cc: John McCutchan <john@johnmccutchan.com>
> > Cc: Robert Love <rlove@rlove.org>
> > Cc: Eric Paris <eparis@parisplace.org>
> > Cc: stable@vger.kernel.org # v3.16+
> > ---
> > Hello,
> > 
> > I'm not sure this is the best way to deal with this but it at least
> > works fine.  If there's a better, please let me know.  If this
> > approach is okay, in the future, maybe we want to implement a helper
> > on fsnotify side to handle notification generation from back-end side?
> 
> Greg, can you please pick this one up?

Will do, thanks.

greg k-h

      reply	other threads:[~2016-06-22  5:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-17 21:51 [PATCH kernfs/for-4.7-fixes] kernfs: don't depend on d_find_any_alias() when generating notifications Tejun Heo
2016-06-21 15:24 ` Tejun Heo
2016-06-22  5:49   ` Greg Kroah-Hartman [this message]

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=20160622054909.GA28598@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=eparis@parisplace.org \
    --cc=evvers@ya.ru \
    --cc=john@johnmccutchan.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rlove@rlove.org \
    --cc=tj@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.