[PATCH 0/2] iterate_bdevs() cleanups

[PATCH 0/2] iterate_bdevs() cleanups

[Jan Kara]

  Hi Al,

  here are two changes to iterate_bdevs() we spoke about. And this time
I tested them and things seem to work as expected ;).

								Honza

[PATCH 1/2] fs: Change iterate_bdevs() to iterate over all_bdevs list instead of blockdev_superblock

[Jan Kara]

It is better to iterate over all_bdevs list in iterate_bdevs() because
inode list of blockdev_super contains also inodes which are not describing
any block device (e.g. root of bdev filesystem).

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/block_dev.c |   45 +++++++++++++++++++++++++--------------------
 1 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 38e721b..220391c 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1716,36 +1716,41 @@ EXPORT_SYMBOL(__invalidate_device);
 
 void iterate_bdevs(void (*func)(struct block_device *, void *), void *arg)
 {
-	struct inode *inode, *old_inode = NULL;
-
-	spin_lock(&inode_sb_list_lock);
-	list_for_each_entry(inode, &blockdev_superblock->s_inodes, i_sb_list) {
-		struct address_space *mapping = inode->i_mapping;
+	struct block_device *bdev, *old_bdev = NULL;
+	struct inode *inode;
 
+	spin_lock(&bdev_lock);
+	list_for_each_entry(bdev, &all_bdevs, bd_list) {
+		inode = bdev->bd_inode;
 		spin_lock(&inode->i_lock);
+		/*
+		 * With i_lock we can safely check bdev isn't freeing and
+		 * grab our reference.
+		 */
 		if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW) ||
-		    mapping->nrpages == 0) {
+		    inode->i_mapping->nrpages == 0) {
 			spin_unlock(&inode->i_lock);
 			continue;
 		}
-		__iget(inode);
+		bdgrab(bdev);
 		spin_unlock(&inode->i_lock);
-		spin_unlock(&inode_sb_list_lock);
+		spin_unlock(&bdev_lock);
 		/*
-		 * We hold a reference to 'inode' so it couldn't have been
-		 * removed from s_inodes list while we dropped the
-		 * inode_sb_list_lock.  We cannot iput the inode now as we can
-		 * be holding the last reference and we cannot iput it under
-		 * inode_sb_list_lock. So we keep the reference and iput it
-		 * later.
+		 * We hold a reference to 'bdev' so it couldn't have been
+		 * removed from the list while we dropped the bdev_lock.  We
+		 * cannot put the reference now as we can be holding the last
+		 * reference and we cannot put it under bdev_lock. So we keep
+		 * the reference and put it later.
 		 */
-		iput(old_inode);
-		old_inode = inode;
+		if (old_bdev)
+			bdput(old_bdev);
+		old_bdev = bdev;
 
-		func(I_BDEV(inode), arg);
+		func(bdev, arg);
 
-		spin_lock(&inode_sb_list_lock);
+		spin_lock(&bdev_lock);
 	}
-	spin_unlock(&inode_sb_list_lock);
-	iput(old_inode);
+	spin_unlock(&bdev_lock);
+	if (old_bdev)
+		bdput(old_bdev);
 }
-- 
1.7.1

[PATCH 2/2] fs: Move check for mappings without pages from iterate_bdevs()

[Jan Kara]

Currently, iterate_bdevs() skips block devices without any pages in page
cache. That is fine for current use by sync(2) but may be rather surprising
for possible future users. So move the checks from iterate_bdevs() to
callback functions used by sync(2).

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/block_dev.c |    3 +--
 fs/sync.c      |   12 ++++++++++--
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 220391c..ad31709 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1727,8 +1727,7 @@ void iterate_bdevs(void (*func)(struct block_device *, void *), void *arg)
 		 * With i_lock we can safely check bdev isn't freeing and
 		 * grab our reference.
 		 */
-		if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW) ||
-		    inode->i_mapping->nrpages == 0) {
+		if (inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) {
 			spin_unlock(&inode->i_lock);
 			continue;
 		}
diff --git a/fs/sync.c b/fs/sync.c
index eb8722d..536a272 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -81,12 +81,20 @@ static void sync_fs_one_sb(struct super_block *sb, void *arg)
 
 static void fdatawrite_one_bdev(struct block_device *bdev, void *arg)
 {
-	filemap_fdatawrite(bdev->bd_inode->i_mapping);
+	struct address_space *mapping = bdev->bd_inode->i_mapping;
+
+	if (mapping->nrpages == 0)
+		return;
+	filemap_fdatawrite(mapping);
 }
 
 static void fdatawait_one_bdev(struct block_device *bdev, void *arg)
 {
-	filemap_fdatawait(bdev->bd_inode->i_mapping);
+	struct address_space *mapping = bdev->bd_inode->i_mapping;
+
+	if (mapping->nrpages == 0)
+		return;
+	filemap_fdatawait(mapping);
 }
 
 /*
-- 
1.7.1