linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Chinner <dgc@sgi.com>
To: lkml <linux-kernel@vger.kernel.org>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Race between generic_forget_inode() and sync_sb_inodes()?
Date: Fri, 30 Nov 2007 08:55:44 +1100	[thread overview]
Message-ID: <20071129215544.GK115527101@sgi.com> (raw)


If we are in the process of dropping an inode and it is hashed,
generic_forget_inode() will mark it I_WILL_FREE and drop the
inode_lock before calling write_inode_now().

However, at this point, the inode is still on the sb->s_dirty_list
so sync_sb_inodes() could see it and try to write it back.
i.e.:

	generic_forget_inode		sync_sb_inodes

	i_state |= I_WILL_FREE
	spin_unlock(inode_lock)
	write_inode_now()
					spin_lock(inode_lock)
					__iget(inode)
					__writeback_single_inode()
					spin_unlock(inode_lock)
	spin_lock(inode_lock)
	i_state &= ~I_WILL_FREE
	(remove from lists)
	i_state |= I_FREEING
	spin_unlock(inode_lock)
	i_state = I_CLEAR
					spin_lock(inode_lock)
					iput(inode)
					BUG_ON(i_state == I_CLEAR)
	(inode gets freed)


I came across this because I've been making changes to XFS to avoid the
inode hash, and I've found that I need to remove the inode from the
dirty list when setting I_WILL_FREE to avoid this race. I can't see
how this race is avoided when inodes are hashed, so I'm wondering
if we've just been lucky or there's something that I'm missing that
means the above does not occur.

If it really is a race, then sync_sb_inodes() should really check
I_WILL_FREE before doing an __iget(), I think. If I_WILL_FREE is set
we are already doing writeback so we don't need to do it from 
sync_sb_inodes(), right?

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

             reply	other threads:[~2007-11-29 21:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-29 21:55 David Chinner [this message]
2007-11-29 22:07 ` Race between generic_forget_inode() and sync_sb_inodes()? Neil Brown
2007-11-29 22:24   ` David Chinner
2007-11-29 23:03     ` Neil Brown
2007-11-30  9:41       ` Jarek Poplawski

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=20071129215544.GK115527101@sgi.com \
    --to=dgc@sgi.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).