public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Paris <eparis@redhat.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Evgeniy Polyakov <zbr@ioremap.net>,
	linux-kernel@vger.kernel.org, a.p.zijlstra@chello.nl,
	hch@infradead.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk
Subject: Re: [RFC PATCH -v4 07/14] fsnotify: add in inode fsnotify markings
Date: Mon, 22 Dec 2008 09:45:28 -0500	[thread overview]
Message-ID: <1229957128.3619.7.camel@localhost.localdomain> (raw)
In-Reply-To: <20081222134300.GA21807@ZenIV.linux.org.uk>

On Mon, 2008-12-22 at 13:43 +0000, Al Viro wrote:
> On Sat, Dec 13, 2008 at 11:35:09AM -0500, Eric Paris wrote:
> 
> > So to find an entry we need to first grab the inode->i_lock and start to
> > walk the inode->i_fsnotify_mark_entries list.  Since we hold the i_lock
> > we are not allowed to grab any other locks nor are we allowed to change
> > anything other than entry->i_list.  The secret sauce is that we actually
> > move the entry from the inode list to a private list which we can walk
> > and modify lockless.  Inside the event we actually have to use a
> > different list, free_i_list, for this operation so nothing else that
> > races with us can mess stuff up.  We run the entire inode we are trying
> > to free all entries for an put the entries on the private list.  We do
> > NOT modify event->inode.
> 
> And just what happens if #3 ("remove entry") hits us in the meanwhile?  Freed
> object sitting on free_i_list?

In the code on list, nothing.  Evgeniy Polyakov pointed out that I
grabbed my marks at the wrong time.

http://lkml.org/lkml/2008/12/13/94

The corrected idea is that while under the i_lock I call
fsnotify_get_mark() on every inode mark that I am moving to the
free_i_list.  When #3 races it actually does all of the cleanup,
including setting the "freeme" flag.  But it won't be able to get the
refcnt down to 0.  Once we finish 'cleaning up,' we call put on the
object and the refcnt will actually go to 0 and be freed.

The memory can't be freed as long as it is on a free_{i,g}_list.

-Eric


  reply	other threads:[~2008-12-22 14:45 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-12 21:51 [RFC PATCH -v4 00/14] fsnotify, dnotify, and inotify Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 01/14] filesystem notification: create fs/notify to contain all fs notification Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 02/14] fsnotify: pass a file instead of an inode to open, read, and write Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 03/14] fsnotify: sys_execve and sys_uselib do not call into fsnotify Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 04/14] fsnotify: use the new open-exec hook for inotify and dnotify Eric Paris
2008-12-13 15:29   ` Christoph Hellwig
2008-12-12 21:51 ` [RFC PATCH -v4 05/14] fsnotify: unified filesystem notification backend Eric Paris
2008-12-13  2:54   ` Evgeniy Polyakov
2008-12-13 15:01     ` Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 06/14] fsnotify: add group priorities Eric Paris
2008-12-12 21:51 ` [RFC PATCH -v4 07/14] fsnotify: add in inode fsnotify markings Eric Paris
2008-12-13  3:07   ` Evgeniy Polyakov
2008-12-13 16:35     ` Eric Paris
2008-12-22 13:43       ` Al Viro
2008-12-22 14:45         ` Eric Paris [this message]
2008-12-12 21:51 ` [RFC PATCH -v4 08/14] fsnotify: parent event notification Eric Paris
2008-12-12 21:52 ` [RFC PATCH -v4 09/14] dnotify: reimplement dnotify using fsnotify Eric Paris
2008-12-12 21:52 ` [RFC PATCH -v4 10/14] fsnotify: generic notification queue and waitq Eric Paris
2008-12-12 21:52 ` [RFC PATCH -v4 11/14] fsnotify: include pathnames with entries when possible Eric Paris
2008-12-13  3:19   ` Evgeniy Polyakov
2008-12-13 16:42     ` Eric Paris
2008-12-12 21:52 ` [RFC PATCH -v4 12/14] fsnotify: add correlations between events Eric Paris
2008-12-18 22:28   ` C. Scott Ananian
2008-12-22  2:40     ` Eric Paris
2008-12-22  9:01       ` Evgeniy Polyakov
2008-12-22 20:06       ` C. Scott Ananian
2008-12-12 21:52 ` [RFC PATCH -v4 13/14] inotify: reimplement inotify using fsnotify Eric Paris
2008-12-13  3:22   ` Evgeniy Polyakov
2008-12-13 16:44     ` Eric Paris
2008-12-15 15:48       ` Evgeniy Polyakov
2008-12-12 21:52 ` [RFC PATCH -v4 14/14] shit on top for debugging Eric Paris
2008-12-14 22:40   ` James Morris
2008-12-14 22:47     ` Eric Paris
2008-12-18 23:36 ` [RFC PATCH -v4 00/14] fsnotify, dnotify, and inotify C. Scott Ananian
2008-12-22  3:22   ` Eric Paris
2008-12-22 10:58     ` Niraj Kumar
2008-12-22 19:59     ` C. Scott Ananian
2008-12-22 20:53       ` Eric Paris
2008-12-29 18:19         ` C. Scott Ananian
2008-12-22 21:04       ` Al Viro
2008-12-22 23:08         ` C. Scott Ananian
2008-12-22 23:20           ` Al Viro
2008-12-22 23:21           ` Christoph Hellwig
2008-12-25 18:17             ` C. Scott Ananian
2008-12-25 20:33               ` Al Viro
2008-12-26  0:58                 ` C. Scott Ananian
2008-12-26  1:44                   ` Al Viro
2008-12-27 21:23                     ` C. Scott Ananian

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=1229957128.3619.7.camel@localhost.localdomain \
    --to=eparis@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    --cc=zbr@ioremap.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox