[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH -next] block: remove field 'bd_inode' from block_device
Hi, 在 2023/11/25 22:32, Greg KH 写道: On Sat, Nov 25, 2023 at 05:39:12PM +0800, Yu Kuai wrote:From: Yu Kuai <yukuai3@xxxxxxxxxx> block_devcie is allocated from bdev_alloc() by bdev_alloc_inode(), and currently block_device contains a pointer that point to the address of inode, while such inode is allocated together: bdev_alloc inode = new_inode() // inode is &bdev_inode->vfs_inode bdev = I_BDEV(inode) // bdev is &bdev_inode->bdev bdev->inode = inode Add a new helper to get address of inode from bdev by add operation instead of memory access, which is more efficiency. Also prepare to add a new field 'bd_flags' in the first cacheline(64 bytes). Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx> --- block/bdev.c | 39 +++++++++++++++++------------- block/blk-zoned.c | 4 +-- block/fops.c | 4 +-- block/genhd.c | 8 +++--- block/ioctl.c | 8 +++--- block/partitions/core.c | 9 ++++--- drivers/block/xen-blkback/xenbus.c | 2 +- drivers/md/bcache/super.c | 2 +- drivers/mtd/devices/block2mtd.c | 12 ++++----- drivers/s390/block/dasd_ioctl.c | 2 +- drivers/scsi/scsicam.c | 2 +- fs/bcachefs/util.h | 2 +- fs/btrfs/disk-io.c | 6 ++--- fs/btrfs/volumes.c | 4 +-- fs/btrfs/zoned.c | 2 +- fs/buffer.c | 8 +++--- fs/cramfs/inode.c | 2 +- fs/erofs/data.c | 2 +- fs/ext4/dir.c | 2 +- fs/ext4/ext4_jbd2.c | 2 +- fs/ext4/super.c | 8 +++--- fs/gfs2/glock.c | 2 +- fs/gfs2/ops_fstype.c | 2 +- fs/jbd2/journal.c | 3 ++- fs/jbd2/recovery.c | 2 +- fs/nilfs2/segment.c | 2 +- include/linux/blk_types.h | 10 ++++++-- include/linux/blkdev.h | 4 +-- include/linux/buffer_head.h | 4 +-- 29 files changed, 86 insertions(+), 73 deletions(-)You should do this as a patch series, add the helper function that does nothing, convert all the different portions of the kernel as different patches, and _then_ change the implementation of the block layer to handle the change in the structure. Otherwise this is going to be hard to get accepted. Okay, thanks for the adivce, I'll do that in v2. By the way, I was thinking that this patch is quite simple, and doesn't worth spliting into 10+ patches, Also, one note:@@ -85,6 +84,13 @@ struct block_device { #define bdev_kobj(_bdev) \ (&((_bdev)->bd_device.kobj))+static inline struct inode *bdev_inode(struct block_device *bdev)+{ + void *inode = bdev + 1;That's crazy, if something changes, this will keep working yet the kernel will break and no one will know why. Please use container_of(), that's what it is there for, this exact type of thing. Or if not, are you just assuming that the memory location right after bdev is the inode? That's a tough assumption, how are you going to assure it really stays there? Struct bdev_inode never changes since commit 8fbd544cbca5 ("[PATCH] bdev: add I_BDEV()") from 2004, and I think it won't change unless there is a different way to manage lifetime of block_device. And the 'bdev + 1' is copied from blk_mq_rq_to_pdu(), however, I aggre that use container_of() is better and I will use it in v2. Thanks, Kuai thanks, greg k-h .
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |