Re: BUG: drop_pagecache_sb vs kjournald lockup

[Jan Kara]

> 2.6.25-rc3, 4p ia64, ext3 root drive.
> 
> I was running an XFS stress test on one of the XFS partitions on
> the machine (zero load on the root ext3 drive), when the system
> locked up in kjournald with this on the console:
> 
> BUG: spinlock lockup on CPU#2, kjournald/2150, a000000100e022e0
> 
  <snip traces>

> Looks like everything is backed up on the inode_lock.  Why? Looks
> like drop_pagecache_sb() is doing something ..... suboptimal.
> 
> static void drop_pagecache_sb(struct super_block *sb)
> {
>         struct inode *inode;
> 
>         spin_lock(&inode_lock);
>         list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
>                 if (inode->i_state & (I_FREEING|I_WILL_FREE))
>                         continue;
>                 __invalidate_mapping_pages(inode->i_mapping, 0, -1, true);
>         }
>         spin_unlock(&inode_lock);
> }
> 
> It holds the inode_lock for an amazingly long time, and calls a
> function that ends up in ->release_page which can issue
> transactions.
> 
> Given that transactions can then mark an inode dirty or the
> kjournald might need to mark an inode dirty while holding
> transaction locks, the implementation of drop_pagecache_sb seems to
> be just a little dangerous....
> 
> Anyone know the reason why drop_pagecache_sb() uses such a brute-force
> mechanism to free up clean page cache pages?
  Yes, we know that drop_pagecache_sb() has locking issues but since it
is intended to be used for debugging purposes only, nobody cared enough
to fix it. Completely untested patch below if you dare to try ;)

								Honza
-- 
Jan Kara <jack@xxxxxxx>
SuSE CR Labs
---

From: Jan Kara <jack@xxxxxxx>
Date: Tue, 18 Mar 2008 14:38:06 +0100
Subject: [PATCH] Fix drop_pagecache_sb() to not call __invalidate_mapping_pages() under
inode_lock.

Signed-off-by: Jan Kara <jack@xxxxxxx>
---
 fs/drop_caches.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 59375ef..f5aae26 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -14,15 +14,21 @@ int sysctl_drop_caches;
 
 static void drop_pagecache_sb(struct super_block *sb)
 {
-	struct inode *inode;
+	struct inode *inode, *toput_inode = NULL;
 
 	spin_lock(&inode_lock);
 	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
 		if (inode->i_state & (I_FREEING|I_WILL_FREE))
 			continue;
+		__iget(inode);
+		spin_unlock(&inode_lock);
 		__invalidate_mapping_pages(inode->i_mapping, 0, -1, true);
+		iput(toput_inode);
+		toput_inode = inode;
+		spin_lock(&inode_lock);
 	}
 	spin_unlock(&inode_lock);
+	iput(toput_inode);
 }
 
 void drop_pagecache(void)
-- 
1.5.2.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/