From: Peter Zijlstra <peterz@infradead.org>
To: Chris Mason <chris.mason@fusionio.com>
Cc: Mel Gorman <mgorman@suse.de>,
Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] futex: Remove requirement for lock_page in get_futex_key
Date: Wed, 30 Oct 2013 09:57:32 +0100 [thread overview]
Message-ID: <20131030085732.GF16117@laptop.programming.kicks-ass.net> (raw)
In-Reply-To: <20131029184827.10719.27487@localhost.localdomain>
On Tue, Oct 29, 2013 at 02:48:27PM -0400, Chris Mason wrote:
> > + /* Should be impossible but lets be paranoid for now */
> > + if (WARN_ON(inode->i_mapping != mapping)) {
> > + rcu_read_unlock();
> > + iput(inode);
> > + put_page(page_head);
> > + goto again;
> > + }
> > +
>
> Once you call iput, you add the potential to call the filesystem unlink
> operation if i_nlink had gone to zero. This shouldn't be a problem
> since you've dropped the rcu lock, but just for fun I'd move the
> put_page up a line.
>
> Or, change it to a BUG_ON instead, it really should be impossible.
So I still meant to have a look at the RCU freeing of inodes etc.. but
that comparison was to guard against inode reuse. I don't know if that
actually happens, the inode free path is a tad longer than is trivially
understood.
But if an inode would be put on a free list and reused the
atomic_inc_not_zero() could inc on a different inode than the one we
wanted and thus we need to validate we indeed got the object we set out
to acquire.
Now if its guaranteed that once an inode's refcount hits zero it will be
freed the above test is indeed superfluous and we can do the BUG_ON as
you suggest.
next prev parent reply other threads:[~2013-10-30 8:57 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-29 17:38 [RFC PATCH] futex: Remove requirement for lock_page in get_futex_key Mel Gorman
2013-10-29 18:48 ` Chris Mason
2013-10-30 8:57 ` Peter Zijlstra [this message]
2013-11-01 9:24 ` Mel Gorman
2013-10-30 8:45 ` Thomas Gleixner
2013-10-30 9:26 ` Mel Gorman
2014-02-11 15:51 ` Thomas Gleixner
2015-02-24 16:55 ` Sebastian Andrzej Siewior
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=20131030085732.GF16117@laptop.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=chris.mason@fusionio.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=tglx@linutronix.de \
/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