public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
To: Andrew Morton <akpm@osdl.org>
Cc: viro@parcelfarce.linux.theplanet.co.uk, linux-kernel@vger.kernel.org
Subject: Re: race between __sync_single_inode() and iput()/bdev_clear_inode()
Date: Sun, 20 Mar 2005 20:27:59 +0900	[thread overview]
Message-ID: <87acoyo8u8.fsf@devron.myhome.or.jp> (raw)
In-Reply-To: <20050319223843.04b31ae5.akpm@osdl.org> (Andrew Morton's message of "Sat, 19 Mar 2005 22:38:43 -0800")

Andrew Morton <akpm@osdl.org> writes:

> The __sync_single_inode() caller takes a ref on the inode to prevent things
> like this from happening.

Yes. But, in this case, that inode->i_mapping is pointing the bdev's
->i_mapping by open("/dev/hda2").

   open("/dev/hda2") -> blkdev_open() -> bd_acquire()

   		inode->i_bdev = bdev;
		inode->i_mapping = bdev->bd_inode->i_mapping;
		list_add(&inode->i_devices, &bdev->bd_inodes);

In this race case, the inode is not freeing, but ->i_mapping is freed.

> What was the call path on the other process?  The one running
> destroy_inode()?  unmount?

close("/dev/hda2").

The _bdev's_ inode is freed by close() path after restoreing
the inode->i_mapping of filesytem in bdev_clear_inode().

   close("/dev/hda2")
     -> blkdev_close()
       -> [...]
         -> iput()
           -> generic_delete_inode()
             -> bdev_clear_inode()
               -> __bd_forget()

                   /* inode is filesystem's inode, not bdev. */
                   list_del_init(&inode->i_devices);
                   inode->i_bdev = NULL;
                   inode->i_mapping = &inode->i_data;

             -> destroy_inode()  <- is freeing the bdev's inode.

And __sync_single_inode() side is updating the inode->i_atime on filesystem.

>> +/* Called under inode_lock. */
>> +void wait_inode_ilock(struct inode *inode)
>> +{
>> +	wait_queue_head_t *wqh;
>> +	DEFINE_WAIT_BIT(wq, &inode->i_state, __I_LOCK);
>> +
>> +	if (!(inode->i_state & I_LOCK))
>> +		return;
>> +
>> +	wqh = bit_waitqueue(&inode->i_state, __I_LOCK);
>> +	do {
>> +		__iget(inode);
>> +		spin_unlock(&inode_lock);
>> +		__wait_on_bit(wqh, &wq, inode_wait, TASK_UNINTERRUPTIBLE);
>> +		iput(inode);
>> +		spin_lock(&inode_lock);
>> +	} while (inode->i_state & I_LOCK);
>> +}
>
> Does this differ from wait_on_inode()?

This checks I_LOCK after taking the inode_lock. So, caller can set the
I_LOCK after this.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

  reply	other threads:[~2005-03-20 11:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-03-19 19:46 race between __sync_single_inode() and iput()/bdev_clear_inode() OGAWA Hirofumi
2005-03-20  6:38 ` Andrew Morton
2005-03-20 11:27   ` OGAWA Hirofumi [this message]
2005-03-20 11:43     ` OGAWA Hirofumi

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=87acoyo8u8.fsf@devron.myhome.or.jp \
    --to=hirofumi@mail.parknet.co.jp \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@parcelfarce.linux.theplanet.co.uk \
    /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