Re: [PATCH v3 1/3] vfs: support caching symlink lengths in inodes

[Dave Chinner <david@fromorbit.com>]

On Thu, Nov 21, 2024 at 11:12:52AM +0100, Christian Brauner wrote:
> I think that i_devices should be moved into the union as it's really
> only used with i_cdev but it's not that easily done because list_head
> needs to be initialized.

I'm planning on using i_devices with block devices, too, so the
block device list doesn't need to use i_sb_list anymore (similar to
how i_devices is used by the char dev infrastructure. See the patch
below...

> I roughly envisioned something like:
> 
> union {
>         struct {
>                 struct cdev             *i_cdev;
>                 struct list_head        i_devices;
>         };
>         struct {
>                 char                    *i_link;
>                 unsigned int            i_link_len;
>         };
>         struct pipe_inode_info          *i_pipe;
>         unsigned                        i_dir_seq;
> };
> 

I'd probably have to undo any unioning/association with i_cdev to
use i_devices with block devs...

-Dave
-- 
Dave Chinner
david@fromorbit.com


bdev: stop using sb->s_inodes

From: Dave Chinner <dchinner@redhat.com>

Iteration of block device inodes is done via the
blockdev_superblock->s_inodes list. We want to remove this list and
the inode i_sb_list list heads, so we need some other way for block
devices to be iterated.

Take a leaf from the chardev code and use the inode->i_devices list
head to link all the block device inodes together and replace the
s_inodes list with a bdev private global list.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 block/bdev.c | 56 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 33f9c4605e3a..d733507f584a 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -317,6 +317,8 @@ EXPORT_SYMBOL(bdev_thaw);
 
 static  __cacheline_aligned_in_smp DEFINE_MUTEX(bdev_lock);
 static struct kmem_cache *bdev_cachep __ro_after_init;
+static LIST_HEAD(bdev_inodes);
+static DEFINE_SPINLOCK(bdev_inodes_lock);
 
 static struct inode *bdev_alloc_inode(struct super_block *sb)
 {
@@ -362,6 +364,10 @@ static void init_once(void *data)
 
 static void bdev_evict_inode(struct inode *inode)
 {
+	spin_lock(&bdev_inodes_lock);
+	list_del_init(&inode->i_devices);
+	spin_unlock(&bdev_inodes_lock);
+
 	truncate_inode_pages_final(&inode->i_data);
 	invalidate_inode_buffers(inode); /* is it needed here? */
 	clear_inode(inode);
@@ -412,19 +418,35 @@ void __init bdev_cache_init(void)
 	blockdev_superblock = blockdev_mnt->mnt_sb;   /* For writeback */
 }
 
-struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
+static struct inode *bdev_new_inode(void)
 {
-	struct block_device *bdev;
 	struct inode *inode;
 
-	inode = new_inode(blockdev_superblock);
+	inode = new_inode_pseudo(blockdev_superblock);
 	if (!inode)
 		return NULL;
+
+	spin_lock(&bdev_inodes_lock);
+	list_add(&inode->i_devices, &bdev_inodes);
+	spin_unlock(&bdev_inodes_lock);
+
 	inode->i_mode = S_IFBLK;
 	inode->i_rdev = 0;
 	inode->i_data.a_ops = &def_blk_aops;
 	mapping_set_gfp_mask(&inode->i_data, GFP_USER);
 
+	return inode;
+}
+
+struct block_device *bdev_alloc(struct gendisk *disk, u8 partno)
+{
+	struct block_device *bdev;
+	struct inode *inode;
+
+	inode = bdev_new_inode();
+	if (!inode)
+		return NULL;
+
 	bdev = I_BDEV(inode);
 	mutex_init(&bdev->bd_fsfreeze_mutex);
 	spin_lock_init(&bdev->bd_size_lock);
@@ -477,10 +499,10 @@ long nr_blockdev_pages(void)
 	struct inode *inode;
 	long ret = 0;
 
-	spin_lock(&blockdev_superblock->s_inode_list_lock);
-	list_for_each_entry(inode, &blockdev_superblock->s_inodes, i_sb_list)
+	spin_lock(&bdev_inodes_lock);
+	list_for_each_entry(inode, &bdev_inodes, i_devices)
 		ret += inode->i_mapping->nrpages;
-	spin_unlock(&blockdev_superblock->s_inode_list_lock);
+	spin_unlock(&bdev_inodes_lock);
 
 	return ret;
 }
@@ -1218,8 +1240,8 @@ void sync_bdevs(bool wait)
 {
 	struct inode *inode, *old_inode = NULL;
 
-	spin_lock(&blockdev_superblock->s_inode_list_lock);
-	list_for_each_entry(inode, &blockdev_superblock->s_inodes, i_sb_list) {
+	spin_lock(&bdev_inodes_lock);
+	list_for_each_entry(inode, &bdev_inodes, i_devices) {
 		struct address_space *mapping = inode->i_mapping;
 		struct block_device *bdev;
 
@@ -1231,14 +1253,14 @@ void sync_bdevs(bool wait)
 		}
 		__iget(inode);
 		spin_unlock(&inode->i_lock);
-		spin_unlock(&blockdev_superblock->s_inode_list_lock);
+		spin_unlock(&bdev_inodes_lock);
+
 		/*
-		 * We hold a reference to 'inode' so it couldn't have been
-		 * removed from s_inodes list while we dropped the
-		 * s_inode_list_lock  We cannot iput the inode now as we can
-		 * be holding the last reference and we cannot iput it under
-		 * s_inode_list_lock. So we keep the reference and iput it
-		 * later.
+		 * We hold a reference to 'inode' so it won't get removed from
+		 * bdev inodes list while we drop the lock.  We need to hold the
+		 * reference until we have a reference on the next inode on the
+		 * list, so we can't drop it until the next time we let go of
+		 * the bdev_inodes_lock.
 		 */
 		iput(old_inode);
 		old_inode = inode;
@@ -1260,9 +1282,9 @@ void sync_bdevs(bool wait)
 		}
 		mutex_unlock(&bdev->bd_disk->open_mutex);
 
-		spin_lock(&blockdev_superblock->s_inode_list_lock);
+		spin_lock(&bdev_inodes_lock);
 	}
-	spin_unlock(&blockdev_superblock->s_inode_list_lock);
+	spin_unlock(&bdev_inodes_lock);
 	iput(old_inode);
 }