linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Stroetmann <stroetmann@ontolinux.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 4/4] fs: kill I_WILL_FREE
Date: Sun, 24 Oct 2010 23:50:55 +0200	[thread overview]
Message-ID: <4CC4AA3F.3050207@ontolinux.com> (raw)
In-Reply-To: <4CC4A947.3040308@ontolinux.com>

  and one again
On the 24.10.2010 23:46, Christian Stroetmann wrote:
>  little typos
>
> On the 24.10.2010 19:40, Christoph Hellwig wrote:
>> The I_WILL_FREE is currently set for inodes that we write out during
>> umount after dropping their last reference.  It is handled equally
>> to I_FREEING in most places.  The two execptions are:
>>
>>   - writeback_single_inode skips all list manipulations for I_FREEING,
>>     but not for I_WILL_FREE.  We don't care about which list an
>>     I_WILL_FREE inode is on, because we will remove it from the list
>>     a little bit later.
>>   - __mark_inode_dirty skips I_FREEING inodes but not I_WILL_FREE
>>     inodes.  This only matters for filesystem that re-dirty the inode
>>     during writeback and then use the I_DIRTY flags inside 
>> ->evict_inode.
>>     The formers is done by XFS, but it uses it's internal state to flush
>>     the inode.  I could not find any filesystem that looks at I_DIRTY
>>     inside ->evict_inode either.
>>
>> Besides cleaning up the code removing I_WILL_FREE will allow us to
>> avoid one i_lock roundtrip once inode_lock is split and keep iput_final
>> more logic.  This includes removing the __remove_inode_hash call in
>> iput_final, given that we never drop the protection from lookups now
>> that I_FREEING is set earlier.
>>
>> Signed-off-by: Christoph Hellwig<hch@lst.de>
>>
>> Index: linux-2.6/fs/btrfs/inode.c
>> ===================================================================
>> --- linux-2.6.orig/fs/btrfs/inode.c    2010-10-24 16:30:05.000000000 
>> +0200
>> +++ linux-2.6/fs/btrfs/inode.c    2010-10-24 16:32:43.647253791 +0200
>> @@ -3862,8 +3862,7 @@ again:
>>           else if (inode->i_ino>  entry->vfs_inode.i_ino)
>>               p =&parent->rb_right;
>>           else {
>> -            WARN_ON(!(entry->vfs_inode.i_state&
>> -                  (I_WILL_FREE | I_FREEING)));
>> +            WARN_ON(!(entry->vfs_inode.i_state&  I_FREEING));
>>               rb_erase(parent,&root->inode_tree);
>>               RB_CLEAR_NODE(parent);
>>               spin_unlock(&root->inode_lock);
>> Index: linux-2.6/fs/drop_caches.c
>> ===================================================================
>> --- linux-2.6.orig/fs/drop_caches.c    2010-10-24 16:30:05.000000000 
>> +0200
>> +++ linux-2.6/fs/drop_caches.c    2010-10-24 16:32:43.647253791 +0200
>> @@ -18,7 +18,7 @@ static void drop_pagecache_sb(struct sup
>>
>>       spin_lock(&inode_lock);
>>       list_for_each_entry(inode,&sb->s_inodes, i_sb_list) {
>> -        if (inode->i_state&  (I_FREEING|I_WILL_FREE|I_NEW))
>> +        if (inode->i_state&  (I_FREEING | I_NEW))
>>               continue;
>>           if (inode->i_mapping->nrpages == 0)
>>               continue;
>> Index: linux-2.6/fs/gfs2/inode.c
>> ===================================================================
>> --- linux-2.6.orig/fs/gfs2/inode.c    2010-10-24 16:30:05.000000000 
>> +0200
>> +++ linux-2.6/fs/gfs2/inode.c    2010-10-24 16:32:43.650253651 +0200
>> @@ -84,7 +84,7 @@ static int iget_skip_test(struct inode *
>>       struct gfs2_skip_data *data = opaque;
>>
>>       if (ip->i_no_addr == data->no_addr) {
>> -        if (inode->i_state&  (I_FREEING|I_WILL_FREE)){
>> +        if (inode->i_state&  I_FREEING){
>>               data->skipped = 1;
>>               return 0;
>>           }
>> Index: linux-2.6/fs/fs-writeback.c
>> ===================================================================
>> --- linux-2.6.orig/fs/fs-writeback.c    2010-10-24 16:32:39.000000000 
>> +0200
>> +++ linux-2.6/fs/fs-writeback.c    2010-10-24 16:32:43.654254629 +0200
>> @@ -301,8 +301,7 @@ static void inode_wait_for_writeback(str
>>
>>   /*
>>    * Write out an inode's dirty pages.  Called under inode_lock.  
>> Either the
>> - * caller has ref on the inode (either via __iget or via syscall 
>> against an fd)
>> - * or the inode has I_WILL_FREE set (via generic_forget_inode)
>> + * caller has ref on the inode or the inode is beeing freed.
being
  [...]

  reply	other threads:[~2010-10-24 21:52 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-24 17:40 [PATCH 1/4] fs: do not drop inode_lock in dispose_list Christoph Hellwig
2010-10-24 17:40 ` [PATCH 2/4] fs: fold invalidate_list into invalidate_inodes Christoph Hellwig
2010-10-24 21:45   ` Christian Stroetmann
2010-10-24 17:40 ` [PATCH 3/4] fs: skip I_FREEING inodes in writeback_sb_inodes Christoph Hellwig
2010-10-24 21:46   ` Christian Stroetmann
2010-10-24 17:40 ` [PATCH 4/4] fs: kill I_WILL_FREE Christoph Hellwig
2010-10-24 21:46   ` Christian Stroetmann
2010-10-24 21:50     ` Christian Stroetmann [this message]
2010-10-26  1:28   ` Al Viro
2010-10-26 19:18   ` Al Viro
2010-10-25  5:33 ` [PATCH 1/4] fs: do not drop inode_lock in dispose_list Dave Chinner
2010-10-25  5:46   ` Dave Chinner
2010-10-25  9:20     ` Dave Chinner
2010-10-25 10:07     ` Christoph Hellwig
2010-10-25 23:07       ` Dave Chinner

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=4CC4AA3F.3050207@ontolinux.com \
    --to=stroetmann@ontolinux.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@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).