From patchwork Sat Jan 2 01:52:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zhihui Zhang X-Patchwork-Id: 7940401 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2EA15BEEE5 for ; Sat, 2 Jan 2016 01:52:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7F33620450 for ; Sat, 2 Jan 2016 01:52:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E792F20444 for ; Sat, 2 Jan 2016 01:52:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752402AbcABBwQ (ORCPT ); Fri, 1 Jan 2016 20:52:16 -0500 Received: from mail-qg0-f51.google.com ([209.85.192.51]:33137 "EHLO mail-qg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751354AbcABBwP (ORCPT ); Fri, 1 Jan 2016 20:52:15 -0500 Received: by mail-qg0-f51.google.com with SMTP id b35so92078361qge.0 for ; Fri, 01 Jan 2016 17:52:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=26LL8Bic4sDjkFdXGjH1CMsIDcTX40B/oIga9c1ZoVU=; b=FC4gRdMhUJoaNl5hDdc2zX1JDoJL+zw6388TdvrEq1I2ASLXA0h1CS/GjFeG0RWt5D 0w2vUYtTOqGk+TYVv77tqFxxjdVV85NT7bIr8EqjdTQNq2D+21iYAvLAq6ow60CgXIO6 jUvfX9CioTivXklXU/HvSIYPz2hMTzyli2+nKEn9yRCnCn5VnZWnc3gGfqZUbaVBj3ET JtBbvsoowIuhe2fdKGvbGiI3/EWg9kPOHXM6108gd/YsTEXAmCkWe6mcQMZ/7LMw/f/T jGgZ1Kz8eJBCBAWqF3DZMUXT+2ojlSza9rlCo+1ho60oke6m/dWuW1yXQWhBYvP9FBap 2K1g== X-Received: by 10.140.92.84 with SMTP id a78mr89752085qge.10.1451699533280; Fri, 01 Jan 2016 17:52:13 -0800 (PST) Received: from localhost.localdomain (c-24-3-138-198.hsd1.pa.comcast.net. [24.3.138.198]) by smtp.gmail.com with ESMTPSA id w145sm26977662qhw.36.2016.01.01.17.52.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 01 Jan 2016 17:52:12 -0800 (PST) From: Zhihui Zhang To: viro@zeniv.linux.org.uk, clm@fb.com, tytso@mit.edu, swhiteho@redhat.com, mfasheh@suse.com Cc: linux-fsdevel@vger.kernel.org Subject: [PATCH] Remove I_WILL_FREE Date: Fri, 1 Jan 2016 20:52:06 -0500 Message-Id: <1451699526-28608-1-git-send-email-zzhsuny@gmail.com> X-Mailer: git-send-email 1.9.1 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I_WILL_FREE does not really add any new functionality over I_FREEING. If I_WILL_FREE is set, it will be replaced by I_FREEING under the inode lock. And they always appear in the same if statement. We can set I_FREEING a little bit earlier to achieve the same result. Signed-off-by: Zhihui Zhang --- fs/block_dev.c | 2 +- fs/btrfs/inode.c | 3 +-- fs/drop_caches.c | 2 +- fs/ext4/inode.c | 4 ++-- fs/fs-writeback.c | 14 +++++--------- fs/gfs2/inode.c | 2 +- fs/inode.c | 20 +++++++++----------- fs/notify/inode_mark.c | 6 +++--- fs/ocfs2/inode.c | 3 +-- fs/quota/dquot.c | 2 +- include/linux/fs.h | 29 ++++++++++++----------------- include/trace/events/writeback.h | 1 - 12 files changed, 37 insertions(+), 51 deletions(-) diff --git a/fs/block_dev.c b/fs/block_dev.c index 44d4a1e..f4d3b86 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1808,7 +1808,7 @@ void iterate_bdevs(void (*func)(struct block_device *, void *), void *arg) struct address_space *mapping = inode->i_mapping; spin_lock(&inode->i_lock); - if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW) || + if (inode->i_state & (I_FREEING|I_NEW) || mapping->nrpages == 0) { spin_unlock(&inode->i_lock); continue; diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index a70c579..8901300 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5433,8 +5433,7 @@ static void inode_tree_add(struct inode *inode) else if (ino > btrfs_ino(&entry->vfs_inode)) 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_replace_node(parent, new, &root->inode_tree); RB_CLEAR_NODE(parent); spin_unlock(&root->inode_lock); diff --git a/fs/drop_caches.c b/fs/drop_caches.c index d72d52b..9a1e3dc 100644 --- a/fs/drop_caches.c +++ b/fs/drop_caches.c @@ -20,7 +20,7 @@ static void drop_pagecache_sb(struct super_block *sb, void *unused) spin_lock(&sb->s_inode_list_lock); list_for_each_entry(inode, &sb->s_inodes, i_sb_list) { spin_lock(&inode->i_lock); - if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) || + if ((inode->i_state & (I_FREEING|I_NEW)) || (inode->i_mapping->nrpages == 0)) { spin_unlock(&inode->i_lock); continue; diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index ea433a7..93de212 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -4367,12 +4367,12 @@ static int other_inode_match(struct inode * inode, unsigned long ino, struct other_inode *oi = (struct other_inode *) data; if ((inode->i_ino != ino) || - (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW | + (inode->i_state & (I_FREEING | I_NEW | I_DIRTY_SYNC | I_DIRTY_DATASYNC)) || ((inode->i_state & I_DIRTY_TIME) == 0)) return 0; spin_lock(&inode->i_lock); - if (((inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW | + if (((inode->i_state & (I_FREEING | I_NEW | I_DIRTY_SYNC | I_DIRTY_DATASYNC)) == 0) && (inode->i_state & I_DIRTY_TIME)) { struct ext4_inode_info *ei = EXT4_I(inode); diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 023f6a1..e3ae7b7 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -1298,7 +1298,7 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc) /* * Write out an inode's dirty pages. Either the caller has an active reference - * on the inode or the inode has I_WILL_FREE set. + * on the inode or the inode has I_FREEING set. * * This function is designed to be called for writing back one inode which * we go e.g. from filesystem. Flusher thread uses __writeback_single_inode() @@ -1311,17 +1311,13 @@ writeback_single_inode(struct inode *inode, struct bdi_writeback *wb, int ret = 0; spin_lock(&inode->i_lock); - if (!atomic_read(&inode->i_count)) - WARN_ON(!(inode->i_state & (I_WILL_FREE|I_FREEING))); - else - WARN_ON(inode->i_state & I_WILL_FREE); if (inode->i_state & I_SYNC) { if (wbc->sync_mode != WB_SYNC_ALL) goto out; /* * It's a data-integrity sync. We must wait. Since callers hold - * inode reference or inode has I_WILL_FREE set, it cannot go + * inode reference or inode has I_FREEING set, it cannot go * away under us. */ __inode_wait_for_writeback(inode); @@ -1446,7 +1442,7 @@ static long writeback_sb_inodes(struct super_block *sb, * kind writeout is handled by the freer. */ spin_lock(&inode->i_lock); - if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) { + if (inode->i_state & (I_NEW | I_FREEING)) { spin_unlock(&inode->i_lock); redirty_tail(inode, wb); continue; @@ -2130,7 +2126,7 @@ static void wait_sb_inodes(struct super_block *sb) struct address_space *mapping = inode->i_mapping; spin_lock(&inode->i_lock); - if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) || + if ((inode->i_state & (I_FREEING|I_NEW)) || (mapping->nrpages == 0)) { spin_unlock(&inode->i_lock); continue; @@ -2301,7 +2297,7 @@ EXPORT_SYMBOL(sync_inodes_sb); * This function commits an inode to disk immediately if it is dirty. This is * primarily needed by knfsd. * - * The caller must either have a ref on the inode or must have set I_WILL_FREE. + * The caller must either have a ref on the inode or must have set I_FREEING. */ int write_inode_now(struct inode *inode, int sync) { diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c index 063fdfc..da82600 100644 --- a/fs/gfs2/inode.c +++ b/fs/gfs2/inode.c @@ -50,7 +50,7 @@ static int iget_test(struct inode *inode, void *opaque) if (ip->i_no_addr == data->no_addr) { if (data->non_block && - inode->i_state & (I_FREEING|I_CLEAR|I_WILL_FREE)) { + inode->i_state & (I_FREEING|I_CLEAR)) { data->skipped = 1; return 0; } diff --git a/fs/inode.c b/fs/inode.c index 1be5f90..b48365c 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -405,7 +405,7 @@ static void inode_lru_list_add(struct inode *inode) void inode_add_lru(struct inode *inode) { if (!(inode->i_state & (I_DIRTY_ALL | I_SYNC | - I_FREEING | I_WILL_FREE)) && + I_FREEING)) && !atomic_read(&inode->i_count) && inode->i_sb->s_flags & MS_ACTIVE) inode_lru_list_add(inode); } @@ -600,7 +600,7 @@ again: continue; spin_lock(&inode->i_lock); - if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) { + if (inode->i_state & (I_NEW | I_FREEING)) { spin_unlock(&inode->i_lock); continue; } @@ -646,7 +646,7 @@ int invalidate_inodes(struct super_block *sb, bool kill_dirty) spin_lock(&sb->s_inode_list_lock); list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) { spin_lock(&inode->i_lock); - if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) { + if (inode->i_state & (I_NEW | I_FREEING)) { spin_unlock(&inode->i_lock); continue; } @@ -783,7 +783,7 @@ repeat: if (!test(inode, data)) continue; spin_lock(&inode->i_lock); - if (inode->i_state & (I_FREEING|I_WILL_FREE)) { + if (inode->i_state & (I_FREEING)) { __wait_on_freeing_inode(inode); goto repeat; } @@ -810,7 +810,7 @@ repeat: if (inode->i_sb != sb) continue; spin_lock(&inode->i_lock); - if (inode->i_state & (I_FREEING|I_WILL_FREE)) { + if (inode->i_state & I_FREEING) { __wait_on_freeing_inode(inode); goto repeat; } @@ -1191,7 +1191,7 @@ EXPORT_SYMBOL(iunique); struct inode *igrab(struct inode *inode) { spin_lock(&inode->i_lock); - if (!(inode->i_state & (I_FREEING|I_WILL_FREE))) { + if (!(inode->i_state & I_FREEING)) { __iget(inode); spin_unlock(&inode->i_lock); } else { @@ -1353,7 +1353,7 @@ int insert_inode_locked(struct inode *inode) if (old->i_sb != sb) continue; spin_lock(&old->i_lock); - if (old->i_state & (I_FREEING|I_WILL_FREE)) { + if (old->i_state & I_FREEING) { spin_unlock(&old->i_lock); continue; } @@ -1396,7 +1396,7 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval, if (!test(old, data)) continue; spin_lock(&old->i_lock); - if (old->i_state & (I_FREEING|I_WILL_FREE)) { + if (old->i_state & I_FREEING) { spin_unlock(&old->i_lock); continue; } @@ -1460,16 +1460,14 @@ static void iput_final(struct inode *inode) return; } + inode->i_state |= I_FREEING; if (!drop) { - inode->i_state |= I_WILL_FREE; spin_unlock(&inode->i_lock); write_inode_now(inode, 1); spin_lock(&inode->i_lock); WARN_ON(inode->i_state & I_NEW); - inode->i_state &= ~I_WILL_FREE; } - inode->i_state |= I_FREEING; if (!list_empty(&inode->i_lru)) inode_lru_list_del(inode); spin_unlock(&inode->i_lock); diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c index e785fd9..800a0d4 100644 --- a/fs/notify/inode_mark.c +++ b/fs/notify/inode_mark.c @@ -158,11 +158,11 @@ void fsnotify_unmount_inodes(struct super_block *sb) /* * We cannot __iget() an inode in state I_FREEING, - * I_WILL_FREE, or I_NEW which is fine because by that point + * or I_NEW which is fine because by that point * the inode cannot have any associated watches. */ spin_lock(&inode->i_lock); - if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) { + if (inode->i_state & (I_FREEING|I_NEW)) { spin_unlock(&inode->i_lock); continue; } @@ -191,7 +191,7 @@ void fsnotify_unmount_inodes(struct super_block *sb) /* In case the dropping of a reference would nuke next_i. */ while (&next_i->i_sb_list != &sb->s_inodes) { spin_lock(&next_i->i_lock); - if (!(next_i->i_state & (I_FREEING | I_WILL_FREE)) && + if (!(next_i->i_state & I_FREEING) && atomic_read(&next_i->i_count)) { __iget(next_i); need_iput = next_i; diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c index 8f87e05..f5dab4a 100644 --- a/fs/ocfs2/inode.c +++ b/fs/ocfs2/inode.c @@ -1205,12 +1205,11 @@ int ocfs2_drop_inode(struct inode *inode) inode->i_nlink, oi->ip_flags); assert_spin_locked(&inode->i_lock); - inode->i_state |= I_WILL_FREE; + inode->i_state |= I_FREEING; spin_unlock(&inode->i_lock); write_inode_now(inode, 1); spin_lock(&inode->i_lock); WARN_ON(inode->i_state & I_NEW); - inode->i_state &= ~I_WILL_FREE; return 1; } diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c index ef0d64b..4b8e265 100644 --- a/fs/quota/dquot.c +++ b/fs/quota/dquot.c @@ -931,7 +931,7 @@ static void add_dquot_ref(struct super_block *sb, int type) spin_lock(&sb->s_inode_list_lock); list_for_each_entry(inode, &sb->s_inodes, i_sb_list) { spin_lock(&inode->i_lock); - if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) || + if ((inode->i_state & (I_FREEING|I_NEW)) || !atomic_read(&inode->i_writecount) || !dqinit_needed(inode, type)) { spin_unlock(&inode->i_lock); diff --git a/include/linux/fs.h b/include/linux/fs.h index 3aa5142..b490034 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1785,7 +1785,7 @@ struct super_operations { * I_DIRTY_DATASYNC and I_DIRTY_PAGES. * * Four bits define the lifetime of an inode. Initially, inodes are I_NEW, - * until that flag is cleared. I_WILL_FREE, I_FREEING and I_CLEAR are set at + * until that flag is cleared. I_FREEING and I_CLEAR are set at * various stages of removing an inode. * * Two bits are used for locking and completion notification, I_NEW and I_SYNC. @@ -1801,20 +1801,17 @@ struct super_operations { * New inodes set I_NEW. If two processes both create * the same inode, one of them will release its inode and * wait for I_NEW to be released before returning. - * Inodes in I_WILL_FREE, I_FREEING or I_CLEAR state can + * Inodes in I_FREEING or I_CLEAR state can * also cause waiting on I_NEW, without I_NEW actually * being set. find_inode() uses this to prevent returning * nearly-dead inodes. - * I_WILL_FREE Must be set when calling write_inode_now() if i_count - * is zero. I_FREEING must be set when I_WILL_FREE is - * cleared. * I_FREEING Set when inode is about to be freed but still has dirty * pages or buffers attached or the inode itself is still * dirty. * I_CLEAR Added by clear_inode(). In this state the inode is * clean and can be destroyed. Inode keeps I_FREEING. * - * Inodes that are I_WILL_FREE, I_FREEING or I_CLEAR are + * Inodes that are I_FREEING or I_CLEAR are * prohibited for many purposes. iget() must wait for * the inode to be completely released, then create it * anew. Other functions will just ignore such inodes, @@ -1834,26 +1831,24 @@ struct super_operations { * wb stat updates to grab mapping->tree_lock. See * inode_switch_wb_work_fn() for details. * - * Q: What is the difference between I_WILL_FREE and I_FREEING? */ #define I_DIRTY_SYNC (1 << 0) #define I_DIRTY_DATASYNC (1 << 1) #define I_DIRTY_PAGES (1 << 2) #define __I_NEW 3 #define I_NEW (1 << __I_NEW) -#define I_WILL_FREE (1 << 4) -#define I_FREEING (1 << 5) -#define I_CLEAR (1 << 6) -#define __I_SYNC 7 +#define I_FREEING (1 << 4) +#define I_CLEAR (1 << 5) +#define __I_SYNC 6 #define I_SYNC (1 << __I_SYNC) -#define I_REFERENCED (1 << 8) -#define __I_DIO_WAKEUP 9 +#define I_REFERENCED (1 << 7) +#define __I_DIO_WAKEUP 8 #define I_DIO_WAKEUP (1 << __I_DIO_WAKEUP) -#define I_LINKABLE (1 << 10) -#define I_DIRTY_TIME (1 << 11) -#define __I_DIRTY_TIME_EXPIRED 12 +#define I_LINKABLE (1 << 9) +#define I_DIRTY_TIME (1 << 10) +#define __I_DIRTY_TIME_EXPIRED 11 #define I_DIRTY_TIME_EXPIRED (1 << __I_DIRTY_TIME_EXPIRED) -#define I_WB_SWITCH (1 << 13) +#define I_WB_SWITCH (1 << 12) #define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES) #define I_DIRTY_ALL (I_DIRTY | I_DIRTY_TIME) diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h index fff846b..e8e7995 100644 --- a/include/trace/events/writeback.h +++ b/include/trace/events/writeback.h @@ -14,7 +14,6 @@ {I_DIRTY_DATASYNC, "I_DIRTY_DATASYNC"}, \ {I_DIRTY_PAGES, "I_DIRTY_PAGES"}, \ {I_NEW, "I_NEW"}, \ - {I_WILL_FREE, "I_WILL_FREE"}, \ {I_FREEING, "I_FREEING"}, \ {I_CLEAR, "I_CLEAR"}, \ {I_SYNC, "I_SYNC"}, \