* fs: break out inode LRU operations from inode_lock V2
@ 2010-10-27 23:02 Dave Chinner
2010-10-27 23:02 ` [PATCH 1/4] fs: protect inode->i_state with inode->i_lock Dave Chinner
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Dave Chinner @ 2010-10-27 23:02 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel
Hi Al,
The following patches break the inode LRU operations and the first
half of iput_final() out from under the inode_lock. The patches
should apply to your current merge-stem tree.
Version 2:
- make i_state checks/__iget() atomic under inode->i_lock
- merge dispose_one_inode() into evict()
- move inode hash removal after ->evict_inode call.
- always take the inode_lru_lock() when checking whether the inode
is on the lru or not.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] fs: protect inode->i_state with inode->i_lock
2010-10-27 23:02 fs: break out inode LRU operations from inode_lock V2 Dave Chinner
@ 2010-10-27 23:02 ` Dave Chinner
2010-10-27 23:02 ` [PATCH 2/4] fs: factor inode disposal Dave Chinner
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2010-10-27 23:02 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel
From: Dave Chinner <dchinner@redhat.com>
Protect inode state transitions and validity checks with the
inode->i_lock. This enables us to make inode state transitions
independently of the inode_lock and is the first step to peeling
away the inode_lock from the code.
This requires that __iget() is done atomically with i_state checks
during list traversals so that we don't race with another thread
marking the inode I_FREEING between the state check and grabbing the
reference.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/drop_caches.c | 9 +++--
fs/fs-writeback.c | 42 +++++++++++++++++-----
fs/inode.c | 93 +++++++++++++++++++++++++++++++++++++++--------
fs/notify/inode_mark.c | 21 ++++++++---
fs/quota/dquot.c | 13 ++++---
5 files changed, 138 insertions(+), 40 deletions(-)
diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 2195c21..62dd8ee 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -18,11 +18,14 @@ static void drop_pagecache_sb(struct super_block *sb, void *unused)
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))
- continue;
- if (inode->i_mapping->nrpages == 0)
+ spin_lock(&inode->i_lock);
+ if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) ||
+ (inode->i_mapping->nrpages == 0)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
__iget(inode);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
invalidate_mapping_pages(inode->i_mapping, 0, -1);
iput(toput_inode);
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index f6af81a..50de4f1 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -293,9 +293,11 @@ static void inode_wait_for_writeback(struct inode *inode)
wqh = bit_waitqueue(&inode->i_state, __I_SYNC);
while (inode->i_state & I_SYNC) {
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
__wait_on_bit(wqh, &wq, inode_wait, TASK_UNINTERRUPTIBLE);
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
}
}
@@ -319,6 +321,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
unsigned dirty;
int ret;
+ spin_lock(&inode->i_lock);
if (!atomic_read(&inode->i_count))
WARN_ON(!(inode->i_state & (I_WILL_FREE|I_FREEING)));
else
@@ -334,6 +337,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
* completed a full scan of b_io.
*/
if (wbc->sync_mode != WB_SYNC_ALL) {
+ spin_unlock(&inode->i_lock);
requeue_io(inode);
return 0;
}
@@ -349,6 +353,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
/* Set I_SYNC, reset I_DIRTY_PAGES */
inode->i_state |= I_SYNC;
inode->i_state &= ~I_DIRTY_PAGES;
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
ret = do_writepages(mapping, wbc);
@@ -370,8 +375,10 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
* write_inode()
*/
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
dirty = inode->i_state & I_DIRTY;
inode->i_state &= ~(I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
/* Don't write the inode if only I_DIRTY_PAGES was set */
if (dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
@@ -381,6 +388,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
}
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
inode->i_state &= ~I_SYNC;
if (!(inode->i_state & I_FREEING)) {
if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
@@ -422,6 +430,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
}
}
inode_sync_complete(inode);
+ spin_unlock(&inode->i_lock);
return ret;
}
@@ -492,7 +501,9 @@ static int writeback_sb_inodes(struct super_block *sb, struct bdi_writeback *wb,
* kind does not need peridic writeout yet, and for the latter
* kind writeout is handled by the freer.
*/
+ spin_lock(&inode->i_lock);
if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
+ spin_unlock(&inode->i_lock);
requeue_io(inode);
continue;
}
@@ -501,10 +512,14 @@ static int writeback_sb_inodes(struct super_block *sb, struct bdi_writeback *wb,
* Was this inode dirtied after sync_sb_inodes was called?
* This keeps sync from extra jobs and livelock.
*/
- if (inode_dirtied_after(inode, wbc->wb_start))
+ if (inode_dirtied_after(inode, wbc->wb_start)) {
+ spin_unlock(&inode->i_lock);
return 1;
+ }
__iget(inode);
+ spin_unlock(&inode->i_lock);
+
pages_skipped = wbc->pages_skipped;
writeback_single_inode(inode, wbc);
if (wbc->pages_skipped != pages_skipped) {
@@ -681,7 +696,9 @@ static long wb_writeback(struct bdi_writeback *wb,
if (!list_empty(&wb->b_more_io)) {
inode = wb_inode(wb->b_more_io.prev);
trace_wbc_writeback_wait(&wbc, wb->bdi);
+ spin_lock(&inode->i_lock);
inode_wait_for_writeback(inode);
+ spin_unlock(&inode->i_lock);
}
spin_unlock(&inode_lock);
}
@@ -947,6 +964,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
block_dump___mark_inode_dirty(inode);
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
if ((inode->i_state & flags) != flags) {
const int was_dirty = inode->i_state & I_DIRTY;
@@ -958,7 +976,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
* superblock list, based upon its state.
*/
if (inode->i_state & I_SYNC)
- goto out;
+ goto out_unlock_inode;
/*
* Only add valid (hashed) inodes to the superblock's
@@ -966,11 +984,12 @@ void __mark_inode_dirty(struct inode *inode, int flags)
*/
if (!S_ISBLK(inode->i_mode)) {
if (inode_unhashed(inode))
- goto out;
+ goto out_unlock_inode;
}
if (inode->i_state & I_FREEING)
- goto out;
+ goto out_unlock_inode;
+ spin_unlock(&inode->i_lock);
/*
* If the inode was already on b_dirty/b_io/b_more_io, don't
* reposition it (that would break b_dirty time-ordering).
@@ -995,7 +1014,10 @@ void __mark_inode_dirty(struct inode *inode, int flags)
inode->dirtied_when = jiffies;
list_move(&inode->i_wb_list, &bdi->wb.b_dirty);
}
+ goto out;
}
+out_unlock_inode:
+ spin_unlock(&inode->i_lock);
out:
spin_unlock(&inode_lock);
@@ -1041,14 +1063,16 @@ static void wait_sb_inodes(struct super_block *sb)
* we still have to wait for that writeout.
*/
list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
- struct address_space *mapping;
+ struct address_space *mapping = inode->i_mapping;
- if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW))
- continue;
- mapping = inode->i_mapping;
- if (mapping->nrpages == 0)
+ spin_lock(&inode->i_lock);
+ if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) ||
+ (mapping->nrpages == 0)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
__iget(inode);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
/*
* We hold a reference to 'inode' so it couldn't have
diff --git a/fs/inode.c b/fs/inode.c
index a6d6068..e1959f3 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -26,6 +26,17 @@
#include <linux/posix_acl.h>
/*
+ * inode locking rules.
+ *
+ * inode->i_lock protects:
+ * inode->i_state, __iget()
+ *
+ * Lock ordering:
+ * inode_lock
+ * inode->i_lock
+ */
+
+/*
* This is needed for the following functions:
* - inode_has_buffers
* - invalidate_bdev
@@ -429,7 +440,9 @@ void end_writeback(struct inode *inode)
BUG_ON(!(inode->i_state & I_FREEING));
BUG_ON(inode->i_state & I_CLEAR);
inode_sync_wait(inode);
+ spin_lock(&inode->i_lock);
inode->i_state = I_FREEING | I_CLEAR;
+ spin_unlock(&inode->i_lock);
}
EXPORT_SYMBOL(end_writeback);
@@ -498,12 +511,17 @@ void evict_inodes(struct super_block *sb)
if (atomic_read(&inode->i_count))
continue;
+ spin_lock(&inode->i_lock);
if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
+ spin_unlock(&inode->i_lock);
WARN_ON(1);
continue;
}
inode->i_state |= I_FREEING;
+ if (!(inode->i_state & (I_DIRTY | I_SYNC)))
+ percpu_counter_dec(&nr_inodes_unused);
+ spin_unlock(&inode->i_lock);
/*
* Move the inode off the IO lists and LRU once I_FREEING is
@@ -511,8 +529,6 @@ void evict_inodes(struct super_block *sb)
*/
list_move(&inode->i_lru, &dispose);
list_del_init(&inode->i_wb_list);
- if (!(inode->i_state & (I_DIRTY | I_SYNC)))
- percpu_counter_dec(&nr_inodes_unused);
}
spin_unlock(&inode_lock);
@@ -537,14 +553,21 @@ int invalidate_inodes(struct super_block *sb)
spin_lock(&inode_lock);
list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) {
- if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE))
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
if (atomic_read(&inode->i_count)) {
+ spin_unlock(&inode->i_lock);
busy = 1;
continue;
}
inode->i_state |= I_FREEING;
+ if (!(inode->i_state & (I_DIRTY | I_SYNC)))
+ percpu_counter_dec(&nr_inodes_unused);
+ spin_unlock(&inode->i_lock);
/*
* Move the inode off the IO lists and LRU once I_FREEING is
@@ -552,8 +575,6 @@ int invalidate_inodes(struct super_block *sb)
*/
list_move(&inode->i_lru, &dispose);
list_del_init(&inode->i_wb_list);
- if (!(inode->i_state & (I_DIRTY | I_SYNC)))
- percpu_counter_dec(&nr_inodes_unused);
}
spin_unlock(&inode_lock);
@@ -612,8 +633,10 @@ static void prune_icache(int nr_to_scan)
* Referenced or dirty inodes are still in use. Give them
* another pass through the LRU as we canot reclaim them now.
*/
+ spin_lock(&inode->i_lock);
if (atomic_read(&inode->i_count) ||
(inode->i_state & ~I_REFERENCED)) {
+ spin_unlock(&inode->i_lock);
list_del_init(&inode->i_lru);
percpu_counter_dec(&nr_inodes_unused);
continue;
@@ -621,12 +644,14 @@ static void prune_icache(int nr_to_scan)
/* recently referenced inodes get one more pass */
if (inode->i_state & I_REFERENCED) {
- list_move(&inode->i_lru, &inode_lru);
inode->i_state &= ~I_REFERENCED;
+ spin_unlock(&inode->i_lock);
+ list_move(&inode->i_lru, &inode_lru);
continue;
}
if (inode_has_buffers(inode) || inode->i_data.nrpages) {
__iget(inode);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
if (remove_inode_buffers(inode))
reap += invalidate_mapping_pages(&inode->i_data,
@@ -637,11 +662,15 @@ static void prune_icache(int nr_to_scan)
if (inode != list_entry(inode_lru.next,
struct inode, i_lru))
continue; /* wrong inode or list_empty */
- if (!can_unuse(inode))
+ spin_lock(&inode->i_lock);
+ if (!can_unuse(inode)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
}
WARN_ON(inode->i_state & I_NEW);
inode->i_state |= I_FREEING;
+ spin_unlock(&inode->i_lock);
/*
* Move the inode off the IO lists and LRU once I_FREEING is
@@ -708,11 +737,13 @@ repeat:
continue;
if (!test(inode, data))
continue;
+ spin_lock(&inode->i_lock);
if (inode->i_state & (I_FREEING|I_WILL_FREE)) {
__wait_on_freeing_inode(inode);
goto repeat;
}
__iget(inode);
+ spin_unlock(&inode->i_lock);
return inode;
}
return NULL;
@@ -734,11 +765,13 @@ repeat:
continue;
if (inode->i_sb != sb)
continue;
+ spin_lock(&inode->i_lock);
if (inode->i_state & (I_FREEING|I_WILL_FREE)) {
__wait_on_freeing_inode(inode);
goto repeat;
}
__iget(inode);
+ spin_unlock(&inode->i_lock);
return inode;
}
return NULL;
@@ -803,8 +836,10 @@ struct inode *new_inode(struct super_block *sb)
inode = alloc_inode(sb);
if (inode) {
spin_lock(&inode_lock);
- __inode_sb_list_add(inode);
+ spin_lock(&inode->i_lock);
inode->i_state = 0;
+ spin_unlock(&inode->i_lock);
+ __inode_sb_list_add(inode);
spin_unlock(&inode_lock);
}
return inode;
@@ -871,9 +906,11 @@ static struct inode *get_new_inode(struct super_block *sb,
if (set(inode, data))
goto set_failed;
+ spin_lock(&inode->i_lock);
+ inode->i_state = I_NEW;
+ spin_unlock(&inode->i_lock);
hlist_add_head(&inode->i_hash, head);
__inode_sb_list_add(inode);
- inode->i_state = I_NEW;
spin_unlock(&inode_lock);
/* Return the locked inode with I_NEW set, the
@@ -917,10 +954,12 @@ static struct inode *get_new_inode_fast(struct super_block *sb,
/* We released the lock, so.. */
old = find_inode_fast(sb, head, ino);
if (!old) {
+ spin_lock(&inode->i_lock);
inode->i_ino = ino;
+ inode->i_state = I_NEW;
+ spin_unlock(&inode->i_lock);
hlist_add_head(&inode->i_hash, head);
__inode_sb_list_add(inode);
- inode->i_state = I_NEW;
spin_unlock(&inode_lock);
/* Return the locked inode with I_NEW set, the
@@ -1005,15 +1044,19 @@ EXPORT_SYMBOL(iunique);
struct inode *igrab(struct inode *inode)
{
spin_lock(&inode_lock);
- if (!(inode->i_state & (I_FREEING|I_WILL_FREE)))
+ spin_lock(&inode->i_lock);
+ if (!(inode->i_state & (I_FREEING|I_WILL_FREE))) {
__iget(inode);
- else
+ spin_unlock(&inode->i_lock);
+ } else {
+ spin_unlock(&inode->i_lock);
/*
* Handle the case where s_op->clear_inode is not been
* called yet, and somebody is calling igrab
* while the inode is getting freed.
*/
inode = NULL;
+ }
spin_unlock(&inode_lock);
return inode;
}
@@ -1242,7 +1285,9 @@ int insert_inode_locked(struct inode *inode)
ino_t ino = inode->i_ino;
struct hlist_head *head = inode_hashtable + hash(sb, ino);
+ spin_lock(&inode->i_lock);
inode->i_state |= I_NEW;
+ spin_unlock(&inode->i_lock);
while (1) {
struct hlist_node *node;
struct inode *old = NULL;
@@ -1252,8 +1297,11 @@ int insert_inode_locked(struct inode *inode)
continue;
if (old->i_sb != sb)
continue;
- if (old->i_state & (I_FREEING|I_WILL_FREE))
+ spin_lock(&old->i_lock);
+ if (old->i_state & (I_FREEING|I_WILL_FREE)) {
+ spin_unlock(&old->i_lock);
continue;
+ }
break;
}
if (likely(!node)) {
@@ -1262,6 +1310,7 @@ int insert_inode_locked(struct inode *inode)
return 0;
}
__iget(old);
+ spin_unlock(&old->i_lock);
spin_unlock(&inode_lock);
wait_on_inode(old);
if (unlikely(!inode_unhashed(old))) {
@@ -1279,7 +1328,9 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
struct super_block *sb = inode->i_sb;
struct hlist_head *head = inode_hashtable + hash(sb, hashval);
+ spin_lock(&inode->i_lock);
inode->i_state |= I_NEW;
+ spin_unlock(&inode->i_lock);
while (1) {
struct hlist_node *node;
@@ -1291,8 +1342,11 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
continue;
if (!test(old, data))
continue;
- if (old->i_state & (I_FREEING|I_WILL_FREE))
+ spin_lock(&old->i_lock);
+ if (old->i_state & (I_FREEING|I_WILL_FREE)) {
+ spin_unlock(&old->i_lock);
continue;
+ }
break;
}
if (likely(!node)) {
@@ -1301,6 +1355,7 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
return 0;
}
__iget(old);
+ spin_unlock(&old->i_lock);
spin_unlock(&inode_lock);
wait_on_inode(old);
if (unlikely(!inode_unhashed(old))) {
@@ -1346,6 +1401,9 @@ static void iput_final(struct inode *inode)
const struct super_operations *op = inode->i_sb->s_op;
int drop;
+ spin_lock(&inode->i_lock);
+ WARN_ON(inode->i_state & I_NEW);
+
if (op && op->drop_inode)
drop = op->drop_inode(inode);
else
@@ -1357,21 +1415,23 @@ static void iput_final(struct inode *inode)
if (!(inode->i_state & (I_DIRTY|I_SYNC))) {
inode_lru_list_add(inode);
}
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
return;
}
- WARN_ON(inode->i_state & I_NEW);
inode->i_state |= I_WILL_FREE;
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
write_inode_now(inode, 1);
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
WARN_ON(inode->i_state & I_NEW);
inode->i_state &= ~I_WILL_FREE;
__remove_inode_hash(inode);
}
- WARN_ON(inode->i_state & I_NEW);
inode->i_state |= I_FREEING;
+ spin_unlock(&inode->i_lock);
/*
* Move the inode off the IO lists and LRU once I_FREEING is
@@ -1592,6 +1652,7 @@ static void __wait_on_freeing_inode(struct inode *inode)
DEFINE_WAIT_BIT(wait, &inode->i_state, __I_NEW);
wq = bit_waitqueue(&inode->i_state, __I_NEW);
prepare_to_wait(wq, &wait.wait, TASK_UNINTERRUPTIBLE);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
schedule();
finish_wait(wq, &wait.wait);
diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
index 21ed106..9c0fb09 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -249,8 +249,11 @@ void fsnotify_unmount_inodes(struct list_head *list)
* I_WILL_FREE, or I_NEW which is fine because by that point
* the inode cannot have any associated watches.
*/
- if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW))
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
/*
* If i_count is zero, the inode cannot have any watches and
@@ -258,8 +261,10 @@ void fsnotify_unmount_inodes(struct list_head *list)
* evict all inodes with zero i_count from icache which is
* unnecessarily violent and may in fact be illegal to do.
*/
- if (!atomic_read(&inode->i_count))
+ if (!atomic_read(&inode->i_count)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
need_iput_tmp = need_iput;
need_iput = NULL;
@@ -269,13 +274,17 @@ void fsnotify_unmount_inodes(struct list_head *list)
__iget(inode);
else
need_iput_tmp = NULL;
+ spin_unlock(&inode->i_lock);
/* In case the dropping of a reference would nuke next_i. */
if ((&next_i->i_sb_list != list) &&
- atomic_read(&next_i->i_count) &&
- !(next_i->i_state & (I_FREEING | I_WILL_FREE))) {
- __iget(next_i);
- need_iput = next_i;
+ atomic_read(&next_i->i_count)) {
+ spin_lock(&next_i->i_lock);
+ if (!(next_i->i_state & (I_FREEING | I_WILL_FREE))) {
+ __iget(next_i);
+ need_iput = next_i;
+ }
+ spin_unlock(&next_i->i_lock);
}
/*
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index aad1316..a2d89f8 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -898,18 +898,19 @@ static void add_dquot_ref(struct super_block *sb, int type)
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))
+ spin_lock(&inode->i_lock);
+ if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) ||
+ !atomic_read(&inode->i_writecount) ||
+ !dqinit_needed(inode, type)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
#ifdef CONFIG_QUOTA_DEBUG
if (unlikely(inode_get_rsv_space(inode) > 0))
reserved = 1;
#endif
- if (!atomic_read(&inode->i_writecount))
- continue;
- if (!dqinit_needed(inode, type))
- continue;
-
__iget(inode);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
iput(old_inode);
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] fs: factor inode disposal
2010-10-27 23:02 fs: break out inode LRU operations from inode_lock V2 Dave Chinner
2010-10-27 23:02 ` [PATCH 1/4] fs: protect inode->i_state with inode->i_lock Dave Chinner
@ 2010-10-27 23:02 ` Dave Chinner
2010-10-27 23:02 ` [PATCH 3/4] fs: Lock the inode LRU list separately Dave Chinner
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2010-10-27 23:02 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel
From: Dave Chinner <dchinner@redhat.com>
We have a couple of places that dispose of inodes. factor the
disposal into evict() to isolate this code and make it simpler to
peel away the inode_lock from the code.
While doing this, change the logic flow in iput_final() to separate
the different cases that need to be handled to make the transitions
the inode goes through more obvious.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/inode.c | 89 +++++++++++++++++++++++++----------------------------------
1 files changed, 38 insertions(+), 51 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index e1959f3..bd1688b 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -408,17 +408,6 @@ void __insert_inode_hash(struct inode *inode, unsigned long hashval)
EXPORT_SYMBOL(__insert_inode_hash);
/**
- * __remove_inode_hash - remove an inode from the hash
- * @inode: inode to unhash
- *
- * Remove an inode from the superblock.
- */
-static void __remove_inode_hash(struct inode *inode)
-{
- hlist_del_init(&inode->i_hash);
-}
-
-/**
* remove_inode_hash - remove an inode from the hash
* @inode: inode to unhash
*
@@ -446,10 +435,26 @@ void end_writeback(struct inode *inode)
}
EXPORT_SYMBOL(end_writeback);
+/*
+ * Free the inode passed in, removing it from the lists it is still connected
+ * to. We remove any pages still attached to the inode and wait for any IO that
+ * is still in progress before finally destroying the inode.
+ *
+ * An inode must already be marked I_FREEING so that we avoid the inode being
+ * moved back onto lists if we race with other code that manipulates the lists
+ * (e.g. writeback_single_inode). The caller is responsible for setting this.
+ */
static void evict(struct inode *inode)
{
const struct super_operations *op = inode->i_sb->s_op;
+ BUG_ON(!(inode->i_state & I_FREEING));
+
+ spin_lock(&inode_lock);
+ list_del_init(&inode->i_wb_list);
+ __inode_sb_list_del(inode);
+ spin_unlock(&inode_lock);
+
if (op->evict_inode) {
op->evict_inode(inode);
} else {
@@ -461,6 +466,11 @@ static void evict(struct inode *inode)
bd_forget(inode);
if (S_ISCHR(inode->i_mode) && inode->i_cdev)
cd_forget(inode);
+
+ remove_inode_hash(inode);
+ wake_up_inode(inode);
+ BUG_ON(inode->i_state != (I_FREEING | I_CLEAR));
+ destroy_inode(inode);
}
/*
@@ -479,14 +489,6 @@ static void dispose_list(struct list_head *head)
list_del_init(&inode->i_lru);
evict(inode);
-
- spin_lock(&inode_lock);
- __remove_inode_hash(inode);
- __inode_sb_list_del(inode);
- spin_unlock(&inode_lock);
-
- wake_up_inode(inode);
- destroy_inode(inode);
}
}
@@ -524,11 +526,10 @@ void evict_inodes(struct super_block *sb)
spin_unlock(&inode->i_lock);
/*
- * Move the inode off the IO lists and LRU once I_FREEING is
- * set so that it won't get moved back on there if it is dirty.
+ * Move the inode off the LRU once I_FREEING is set so that it
+ * won't get moved back on.
*/
list_move(&inode->i_lru, &dispose);
- list_del_init(&inode->i_wb_list);
}
spin_unlock(&inode_lock);
@@ -570,11 +571,10 @@ int invalidate_inodes(struct super_block *sb)
spin_unlock(&inode->i_lock);
/*
- * Move the inode off the IO lists and LRU once I_FREEING is
- * set so that it won't get moved back on there if it is dirty.
+ * Move the inode off the LRU once I_FREEING is
+ * set so that it won't get moved back on.
*/
list_move(&inode->i_lru, &dispose);
- list_del_init(&inode->i_wb_list);
}
spin_unlock(&inode_lock);
@@ -673,11 +673,10 @@ static void prune_icache(int nr_to_scan)
spin_unlock(&inode->i_lock);
/*
- * Move the inode off the IO lists and LRU once I_FREEING is
- * set so that it won't get moved back on there if it is dirty.
+ * Move the inode off the LRU once I_FREEING is
+ * set so that it won't get moved back on.
*/
list_move(&inode->i_lru, &freeable);
- list_del_init(&inode->i_wb_list);
percpu_counter_dec(&nr_inodes_unused);
}
if (current_is_kswapd())
@@ -1409,16 +1408,16 @@ static void iput_final(struct inode *inode)
else
drop = generic_drop_inode(inode);
+ if (!drop && (sb->s_flags & MS_ACTIVE)) {
+ inode->i_state |= I_REFERENCED;
+ if (!(inode->i_state & (I_DIRTY|I_SYNC)))
+ inode_lru_list_add(inode);
+ spin_unlock(&inode->i_lock);
+ spin_unlock(&inode_lock);
+ return;
+ }
+
if (!drop) {
- if (sb->s_flags & MS_ACTIVE) {
- inode->i_state |= I_REFERENCED;
- if (!(inode->i_state & (I_DIRTY|I_SYNC))) {
- inode_lru_list_add(inode);
- }
- spin_unlock(&inode->i_lock);
- spin_unlock(&inode_lock);
- return;
- }
inode->i_state |= I_WILL_FREE;
spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
@@ -1427,26 +1426,14 @@ static void iput_final(struct inode *inode)
spin_lock(&inode->i_lock);
WARN_ON(inode->i_state & I_NEW);
inode->i_state &= ~I_WILL_FREE;
- __remove_inode_hash(inode);
}
inode->i_state |= I_FREEING;
- spin_unlock(&inode->i_lock);
-
- /*
- * Move the inode off the IO lists and LRU once I_FREEING is
- * set so that it won't get moved back on there if it is dirty.
- */
inode_lru_list_del(inode);
- list_del_init(&inode->i_wb_list);
-
- __inode_sb_list_del(inode);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
+
evict(inode);
- remove_inode_hash(inode);
- wake_up_inode(inode);
- BUG_ON(inode->i_state != (I_FREEING | I_CLEAR));
- destroy_inode(inode);
}
/**
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] fs: Lock the inode LRU list separately
2010-10-27 23:02 fs: break out inode LRU operations from inode_lock V2 Dave Chinner
2010-10-27 23:02 ` [PATCH 1/4] fs: protect inode->i_state with inode->i_lock Dave Chinner
2010-10-27 23:02 ` [PATCH 2/4] fs: factor inode disposal Dave Chinner
@ 2010-10-27 23:02 ` Dave Chinner
2010-10-27 23:02 ` [PATCH 4/4] fs: remove inode_lock from iput_final and prune_icache Dave Chinner
2010-10-28 13:58 ` fs: break out inode LRU operations from inode_lock V2 Christoph Hellwig
4 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2010-10-27 23:02 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel
From: Dave Chinner <dchinner@redhat.com>
Introduce the inode_lru_lock to protect the inode_lru list. This
lock is nested inside the inode->i_lock to allow the inode to be
added to the LRU list in iput_final without needing to deal with
lock inversions. This keeps iput_final() clean and neat.
Further, where marking the inode I_FREEING and removing it from the
LRU, move the LRU list manipulation within the inode->i_lock to keep
the list manipulation consistent with iput_final. This also means
that most of the open coded LRU list removal + unused inode
accounting can now use the inode_lru_list_del() wrappers which
cleans the code up further.
However, this locking change means what the LRU traversal in
prune_icache() inverts this lock ordering and needs to use trylock
semantics on the inode->i_lock to avoid deadlocking. In these cases,
if we fail to lock the inode we move it to the back of the LRU to
prevent spinning on it.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/inode.c | 45 +++++++++++++++++++++++++++++----------------
1 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index bd1688b..da741e7 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -30,10 +30,13 @@
*
* inode->i_lock protects:
* inode->i_state, __iget()
+ * inode_lru_lock protects:
+ * inode_lru, inode->i_lru
*
* Lock ordering:
* inode_lock
* inode->i_lock
+ * inode_lru_lock
*/
/*
@@ -83,6 +86,7 @@ static unsigned int i_hash_shift __read_mostly;
*/
static LIST_HEAD(inode_lru);
+static DEFINE_SPINLOCK(inode_lru_lock);
static struct hlist_head *inode_hashtable __read_mostly;
/*
@@ -343,18 +347,22 @@ EXPORT_SYMBOL(ihold);
static void inode_lru_list_add(struct inode *inode)
{
+ spin_lock(&inode_lru_lock);
if (list_empty(&inode->i_lru)) {
list_add(&inode->i_lru, &inode_lru);
percpu_counter_inc(&nr_inodes_unused);
}
+ spin_unlock(&inode_lru_lock);
}
static void inode_lru_list_del(struct inode *inode)
{
+ spin_lock(&inode_lru_lock);
if (!list_empty(&inode->i_lru)) {
list_del_init(&inode->i_lru);
percpu_counter_dec(&nr_inodes_unused);
}
+ spin_unlock(&inode_lru_lock);
}
static inline void __inode_sb_list_add(struct inode *inode)
@@ -521,15 +529,10 @@ void evict_inodes(struct super_block *sb)
}
inode->i_state |= I_FREEING;
- if (!(inode->i_state & (I_DIRTY | I_SYNC)))
- percpu_counter_dec(&nr_inodes_unused);
+ inode_lru_list_del(inode);
spin_unlock(&inode->i_lock);
- /*
- * Move the inode off the LRU once I_FREEING is set so that it
- * won't get moved back on.
- */
- list_move(&inode->i_lru, &dispose);
+ list_add(&inode->i_lru, &dispose);
}
spin_unlock(&inode_lock);
@@ -566,15 +569,10 @@ int invalidate_inodes(struct super_block *sb)
}
inode->i_state |= I_FREEING;
- if (!(inode->i_state & (I_DIRTY | I_SYNC)))
- percpu_counter_dec(&nr_inodes_unused);
+ inode_lru_list_del(inode);
spin_unlock(&inode->i_lock);
- /*
- * Move the inode off the LRU once I_FREEING is
- * set so that it won't get moved back on.
- */
- list_move(&inode->i_lru, &dispose);
+ list_add(&inode->i_lru, &dispose);
}
spin_unlock(&inode_lock);
@@ -621,6 +619,7 @@ static void prune_icache(int nr_to_scan)
down_read(&iprune_sem);
spin_lock(&inode_lock);
+ spin_lock(&inode_lru_lock);
for (nr_scanned = 0; nr_scanned < nr_to_scan; nr_scanned++) {
struct inode *inode;
@@ -630,10 +629,19 @@ static void prune_icache(int nr_to_scan)
inode = list_entry(inode_lru.prev, struct inode, i_lru);
/*
+ * we are inverting the inode_lru_lock/inode->i_lock here,
+ * so use a trylock. If we fail to get the lock, just move the
+ * inode to the back of the list so we don't spin on it.
+ */
+ if (!spin_trylock(&inode->i_lock)) {
+ list_move(&inode->i_lru, &inode_lru);
+ continue;
+ }
+
+ /*
* Referenced or dirty inodes are still in use. Give them
* another pass through the LRU as we canot reclaim them now.
*/
- spin_lock(&inode->i_lock);
if (atomic_read(&inode->i_count) ||
(inode->i_state & ~I_REFERENCED)) {
spin_unlock(&inode->i_lock);
@@ -652,17 +660,21 @@ static void prune_icache(int nr_to_scan)
if (inode_has_buffers(inode) || inode->i_data.nrpages) {
__iget(inode);
spin_unlock(&inode->i_lock);
+ spin_unlock(&inode_lru_lock);
spin_unlock(&inode_lock);
if (remove_inode_buffers(inode))
reap += invalidate_mapping_pages(&inode->i_data,
0, -1);
iput(inode);
spin_lock(&inode_lock);
+ spin_lock(&inode_lru_lock);
if (inode != list_entry(inode_lru.next,
struct inode, i_lru))
continue; /* wrong inode or list_empty */
- spin_lock(&inode->i_lock);
+ /* avoid lock inversions with trylock */
+ if (!spin_trylock(&inode->i_lock))
+ continue;
if (!can_unuse(inode)) {
spin_unlock(&inode->i_lock);
continue;
@@ -683,6 +695,7 @@ static void prune_icache(int nr_to_scan)
__count_vm_events(KSWAPD_INODESTEAL, reap);
else
__count_vm_events(PGINODESTEAL, reap);
+ spin_unlock(&inode_lru_lock);
spin_unlock(&inode_lock);
dispose_list(&freeable);
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] fs: remove inode_lock from iput_final and prune_icache
2010-10-27 23:02 fs: break out inode LRU operations from inode_lock V2 Dave Chinner
` (2 preceding siblings ...)
2010-10-27 23:02 ` [PATCH 3/4] fs: Lock the inode LRU list separately Dave Chinner
@ 2010-10-27 23:02 ` Dave Chinner
2010-10-28 12:00 ` Christoph Hellwig
2010-10-28 13:58 ` fs: break out inode LRU operations from inode_lock V2 Christoph Hellwig
4 siblings, 1 reply; 11+ messages in thread
From: Dave Chinner @ 2010-10-27 23:02 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel
From: Dave Chinner <dchinner@redhat.com>
Now that inode state changes are protected by the inode->i_lock and
the inode LRU manipulations by the inode_lru_lock, we can remove the
inode_lock from prune_icache and the initial part of iput_final().
instead of using the inode_lock to protect the inode during
iput_final, use the inode->i_lock instead. This protects the inode
against new references being taken while we change the inode state
to I_FREEING, as well as preventing prune_icache from grabbing the
inode while we are manipulating it. Hence we no longer need the
inode_lock in iput_final prior to setting I_FREEING on the inode.
For prune_icache, we no longer need the inode_lock to protect the
LRU list, and the inodes themselves are protected against freeing
races by the inode->i_lock. Hence we can lift the inode_lock from
prune_icache as well.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/inode.c | 15 +++------------
1 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index da741e7..f64aeda 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -618,7 +618,6 @@ static void prune_icache(int nr_to_scan)
unsigned long reap = 0;
down_read(&iprune_sem);
- spin_lock(&inode_lock);
spin_lock(&inode_lru_lock);
for (nr_scanned = 0; nr_scanned < nr_to_scan; nr_scanned++) {
struct inode *inode;
@@ -644,8 +643,8 @@ static void prune_icache(int nr_to_scan)
*/
if (atomic_read(&inode->i_count) ||
(inode->i_state & ~I_REFERENCED)) {
- spin_unlock(&inode->i_lock);
list_del_init(&inode->i_lru);
+ spin_unlock(&inode->i_lock);
percpu_counter_dec(&nr_inodes_unused);
continue;
}
@@ -653,20 +652,18 @@ static void prune_icache(int nr_to_scan)
/* recently referenced inodes get one more pass */
if (inode->i_state & I_REFERENCED) {
inode->i_state &= ~I_REFERENCED;
- spin_unlock(&inode->i_lock);
list_move(&inode->i_lru, &inode_lru);
+ spin_unlock(&inode->i_lock);
continue;
}
if (inode_has_buffers(inode) || inode->i_data.nrpages) {
__iget(inode);
spin_unlock(&inode->i_lock);
spin_unlock(&inode_lru_lock);
- spin_unlock(&inode_lock);
if (remove_inode_buffers(inode))
reap += invalidate_mapping_pages(&inode->i_data,
0, -1);
iput(inode);
- spin_lock(&inode_lock);
spin_lock(&inode_lru_lock);
if (inode != list_entry(inode_lru.next,
@@ -696,7 +693,6 @@ static void prune_icache(int nr_to_scan)
else
__count_vm_events(PGINODESTEAL, reap);
spin_unlock(&inode_lru_lock);
- spin_unlock(&inode_lock);
dispose_list(&freeable);
up_read(&iprune_sem);
@@ -1413,7 +1409,6 @@ static void iput_final(struct inode *inode)
const struct super_operations *op = inode->i_sb->s_op;
int drop;
- spin_lock(&inode->i_lock);
WARN_ON(inode->i_state & I_NEW);
if (op && op->drop_inode)
@@ -1426,16 +1421,13 @@ static void iput_final(struct inode *inode)
if (!(inode->i_state & (I_DIRTY|I_SYNC)))
inode_lru_list_add(inode);
spin_unlock(&inode->i_lock);
- spin_unlock(&inode_lock);
return;
}
if (!drop) {
inode->i_state |= I_WILL_FREE;
spin_unlock(&inode->i_lock);
- spin_unlock(&inode_lock);
write_inode_now(inode, 1);
- spin_lock(&inode_lock);
spin_lock(&inode->i_lock);
WARN_ON(inode->i_state & I_NEW);
inode->i_state &= ~I_WILL_FREE;
@@ -1444,7 +1436,6 @@ static void iput_final(struct inode *inode)
inode->i_state |= I_FREEING;
inode_lru_list_del(inode);
spin_unlock(&inode->i_lock);
- spin_unlock(&inode_lock);
evict(inode);
}
@@ -1463,7 +1454,7 @@ void iput(struct inode *inode)
if (inode) {
BUG_ON(inode->i_state & I_CLEAR);
- if (atomic_dec_and_lock(&inode->i_count, &inode_lock))
+ if (atomic_dec_and_lock(&inode->i_count, &inode->i_lock))
iput_final(inode);
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] fs: remove inode_lock from iput_final and prune_icache
2010-10-27 23:02 ` [PATCH 4/4] fs: remove inode_lock from iput_final and prune_icache Dave Chinner
@ 2010-10-28 12:00 ` Christoph Hellwig
2010-10-28 21:41 ` Dave Chinner
0 siblings, 1 reply; 11+ messages in thread
From: Christoph Hellwig @ 2010-10-28 12:00 UTC (permalink / raw)
To: Dave Chinner; +Cc: viro, linux-fsdevel, linux-kernel
Looks good - although at this point you could already remove inode_lock
from igrab, too.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: fs: break out inode LRU operations from inode_lock V2
2010-10-27 23:02 fs: break out inode LRU operations from inode_lock V2 Dave Chinner
` (3 preceding siblings ...)
2010-10-27 23:02 ` [PATCH 4/4] fs: remove inode_lock from iput_final and prune_icache Dave Chinner
@ 2010-10-28 13:58 ` Christoph Hellwig
4 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2010-10-28 13:58 UTC (permalink / raw)
To: Dave Chinner; +Cc: viro, linux-fsdevel, linux-kernel
On Thu, Oct 28, 2010 at 10:02:14AM +1100, Dave Chinner wrote:
> Hi Al,
>
> The following patches break the inode LRU operations and the first
> half of iput_final() out from under the inode_lock. The patches
> should apply to your current merge-stem tree.
The series looks good to me and survives QA so far.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] fs: remove inode_lock from iput_final and prune_icache
2010-10-28 12:00 ` Christoph Hellwig
@ 2010-10-28 21:41 ` Dave Chinner
0 siblings, 0 replies; 11+ messages in thread
From: Dave Chinner @ 2010-10-28 21:41 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: viro, linux-fsdevel, linux-kernel
On Thu, Oct 28, 2010 at 08:00:57AM -0400, Christoph Hellwig wrote:
> Looks good - although at this point you could already remove inode_lock
> from igrab, too.
Yes, it probably can go at this point - after the next three patches
it's pretty much the only place the lock is left, so it's not really
protecting anything anymore...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] fs: protect inode->i_state with inode->i_lock
2010-10-27 4:23 ` [PATCH 1/4] fs: protect inode->i_state with inode->i_lock Dave Chinner
2010-10-27 7:07 ` Christian Stroetmann
@ 2010-10-27 8:58 ` Christoph Hellwig
1 sibling, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2010-10-27 8:58 UTC (permalink / raw)
To: Dave Chinner; +Cc: viro, linux-fsdevel, linux-kernel
On Wed, Oct 27, 2010 at 03:23:01PM +1100, Dave Chinner wrote:
> 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))
> + spin_lock(&inode->i_lock);
> + if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
> + spin_unlock(&inode->i_lock);
> continue;
> + }
> + spin_unlock(&inode->i_lock);
> if (inode->i_mapping->nrpages == 0)
> continue;
> __iget(inode);
If you want to remove inode_lock from the lru scanning later you already
need to extend i_lock coverage to include __iget here. Otherwise we
could race to mark the inode as I_FREEING or I_WILL_FREE before we
grabbed a reference after your patchset.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] fs: protect inode->i_state with inode->i_lock
2010-10-27 4:23 ` [PATCH 1/4] fs: protect inode->i_state with inode->i_lock Dave Chinner
@ 2010-10-27 7:07 ` Christian Stroetmann
2010-10-27 8:58 ` Christoph Hellwig
1 sibling, 0 replies; 11+ messages in thread
From: Christian Stroetmann @ 2010-10-27 7:07 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-fsdevel, linux-kernel
Aloha;
some typos
On the 27.10.2010 06:23, Dave Chinner wrote:
> From: Dave Chinner<dchinner@redhat.com>
>
> Protect inode state transitions and validity checks with the
> inode->i_lock. This enables us to make inode state transitions
> independently of the inode_lock and is the first step to peeling
> away the inode_lock from the code.
>
> Signed-off-by: Dave Chinner<dchinner@redhat.com>
> ---
> fs/drop_caches.c | 6 +++-
> fs/fs-writeback.c | 31 ++++++++++++++--
> fs/inode.c | 95 +++++++++++++++++++++++++++++++++++++++---------
> fs/notify/inode_mark.c | 17 ++++++---
> fs/quota/dquot.c | 6 +++-
> 5 files changed, 127 insertions(+), 28 deletions(-)
>
> diff --git a/fs/drop_caches.c b/fs/drop_caches.c
> index 2195c21..c495fc3 100644
> --- a/fs/drop_caches.c
> +++ b/fs/drop_caches.c
> @@ -18,8 +18,12 @@ static void drop_pagecache_sb(struct super_block *sb, void *unused)
>
> 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))
> + spin_lock(&inode->i_lock);
> + if (inode->i_state& (I_FREEING|I_WILL_FREE|I_NEW)) {
> + spin_unlock(&inode->i_lock);
> continue;
> + }
> + spin_unlock(&inode->i_lock);
> if (inode->i_mapping->nrpages == 0)
> continue;
> __iget(inode);
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index f6af81a..bd9204d 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -293,9 +293,11 @@ static void inode_wait_for_writeback(struct inode *inode)
>
> wqh = bit_waitqueue(&inode->i_state, __I_SYNC);
> while (inode->i_state& I_SYNC) {
> + spin_unlock(&inode->i_lock);
> spin_unlock(&inode_lock);
> __wait_on_bit(wqh,&wq, inode_wait, TASK_UNINTERRUPTIBLE);
> spin_lock(&inode_lock);
> + spin_lock(&inode->i_lock);
> }
> }
>
> @@ -319,6 +321,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> unsigned dirty;
> int ret;
>
> + spin_lock(&inode->i_lock);
> if (!atomic_read(&inode->i_count))
> WARN_ON(!(inode->i_state& (I_WILL_FREE|I_FREEING)));
> else
> @@ -334,6 +337,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> * completed a full scan of b_io.
> */
> if (wbc->sync_mode != WB_SYNC_ALL) {
> + spin_unlock(&inode->i_lock);
> requeue_io(inode);
> return 0;
> }
> @@ -349,6 +353,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> /* Set I_SYNC, reset I_DIRTY_PAGES */
> inode->i_state |= I_SYNC;
> inode->i_state&= ~I_DIRTY_PAGES;
> + spin_unlock(&inode->i_lock);
> spin_unlock(&inode_lock);
>
> ret = do_writepages(mapping, wbc);
> @@ -370,8 +375,10 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> * write_inode()
> */
> spin_lock(&inode_lock);
> + spin_lock(&inode->i_lock);
> dirty = inode->i_state& I_DIRTY;
> inode->i_state&= ~(I_DIRTY_SYNC | I_DIRTY_DATASYNC);
> + spin_unlock(&inode->i_lock);
> spin_unlock(&inode_lock);
> /* Don't write the inode if only I_DIRTY_PAGES was set */
end point?!
> if (dirty& (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
> @@ -381,6 +388,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> }
>
> spin_lock(&inode_lock);
> + spin_lock(&inode->i_lock);
> inode->i_state&= ~I_SYNC;
> if (!(inode->i_state& I_FREEING)) {
> if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
> @@ -422,6 +430,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
> }
> }
> inode_sync_complete(inode);
> + spin_unlock(&inode->i_lock);
> return ret;
> }
>
> @@ -492,10 +501,13 @@ static int writeback_sb_inodes(struct super_block *sb, struct bdi_writeback *wb,
> * kind does not need peridic writeout yet, and for the latter
periodic
> * kind writeout is handled by the freer.
> */
> + spin_lock(&inode->i_lock);
> if (inode->i_state& (I_NEW | I_FREEING | I_WILL_FREE)) {
> + spin_unlock(&inode->i_lock);
> requeue_io(inode);
> continue;
> }
> + spin_unlock(&inode->i_lock);
>
> /*
> * Was this inode dirtied after sync_sb_inodes was called?
> @@ -681,7 +693,9 @@ static long wb_writeback(struct bdi_writeback *wb,
> if (!list_empty(&wb->b_more_io)) {
> inode = wb_inode(wb->b_more_io.prev);
> trace_wbc_writeback_wait(&wbc, wb->bdi);
> + spin_lock(&inode->i_lock);
> inode_wait_for_writeback(inode);
> + spin_unlock(&inode->i_lock);
> }
> spin_unlock(&inode_lock);
> }
> @@ -947,6 +961,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> block_dump___mark_inode_dirty(inode);
>
> spin_lock(&inode_lock);
> + spin_lock(&inode->i_lock);
> if ((inode->i_state& flags) != flags) {
> const int was_dirty = inode->i_state& I_DIRTY;
>
> @@ -958,7 +973,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> * superblock list, based upon its state.
> */
> if (inode->i_state& I_SYNC)
> - goto out;
> + goto out_unlock_inode;
>
> /*
> * Only add valid (hashed) inodes to the superblock's
> @@ -966,11 +981,12 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> */
> if (!S_ISBLK(inode->i_mode)) {
> if (inode_unhashed(inode))
> - goto out;
> + goto out_unlock_inode;
> }
> if (inode->i_state& I_FREEING)
> - goto out;
> + goto out_unlock_inode;
>
> + spin_unlock(&inode->i_lock);
> /*
> * If the inode was already on b_dirty/b_io/b_more_io, don't
> * reposition it (that would break b_dirty time-ordering).
> @@ -995,7 +1011,10 @@ void __mark_inode_dirty(struct inode *inode, int flags)
> inode->dirtied_when = jiffies;
> list_move(&inode->i_wb_list,&bdi->wb.b_dirty);
> }
> + goto out;
> }
> +out_unlock_inode:
> + spin_unlock(&inode->i_lock);
> out:
> spin_unlock(&inode_lock);
>
> @@ -1043,8 +1062,12 @@ static void wait_sb_inodes(struct super_block *sb)
> list_for_each_entry(inode,&sb->s_inodes, i_sb_list) {
> struct address_space *mapping;
>
> - if (inode->i_state& (I_FREEING|I_WILL_FREE|I_NEW))
> + spin_lock(&inode->i_lock);
> + if (inode->i_state& (I_FREEING|I_WILL_FREE|I_NEW)) {
> + spin_unlock(&inode->i_lock);
> continue;
> + }
> + spin_unlock(&inode->i_lock);
> mapping = inode->i_mapping;
> if (mapping->nrpages == 0)
> continue;
> diff --git a/fs/inode.c b/fs/inode.c
> index a6d6068..eaba6ce 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -26,6 +26,17 @@
> #include<linux/posix_acl.h>
>
> /*
> + * inode locking rules.
> + *
> + * inode->i_lock protects:
> + * i_state
> + *
> + * Lock ordering:
> + * inode_lock
> + * inode->i_lock
> + */
> +
> +/*
> * This is needed for the following functions:
> * - inode_has_buffers
> * - invalidate_bdev
> @@ -429,7 +440,9 @@ void end_writeback(struct inode *inode)
> BUG_ON(!(inode->i_state& I_FREEING));
> BUG_ON(inode->i_state& I_CLEAR);
> inode_sync_wait(inode);
> + spin_lock(&inode->i_lock);
> inode->i_state = I_FREEING | I_CLEAR;
> + spin_unlock(&inode->i_lock);
> }
> EXPORT_SYMBOL(end_writeback);
>
> @@ -498,12 +511,17 @@ void evict_inodes(struct super_block *sb)
> if (atomic_read(&inode->i_count))
> continue;
>
> + spin_lock(&inode->i_lock);
> if (inode->i_state& (I_NEW | I_FREEING | I_WILL_FREE)) {
> + spin_unlock(&inode->i_lock);
> WARN_ON(1);
> continue;
> }
>
> inode->i_state |= I_FREEING;
> + if (!(inode->i_state& (I_DIRTY | I_SYNC)))
> + percpu_counter_dec(&nr_inodes_unused);
> + spin_unlock(&inode->i_lock);
>
> /*
> * Move the inode off the IO lists and LRU once I_FREEING is
> @@ -511,8 +529,6 @@ void evict_inodes(struct super_block *sb)
> */
> list_move(&inode->i_lru,&dispose);
> list_del_init(&inode->i_wb_list);
> - if (!(inode->i_state& (I_DIRTY | I_SYNC)))
> - percpu_counter_dec(&nr_inodes_unused);
> }
> spin_unlock(&inode_lock);
>
> @@ -521,7 +537,7 @@ void evict_inodes(struct super_block *sb)
> }
>
> /**
> - * invalidate_inodes - attempt to free all inodes on a superblock
> + * nvalidate_inodes - attempt to free all inodes on a superblock
attempts
invalidate_inodes
> * @sb: superblock to operate on
> *
> * Attempts to free all inodes for a given superblock. If there were any
> @@ -537,14 +553,21 @@ int invalidate_inodes(struct super_block *sb)
>
> spin_lock(&inode_lock);
> list_for_each_entry_safe(inode, next,&sb->s_inodes, i_sb_list) {
> - if (inode->i_state& (I_NEW | I_FREEING | I_WILL_FREE))
> + spin_lock(&inode->i_lock);
> + if (inode->i_state& (I_NEW | I_FREEING | I_WILL_FREE)) {
> + spin_unlock(&inode->i_lock);
> continue;
> + }
> if (atomic_read(&inode->i_count)) {
> + spin_unlock(&inode->i_lock);
> busy = 1;
> continue;
> }
>
> inode->i_state |= I_FREEING;
> + if (!(inode->i_state& (I_DIRTY | I_SYNC)))
> + percpu_counter_dec(&nr_inodes_unused);
> + spin_unlock(&inode->i_lock);
>
> /*
> * Move the inode off the IO lists and LRU once I_FREEING is
> @@ -552,8 +575,6 @@ int invalidate_inodes(struct super_block *sb)
> */
> list_move(&inode->i_lru,&dispose);
> list_del_init(&inode->i_wb_list);
> - if (!(inode->i_state& (I_DIRTY | I_SYNC)))
> - percpu_counter_dec(&nr_inodes_unused);
> }
> spin_unlock(&inode_lock);
>
> @@ -612,8 +633,10 @@ static void prune_icache(int nr_to_scan)
> * Referenced or dirty inodes are still in use. Give them
> * another pass through the LRU as we canot reclaim them now.
cannot
> */
> + spin_lock(&inode->i_lock);
> if (atomic_read(&inode->i_count) ||
> (inode->i_state& ~I_REFERENCED)) {
> + spin_unlock(&inode->i_lock);
> list_del_init(&inode->i_lru);
> percpu_counter_dec(&nr_inodes_unused);
> continue;
> @@ -621,12 +644,14 @@ static void prune_icache(int nr_to_scan)
>
> /* recently referenced inodes get one more pass */
> if (inode->i_state& I_REFERENCED) {
> - list_move(&inode->i_lru,&inode_lru);
> inode->i_state&= ~I_REFERENCED;
> + spin_unlock(&inode->i_lock);
> + list_move(&inode->i_lru,&inode_lru);
> continue;
> }
> if (inode_has_buffers(inode) || inode->i_data.nrpages) {
> __iget(inode);
> + spin_unlock(&inode->i_lock);
> spin_unlock(&inode_lock);
> if (remove_inode_buffers(inode))
> reap += invalidate_mapping_pages(&inode->i_data,
> @@ -637,11 +662,15 @@ static void prune_icache(int nr_to_scan)
> if (inode != list_entry(inode_lru.next,
> struct inode, i_lru))
> continue; /* wrong inode or list_empty */
> - if (!can_unuse(inode))
> + spin_lock(&inode->i_lock);
> + if (!can_unuse(inode)) {
> + spin_unlock(&inode->i_lock);
> continue;
> + }
> }
> WARN_ON(inode->i_state& I_NEW);
> inode->i_state |= I_FREEING;
> + spin_unlock(&inode->i_lock);
>
> /*
> * Move the inode off the IO lists and LRU once I_FREEING is
> @@ -708,10 +737,12 @@ repeat:
> continue;
> if (!test(inode, data))
> continue;
> + spin_lock(&inode->i_lock);
> if (inode->i_state& (I_FREEING|I_WILL_FREE)) {
> __wait_on_freeing_inode(inode);
> goto repeat;
> }
> + spin_unlock(&inode->i_lock);
> __iget(inode);
> return inode;
> }
> @@ -734,10 +765,12 @@ repeat:
> continue;
> if (inode->i_sb != sb)
> continue;
> + spin_lock(&inode->i_lock);
> if (inode->i_state& (I_FREEING|I_WILL_FREE)) {
> __wait_on_freeing_inode(inode);
> goto repeat;
> }
> + spin_unlock(&inode->i_lock);
> __iget(inode);
> return inode;
> }
> @@ -803,8 +836,10 @@ struct inode *new_inode(struct super_block *sb)
> inode = alloc_inode(sb);
> if (inode) {
> spin_lock(&inode_lock);
> - __inode_sb_list_add(inode);
> + spin_lock(&inode->i_lock);
> inode->i_state = 0;
> + spin_unlock(&inode->i_lock);
> + __inode_sb_list_add(inode);
> spin_unlock(&inode_lock);
> }
> return inode;
> @@ -871,9 +906,11 @@ static struct inode *get_new_inode(struct super_block *sb,
> if (set(inode, data))
> goto set_failed;
>
> + spin_lock(&inode->i_lock);
> + inode->i_state = I_NEW;
> + spin_unlock(&inode->i_lock);
> hlist_add_head(&inode->i_hash, head);
> __inode_sb_list_add(inode);
> - inode->i_state = I_NEW;
> spin_unlock(&inode_lock);
>
> /* Return the locked inode with I_NEW set, the
> @@ -917,10 +954,12 @@ static struct inode *get_new_inode_fast(struct super_block *sb,
> /* We released the lock, so.. */
so what? :-)
> old = find_inode_fast(sb, head, ino);
> if (!old) {
> + spin_lock(&inode->i_lock);
> inode->i_ino = ino;
> + inode->i_state = I_NEW;
> + spin_unlock(&inode->i_lock);
> hlist_add_head(&inode->i_hash, head);
> __inode_sb_list_add(inode);
> - inode->i_state = I_NEW;
> spin_unlock(&inode_lock);
>
> /* Return the locked inode with I_NEW set, the
> @@ -1005,15 +1044,19 @@ EXPORT_SYMBOL(iunique);
> struct inode *igrab(struct inode *inode)
> {
> spin_lock(&inode_lock);
> - if (!(inode->i_state& (I_FREEING|I_WILL_FREE)))
> + spin_lock(&inode->i_lock);
> + if (!(inode->i_state& (I_FREEING|I_WILL_FREE))) {
> __iget(inode);
> - else
> + spin_unlock(&inode->i_lock);
> + } else {
> + spin_unlock(&inode->i_lock);
> /*
> * Handle the case where s_op->clear_inode is not been
> * called yet, and somebody is calling igrab
> * while the inode is getting freed.
> */
> inode = NULL;
> + }
> spin_unlock(&inode_lock);
> return inode;
> }
> @@ -1242,7 +1285,9 @@ int insert_inode_locked(struct inode *inode)
> ino_t ino = inode->i_ino;
> struct hlist_head *head = inode_hashtable + hash(sb, ino);
>
> + spin_lock(&inode->i_lock);
> inode->i_state |= I_NEW;
> + spin_unlock(&inode->i_lock);
> while (1) {
> struct hlist_node *node;
> struct inode *old = NULL;
> @@ -1252,8 +1297,11 @@ int insert_inode_locked(struct inode *inode)
> continue;
> if (old->i_sb != sb)
> continue;
> - if (old->i_state& (I_FREEING|I_WILL_FREE))
> + spin_lock(&old->i_lock);
> + if (old->i_state& (I_FREEING|I_WILL_FREE)) {
> + spin_unlock(&old->i_lock);
> continue;
> + }
> break;
> }
> if (likely(!node)) {
> @@ -1261,6 +1309,7 @@ int insert_inode_locked(struct inode *inode)
> spin_unlock(&inode_lock);
> return 0;
> }
> + spin_unlock(&old->i_lock);
> __iget(old);
> spin_unlock(&inode_lock);
> wait_on_inode(old);
> @@ -1279,7 +1328,9 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
> struct super_block *sb = inode->i_sb;
> struct hlist_head *head = inode_hashtable + hash(sb, hashval);
>
> + spin_lock(&inode->i_lock);
> inode->i_state |= I_NEW;
> + spin_unlock(&inode->i_lock);
>
> while (1) {
> struct hlist_node *node;
> @@ -1291,8 +1342,11 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
> continue;
> if (!test(old, data))
> continue;
> - if (old->i_state& (I_FREEING|I_WILL_FREE))
> + spin_lock(&old->i_lock);
> + if (old->i_state& (I_FREEING|I_WILL_FREE)) {
> + spin_unlock(&old->i_lock);
> continue;
> + }
> break;
> }
> if (likely(!node)) {
> @@ -1300,6 +1354,7 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
> spin_unlock(&inode_lock);
> return 0;
> }
> + spin_unlock(&old->i_lock);
> __iget(old);
> spin_unlock(&inode_lock);
> wait_on_inode(old);
> @@ -1346,6 +1401,9 @@ static void iput_final(struct inode *inode)
> const struct super_operations *op = inode->i_sb->s_op;
> int drop;
>
> + spin_lock(&inode->i_lock);
> + WARN_ON(inode->i_state& I_NEW);
> +
> if (op&& op->drop_inode)
> drop = op->drop_inode(inode);
> else
> @@ -1357,21 +1415,23 @@ static void iput_final(struct inode *inode)
> if (!(inode->i_state& (I_DIRTY|I_SYNC))) {
> inode_lru_list_add(inode);
> }
> + spin_unlock(&inode->i_lock);
> spin_unlock(&inode_lock);
> return;
> }
> - WARN_ON(inode->i_state& I_NEW);
> inode->i_state |= I_WILL_FREE;
> + spin_unlock(&inode->i_lock);
> spin_unlock(&inode_lock);
> write_inode_now(inode, 1);
> spin_lock(&inode_lock);
> + spin_lock(&inode->i_lock);
> WARN_ON(inode->i_state& I_NEW);
> inode->i_state&= ~I_WILL_FREE;
> __remove_inode_hash(inode);
> }
>
> - WARN_ON(inode->i_state& I_NEW);
> inode->i_state |= I_FREEING;
> + spin_unlock(&inode->i_lock);
>
> /*
> * Move the inode off the IO lists and LRU once I_FREEING is
> @@ -1592,6 +1652,7 @@ static void __wait_on_freeing_inode(struct inode *inode)
> DEFINE_WAIT_BIT(wait,&inode->i_state, __I_NEW);
> wq = bit_waitqueue(&inode->i_state, __I_NEW);
> prepare_to_wait(wq,&wait.wait, TASK_UNINTERRUPTIBLE);
> + spin_unlock(&inode->i_lock);
> spin_unlock(&inode_lock);
> schedule();
> finish_wait(wq,&wait.wait);
> diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
> index 21ed106..08f0d16 100644
> --- a/fs/notify/inode_mark.c
> +++ b/fs/notify/inode_mark.c
> @@ -249,8 +249,12 @@ void fsnotify_unmount_inodes(struct list_head *list)
> * I_WILL_FREE, or I_NEW which is fine because by that point
> * the inode cannot have any associated watches.
> */
> - if (inode->i_state& (I_FREEING|I_WILL_FREE|I_NEW))
> + spin_lock(&inode->i_lock);
> + if (inode->i_state& (I_FREEING|I_WILL_FREE|I_NEW)) {
> + spin_unlock(&inode->i_lock);
> continue;
> + }
> + spin_unlock(&inode->i_lock);
>
> /*
> * If i_count is zero, the inode cannot have any watches and
, then the inode
> @@ -272,10 +276,13 @@ void fsnotify_unmount_inodes(struct list_head *list)
>
> /* In case the dropping of a reference would nuke next_i. */
> if ((&next_i->i_sb_list != list)&&
> - atomic_read(&next_i->i_count)&&
> - !(next_i->i_state& (I_FREEING | I_WILL_FREE))) {
> - __iget(next_i);
> - need_iput = next_i;
> + atomic_read(&next_i->i_count)) {
> + spin_lock(&next_i->i_lock);
> + if (!(next_i->i_state& (I_FREEING | I_WILL_FREE))) {
> + __iget(next_i);
> + need_iput = next_i;
> + }
> + spin_unlock(&next_i->i_lock);
> }
>
> /*
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index aad1316..fb7c2c0 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -898,8 +898,12 @@ static void add_dquot_ref(struct super_block *sb, int type)
>
> 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))
> + spin_lock(&inode->i_lock);
> + if (inode->i_state& (I_FREEING|I_WILL_FREE|I_NEW)) {
> + spin_unlock(&inode->i_lock);
> continue;
> + }
> + spin_unlock(&inode->i_lock);
> #ifdef CONFIG_QUOTA_DEBUG
> if (unlikely(inode_get_rsv_space(inode)> 0))
> reserved = 1;
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] fs: protect inode->i_state with inode->i_lock
2010-10-27 4:23 fs: break out inode LRU operations from node_lock Dave Chinner
@ 2010-10-27 4:23 ` Dave Chinner
2010-10-27 7:07 ` Christian Stroetmann
2010-10-27 8:58 ` Christoph Hellwig
0 siblings, 2 replies; 11+ messages in thread
From: Dave Chinner @ 2010-10-27 4:23 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel
From: Dave Chinner <dchinner@redhat.com>
Protect inode state transitions and validity checks with the
inode->i_lock. This enables us to make inode state transitions
independently of the inode_lock and is the first step to peeling
away the inode_lock from the code.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/drop_caches.c | 6 +++-
fs/fs-writeback.c | 31 ++++++++++++++--
fs/inode.c | 95 +++++++++++++++++++++++++++++++++++++++---------
fs/notify/inode_mark.c | 17 ++++++---
fs/quota/dquot.c | 6 +++-
5 files changed, 127 insertions(+), 28 deletions(-)
diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 2195c21..c495fc3 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -18,8 +18,12 @@ static void drop_pagecache_sb(struct super_block *sb, void *unused)
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))
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
+ spin_unlock(&inode->i_lock);
if (inode->i_mapping->nrpages == 0)
continue;
__iget(inode);
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index f6af81a..bd9204d 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -293,9 +293,11 @@ static void inode_wait_for_writeback(struct inode *inode)
wqh = bit_waitqueue(&inode->i_state, __I_SYNC);
while (inode->i_state & I_SYNC) {
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
__wait_on_bit(wqh, &wq, inode_wait, TASK_UNINTERRUPTIBLE);
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
}
}
@@ -319,6 +321,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
unsigned dirty;
int ret;
+ spin_lock(&inode->i_lock);
if (!atomic_read(&inode->i_count))
WARN_ON(!(inode->i_state & (I_WILL_FREE|I_FREEING)));
else
@@ -334,6 +337,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
* completed a full scan of b_io.
*/
if (wbc->sync_mode != WB_SYNC_ALL) {
+ spin_unlock(&inode->i_lock);
requeue_io(inode);
return 0;
}
@@ -349,6 +353,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
/* Set I_SYNC, reset I_DIRTY_PAGES */
inode->i_state |= I_SYNC;
inode->i_state &= ~I_DIRTY_PAGES;
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
ret = do_writepages(mapping, wbc);
@@ -370,8 +375,10 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
* write_inode()
*/
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
dirty = inode->i_state & I_DIRTY;
inode->i_state &= ~(I_DIRTY_SYNC | I_DIRTY_DATASYNC);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
/* Don't write the inode if only I_DIRTY_PAGES was set */
if (dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
@@ -381,6 +388,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
}
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
inode->i_state &= ~I_SYNC;
if (!(inode->i_state & I_FREEING)) {
if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
@@ -422,6 +430,7 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
}
}
inode_sync_complete(inode);
+ spin_unlock(&inode->i_lock);
return ret;
}
@@ -492,10 +501,13 @@ static int writeback_sb_inodes(struct super_block *sb, struct bdi_writeback *wb,
* kind does not need peridic writeout yet, and for the latter
* kind writeout is handled by the freer.
*/
+ spin_lock(&inode->i_lock);
if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
+ spin_unlock(&inode->i_lock);
requeue_io(inode);
continue;
}
+ spin_unlock(&inode->i_lock);
/*
* Was this inode dirtied after sync_sb_inodes was called?
@@ -681,7 +693,9 @@ static long wb_writeback(struct bdi_writeback *wb,
if (!list_empty(&wb->b_more_io)) {
inode = wb_inode(wb->b_more_io.prev);
trace_wbc_writeback_wait(&wbc, wb->bdi);
+ spin_lock(&inode->i_lock);
inode_wait_for_writeback(inode);
+ spin_unlock(&inode->i_lock);
}
spin_unlock(&inode_lock);
}
@@ -947,6 +961,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
block_dump___mark_inode_dirty(inode);
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
if ((inode->i_state & flags) != flags) {
const int was_dirty = inode->i_state & I_DIRTY;
@@ -958,7 +973,7 @@ void __mark_inode_dirty(struct inode *inode, int flags)
* superblock list, based upon its state.
*/
if (inode->i_state & I_SYNC)
- goto out;
+ goto out_unlock_inode;
/*
* Only add valid (hashed) inodes to the superblock's
@@ -966,11 +981,12 @@ void __mark_inode_dirty(struct inode *inode, int flags)
*/
if (!S_ISBLK(inode->i_mode)) {
if (inode_unhashed(inode))
- goto out;
+ goto out_unlock_inode;
}
if (inode->i_state & I_FREEING)
- goto out;
+ goto out_unlock_inode;
+ spin_unlock(&inode->i_lock);
/*
* If the inode was already on b_dirty/b_io/b_more_io, don't
* reposition it (that would break b_dirty time-ordering).
@@ -995,7 +1011,10 @@ void __mark_inode_dirty(struct inode *inode, int flags)
inode->dirtied_when = jiffies;
list_move(&inode->i_wb_list, &bdi->wb.b_dirty);
}
+ goto out;
}
+out_unlock_inode:
+ spin_unlock(&inode->i_lock);
out:
spin_unlock(&inode_lock);
@@ -1043,8 +1062,12 @@ static void wait_sb_inodes(struct super_block *sb)
list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
struct address_space *mapping;
- if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW))
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
+ spin_unlock(&inode->i_lock);
mapping = inode->i_mapping;
if (mapping->nrpages == 0)
continue;
diff --git a/fs/inode.c b/fs/inode.c
index a6d6068..eaba6ce 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -26,6 +26,17 @@
#include <linux/posix_acl.h>
/*
+ * inode locking rules.
+ *
+ * inode->i_lock protects:
+ * i_state
+ *
+ * Lock ordering:
+ * inode_lock
+ * inode->i_lock
+ */
+
+/*
* This is needed for the following functions:
* - inode_has_buffers
* - invalidate_bdev
@@ -429,7 +440,9 @@ void end_writeback(struct inode *inode)
BUG_ON(!(inode->i_state & I_FREEING));
BUG_ON(inode->i_state & I_CLEAR);
inode_sync_wait(inode);
+ spin_lock(&inode->i_lock);
inode->i_state = I_FREEING | I_CLEAR;
+ spin_unlock(&inode->i_lock);
}
EXPORT_SYMBOL(end_writeback);
@@ -498,12 +511,17 @@ void evict_inodes(struct super_block *sb)
if (atomic_read(&inode->i_count))
continue;
+ spin_lock(&inode->i_lock);
if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
+ spin_unlock(&inode->i_lock);
WARN_ON(1);
continue;
}
inode->i_state |= I_FREEING;
+ if (!(inode->i_state & (I_DIRTY | I_SYNC)))
+ percpu_counter_dec(&nr_inodes_unused);
+ spin_unlock(&inode->i_lock);
/*
* Move the inode off the IO lists and LRU once I_FREEING is
@@ -511,8 +529,6 @@ void evict_inodes(struct super_block *sb)
*/
list_move(&inode->i_lru, &dispose);
list_del_init(&inode->i_wb_list);
- if (!(inode->i_state & (I_DIRTY | I_SYNC)))
- percpu_counter_dec(&nr_inodes_unused);
}
spin_unlock(&inode_lock);
@@ -521,7 +537,7 @@ void evict_inodes(struct super_block *sb)
}
/**
- * invalidate_inodes - attempt to free all inodes on a superblock
+ * nvalidate_inodes - attempt to free all inodes on a superblock
* @sb: superblock to operate on
*
* Attempts to free all inodes for a given superblock. If there were any
@@ -537,14 +553,21 @@ int invalidate_inodes(struct super_block *sb)
spin_lock(&inode_lock);
list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) {
- if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE))
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
if (atomic_read(&inode->i_count)) {
+ spin_unlock(&inode->i_lock);
busy = 1;
continue;
}
inode->i_state |= I_FREEING;
+ if (!(inode->i_state & (I_DIRTY | I_SYNC)))
+ percpu_counter_dec(&nr_inodes_unused);
+ spin_unlock(&inode->i_lock);
/*
* Move the inode off the IO lists and LRU once I_FREEING is
@@ -552,8 +575,6 @@ int invalidate_inodes(struct super_block *sb)
*/
list_move(&inode->i_lru, &dispose);
list_del_init(&inode->i_wb_list);
- if (!(inode->i_state & (I_DIRTY | I_SYNC)))
- percpu_counter_dec(&nr_inodes_unused);
}
spin_unlock(&inode_lock);
@@ -612,8 +633,10 @@ static void prune_icache(int nr_to_scan)
* Referenced or dirty inodes are still in use. Give them
* another pass through the LRU as we canot reclaim them now.
*/
+ spin_lock(&inode->i_lock);
if (atomic_read(&inode->i_count) ||
(inode->i_state & ~I_REFERENCED)) {
+ spin_unlock(&inode->i_lock);
list_del_init(&inode->i_lru);
percpu_counter_dec(&nr_inodes_unused);
continue;
@@ -621,12 +644,14 @@ static void prune_icache(int nr_to_scan)
/* recently referenced inodes get one more pass */
if (inode->i_state & I_REFERENCED) {
- list_move(&inode->i_lru, &inode_lru);
inode->i_state &= ~I_REFERENCED;
+ spin_unlock(&inode->i_lock);
+ list_move(&inode->i_lru, &inode_lru);
continue;
}
if (inode_has_buffers(inode) || inode->i_data.nrpages) {
__iget(inode);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
if (remove_inode_buffers(inode))
reap += invalidate_mapping_pages(&inode->i_data,
@@ -637,11 +662,15 @@ static void prune_icache(int nr_to_scan)
if (inode != list_entry(inode_lru.next,
struct inode, i_lru))
continue; /* wrong inode or list_empty */
- if (!can_unuse(inode))
+ spin_lock(&inode->i_lock);
+ if (!can_unuse(inode)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
}
WARN_ON(inode->i_state & I_NEW);
inode->i_state |= I_FREEING;
+ spin_unlock(&inode->i_lock);
/*
* Move the inode off the IO lists and LRU once I_FREEING is
@@ -708,10 +737,12 @@ repeat:
continue;
if (!test(inode, data))
continue;
+ spin_lock(&inode->i_lock);
if (inode->i_state & (I_FREEING|I_WILL_FREE)) {
__wait_on_freeing_inode(inode);
goto repeat;
}
+ spin_unlock(&inode->i_lock);
__iget(inode);
return inode;
}
@@ -734,10 +765,12 @@ repeat:
continue;
if (inode->i_sb != sb)
continue;
+ spin_lock(&inode->i_lock);
if (inode->i_state & (I_FREEING|I_WILL_FREE)) {
__wait_on_freeing_inode(inode);
goto repeat;
}
+ spin_unlock(&inode->i_lock);
__iget(inode);
return inode;
}
@@ -803,8 +836,10 @@ struct inode *new_inode(struct super_block *sb)
inode = alloc_inode(sb);
if (inode) {
spin_lock(&inode_lock);
- __inode_sb_list_add(inode);
+ spin_lock(&inode->i_lock);
inode->i_state = 0;
+ spin_unlock(&inode->i_lock);
+ __inode_sb_list_add(inode);
spin_unlock(&inode_lock);
}
return inode;
@@ -871,9 +906,11 @@ static struct inode *get_new_inode(struct super_block *sb,
if (set(inode, data))
goto set_failed;
+ spin_lock(&inode->i_lock);
+ inode->i_state = I_NEW;
+ spin_unlock(&inode->i_lock);
hlist_add_head(&inode->i_hash, head);
__inode_sb_list_add(inode);
- inode->i_state = I_NEW;
spin_unlock(&inode_lock);
/* Return the locked inode with I_NEW set, the
@@ -917,10 +954,12 @@ static struct inode *get_new_inode_fast(struct super_block *sb,
/* We released the lock, so.. */
old = find_inode_fast(sb, head, ino);
if (!old) {
+ spin_lock(&inode->i_lock);
inode->i_ino = ino;
+ inode->i_state = I_NEW;
+ spin_unlock(&inode->i_lock);
hlist_add_head(&inode->i_hash, head);
__inode_sb_list_add(inode);
- inode->i_state = I_NEW;
spin_unlock(&inode_lock);
/* Return the locked inode with I_NEW set, the
@@ -1005,15 +1044,19 @@ EXPORT_SYMBOL(iunique);
struct inode *igrab(struct inode *inode)
{
spin_lock(&inode_lock);
- if (!(inode->i_state & (I_FREEING|I_WILL_FREE)))
+ spin_lock(&inode->i_lock);
+ if (!(inode->i_state & (I_FREEING|I_WILL_FREE))) {
__iget(inode);
- else
+ spin_unlock(&inode->i_lock);
+ } else {
+ spin_unlock(&inode->i_lock);
/*
* Handle the case where s_op->clear_inode is not been
* called yet, and somebody is calling igrab
* while the inode is getting freed.
*/
inode = NULL;
+ }
spin_unlock(&inode_lock);
return inode;
}
@@ -1242,7 +1285,9 @@ int insert_inode_locked(struct inode *inode)
ino_t ino = inode->i_ino;
struct hlist_head *head = inode_hashtable + hash(sb, ino);
+ spin_lock(&inode->i_lock);
inode->i_state |= I_NEW;
+ spin_unlock(&inode->i_lock);
while (1) {
struct hlist_node *node;
struct inode *old = NULL;
@@ -1252,8 +1297,11 @@ int insert_inode_locked(struct inode *inode)
continue;
if (old->i_sb != sb)
continue;
- if (old->i_state & (I_FREEING|I_WILL_FREE))
+ spin_lock(&old->i_lock);
+ if (old->i_state & (I_FREEING|I_WILL_FREE)) {
+ spin_unlock(&old->i_lock);
continue;
+ }
break;
}
if (likely(!node)) {
@@ -1261,6 +1309,7 @@ int insert_inode_locked(struct inode *inode)
spin_unlock(&inode_lock);
return 0;
}
+ spin_unlock(&old->i_lock);
__iget(old);
spin_unlock(&inode_lock);
wait_on_inode(old);
@@ -1279,7 +1328,9 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
struct super_block *sb = inode->i_sb;
struct hlist_head *head = inode_hashtable + hash(sb, hashval);
+ spin_lock(&inode->i_lock);
inode->i_state |= I_NEW;
+ spin_unlock(&inode->i_lock);
while (1) {
struct hlist_node *node;
@@ -1291,8 +1342,11 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
continue;
if (!test(old, data))
continue;
- if (old->i_state & (I_FREEING|I_WILL_FREE))
+ spin_lock(&old->i_lock);
+ if (old->i_state & (I_FREEING|I_WILL_FREE)) {
+ spin_unlock(&old->i_lock);
continue;
+ }
break;
}
if (likely(!node)) {
@@ -1300,6 +1354,7 @@ int insert_inode_locked4(struct inode *inode, unsigned long hashval,
spin_unlock(&inode_lock);
return 0;
}
+ spin_unlock(&old->i_lock);
__iget(old);
spin_unlock(&inode_lock);
wait_on_inode(old);
@@ -1346,6 +1401,9 @@ static void iput_final(struct inode *inode)
const struct super_operations *op = inode->i_sb->s_op;
int drop;
+ spin_lock(&inode->i_lock);
+ WARN_ON(inode->i_state & I_NEW);
+
if (op && op->drop_inode)
drop = op->drop_inode(inode);
else
@@ -1357,21 +1415,23 @@ static void iput_final(struct inode *inode)
if (!(inode->i_state & (I_DIRTY|I_SYNC))) {
inode_lru_list_add(inode);
}
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
return;
}
- WARN_ON(inode->i_state & I_NEW);
inode->i_state |= I_WILL_FREE;
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
write_inode_now(inode, 1);
spin_lock(&inode_lock);
+ spin_lock(&inode->i_lock);
WARN_ON(inode->i_state & I_NEW);
inode->i_state &= ~I_WILL_FREE;
__remove_inode_hash(inode);
}
- WARN_ON(inode->i_state & I_NEW);
inode->i_state |= I_FREEING;
+ spin_unlock(&inode->i_lock);
/*
* Move the inode off the IO lists and LRU once I_FREEING is
@@ -1592,6 +1652,7 @@ static void __wait_on_freeing_inode(struct inode *inode)
DEFINE_WAIT_BIT(wait, &inode->i_state, __I_NEW);
wq = bit_waitqueue(&inode->i_state, __I_NEW);
prepare_to_wait(wq, &wait.wait, TASK_UNINTERRUPTIBLE);
+ spin_unlock(&inode->i_lock);
spin_unlock(&inode_lock);
schedule();
finish_wait(wq, &wait.wait);
diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
index 21ed106..08f0d16 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -249,8 +249,12 @@ void fsnotify_unmount_inodes(struct list_head *list)
* I_WILL_FREE, or I_NEW which is fine because by that point
* the inode cannot have any associated watches.
*/
- if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW))
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
+ spin_unlock(&inode->i_lock);
/*
* If i_count is zero, the inode cannot have any watches and
@@ -272,10 +276,13 @@ void fsnotify_unmount_inodes(struct list_head *list)
/* In case the dropping of a reference would nuke next_i. */
if ((&next_i->i_sb_list != list) &&
- atomic_read(&next_i->i_count) &&
- !(next_i->i_state & (I_FREEING | I_WILL_FREE))) {
- __iget(next_i);
- need_iput = next_i;
+ atomic_read(&next_i->i_count)) {
+ spin_lock(&next_i->i_lock);
+ if (!(next_i->i_state & (I_FREEING | I_WILL_FREE))) {
+ __iget(next_i);
+ need_iput = next_i;
+ }
+ spin_unlock(&next_i->i_lock);
}
/*
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index aad1316..fb7c2c0 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -898,8 +898,12 @@ static void add_dquot_ref(struct super_block *sb, int type)
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))
+ spin_lock(&inode->i_lock);
+ if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
+ spin_unlock(&inode->i_lock);
continue;
+ }
+ spin_unlock(&inode->i_lock);
#ifdef CONFIG_QUOTA_DEBUG
if (unlikely(inode_get_rsv_space(inode) > 0))
reserved = 1;
--
1.7.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-10-28 21:41 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-27 23:02 fs: break out inode LRU operations from inode_lock V2 Dave Chinner
2010-10-27 23:02 ` [PATCH 1/4] fs: protect inode->i_state with inode->i_lock Dave Chinner
2010-10-27 23:02 ` [PATCH 2/4] fs: factor inode disposal Dave Chinner
2010-10-27 23:02 ` [PATCH 3/4] fs: Lock the inode LRU list separately Dave Chinner
2010-10-27 23:02 ` [PATCH 4/4] fs: remove inode_lock from iput_final and prune_icache Dave Chinner
2010-10-28 12:00 ` Christoph Hellwig
2010-10-28 21:41 ` Dave Chinner
2010-10-28 13:58 ` fs: break out inode LRU operations from inode_lock V2 Christoph Hellwig
-- strict thread matches above, loose matches on Subject: below --
2010-10-27 4:23 fs: break out inode LRU operations from node_lock Dave Chinner
2010-10-27 4:23 ` [PATCH 1/4] fs: protect inode->i_state with inode->i_lock Dave Chinner
2010-10-27 7:07 ` Christian Stroetmann
2010-10-27 8:58 ` Christoph Hellwig
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).